Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Sidecar URL issues fixed across spotlight #558

Merged
merged 4 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/twenty-fireants-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@spotlightjs/spotlight': minor
'@spotlightjs/overlay': minor
'@spotlightjs/sidecar': minor
---

- Sidecar url made generic to support all sidecar server routes.
- No use of static sidecar url.
5 changes: 2 additions & 3 deletions packages/overlay/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,9 @@ export default function App({
// See https://github.com/remix-run/react-router/issues/7634
const navigate = useNavigate();
const clearEvents = useCallback(async () => {
const { origin } = new URL(sidecarUrl);
const clearEventsUrl: string = `${origin}/clear`;

try {
const clearEventsUrl: string = new URL('/clear', sidecarUrl).href;

await db.reset();
await fetch(clearEventsUrl, {
method: 'DELETE',
Expand Down
8 changes: 5 additions & 3 deletions packages/overlay/src/components/OpenInEditor.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
import type React from 'react';
import { useCallback } from 'react';
import { useSpotlightContext } from '~/lib/useSpotlightContext';
import { ReactComponent as PenIcon } from '../assets/pen.svg';

export default function OpenInEditor({ file }: { file: string }) {
const { sidecarUrl } = useSpotlightContext();
const sidecarOpenUrl: string = new URL('/open', sidecarUrl).href;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you abstracted out sidecarUrl and this new URL(x, sidecarUrl).href looks like a pattern, do you think we can change the variable from the context to a function:

const { getSidecarUrl } = useSpotlightContext();
const openUrl = getSidecarUrl('open');

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we can do this

const openInEditor = useCallback(
(evt: React.MouseEvent) => {
// TODO: Make this URL dynamic based on sidecarUrl!
fetch('http://localhost:8969/open', {
fetch(sidecarOpenUrl, {
method: 'POST',
body: file,
credentials: 'omit',
});
evt.stopPropagation();
},
[file],
[file, sidecarOpenUrl],
);
return (
<PenIcon
Expand Down
3 changes: 2 additions & 1 deletion packages/overlay/src/constants.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export const DEFAULT_SIDECAR_URL = 'http://localhost:8969/stream';
export const DEFAULT_SIDECAR_URL = 'http://localhost:8969';
export const DEFAULT_SIDECAR_STREAM_URL = new URL('/stream', DEFAULT_SIDECAR_URL).href;

export const DEFAULT_EXPERIMENTS = {
'sentry:focus-local-events': true,
Expand Down
11 changes: 7 additions & 4 deletions packages/overlay/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { CONTEXT_LINES_ENDPOINT } from '@spotlightjs/sidecar/constants';
import { MemoryRouter } from 'react-router-dom';
import colors from 'tailwindcss/colors';
import App from './App';
import { DEFAULT_ANCHOR, DEFAULT_EXPERIMENTS, DEFAULT_SIDECAR_URL } from './constants';
import { DEFAULT_ANCHOR, DEFAULT_EXPERIMENTS, DEFAULT_SIDECAR_STREAM_URL } from './constants';
import globalStyles from './index.css?inline';
import { initIntegrations, type SpotlightContext } from './integrations/integration';
import { default as sentry } from './integrations/sentry/index';
Expand All @@ -12,6 +12,7 @@ import { activateLogger, log } from './lib/logger';
import { SpotlightContextProvider } from './lib/useSpotlightContext';
import { React, ReactDOM } from './react-instance'; // Import specific exports
import type { SpotlightOverlayOptions, WindowWithSpotlight } from './types';
import { removeURLSuffix } from './utils/remvoveURLSuffix';

export { default as console } from './integrations/console/index';
export { default as hydrationError } from './integrations/hydration-error/index';
Expand All @@ -22,7 +23,7 @@ export {
CONTEXT_LINES_ENDPOINT,
DEFAULT_ANCHOR,
DEFAULT_EXPERIMENTS,
DEFAULT_SIDECAR_URL,
DEFAULT_SIDECAR_STREAM_URL as DEFAULT_SIDECAR_URL,
off,
on,
React,
Expand Down Expand Up @@ -107,7 +108,7 @@ export async function init(options: SpotlightOverlayOptions = {}) {

const {
openOnInit = false,
sidecarUrl = DEFAULT_SIDECAR_URL,
sidecarUrl = DEFAULT_SIDECAR_STREAM_URL,
anchor = DEFAULT_ANCHOR,
integrations,
experiments = DEFAULT_EXPERIMENTS,
Expand All @@ -117,6 +118,7 @@ export async function init(options: SpotlightOverlayOptions = {}) {
} = options;

const isLoadedFromSidecar = new URL(sidecarUrl).origin === document.location.origin;
const sidecarBaseUrl: string = removeURLSuffix(sidecarUrl, '/stream');

const fullPage = options.fullPage ?? isLoadedFromSidecar;
const showTriggerButton = options.showTriggerButton ?? !fullPage;
Expand All @@ -130,12 +132,13 @@ export async function init(options: SpotlightOverlayOptions = {}) {
const finalExperiments = { ...DEFAULT_EXPERIMENTS, ...experiments };

// Sentry is enabled by default
const defaultIntegrations = () => [sentry({ sidecarUrl })];
const defaultIntegrations = () => [sentry({ sidecarUrl: sidecarBaseUrl })];

const context: SpotlightContext = {
open: openSpotlight,
close: closeSpotlight,
experiments: finalExperiments,
sidecarUrl: sidecarBaseUrl,
};

const [initializedIntegrations] = await initIntegrations(integrations ?? defaultIntegrations(), context);
Expand Down
1 change: 1 addition & 0 deletions packages/overlay/src/integrations/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export type SpotlightContext = {
open: (path: string | undefined) => void;
close: () => void;
experiments: ExperimentsConfig;
sidecarUrl: string;
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class SentryDataCache {

protected subscribers: Map<string, Subscription> = new Map();

protected contextLinesProvider: string = new URL(DEFAULT_SIDECAR_URL).origin + CONTEXT_LINES_ENDPOINT;
protected contextLinesProvider: string = new URL(CONTEXT_LINES_ENDPOINT, DEFAULT_SIDECAR_URL).href;

constructor(
initial: (SentryEvent & {
Expand All @@ -45,8 +45,8 @@ class SentryDataCache {
}

setSidecarUrl(url: string) {
const { origin } = new URL(url);
this.contextLinesProvider = origin + CONTEXT_LINES_ENDPOINT;
const { href: contextLinesProviderUrl } = new URL(CONTEXT_LINES_ENDPOINT, url);
this.contextLinesProvider = contextLinesProviderUrl;
}

inferSdkFromEvent(event: SentryEvent) {
Expand Down
3 changes: 2 additions & 1 deletion packages/overlay/src/integrations/sentry/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Client, Envelope, EnvelopeItem } from '@sentry/types';
import { removeURLSuffix } from '~/utils/remvoveURLSuffix';
import { off, on } from '../../lib/eventTarget';
import { log, warn } from '../../lib/logger';
import type { Integration, RawEventContext } from '../integration';
Expand Down Expand Up @@ -30,7 +31,7 @@ export default function sentryIntegration(options: SentryIntegrationOptions = {}
options.retries = 3;
}
if (options.sidecarUrl) {
sentryDataCache.setSidecarUrl(options.sidecarUrl);
sentryDataCache.setSidecarUrl(removeURLSuffix(options.sidecarUrl, '/stream'));
}
addSpotlightIntegrationToSentry(options);

Expand Down
3 changes: 2 additions & 1 deletion packages/overlay/src/lib/useSpotlightContext.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, { createContext, useContext, type ReactNode } from 'react';
import { DEFAULT_EXPERIMENTS } from '../constants';
import { DEFAULT_EXPERIMENTS, DEFAULT_SIDECAR_URL } from '../constants';
import { type SpotlightContext } from '../integrations/integration';

const Context = createContext<SpotlightContext>({
open: () => {},
close: () => {},
experiments: DEFAULT_EXPERIMENTS,
sidecarUrl: DEFAULT_SIDECAR_URL,
});

export const SpotlightContextProvider: React.FC<{
Expand Down
3 changes: 2 additions & 1 deletion packages/overlay/src/sidecar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export function connectToSidecar(
setOnline: React.Dispatch<React.SetStateAction<boolean>>,
): () => void {
log('Connecting to sidecar at', sidecarUrl);
const source = new EventSource(sidecarUrl);
const sidecarStreamUrl: string = new URL('/stream', sidecarUrl).href;
const source = new EventSource(sidecarStreamUrl);

for (const [contentType, listener] of Object.entries(contentTypeListeners)) {
source.addEventListener(contentType, listener);
Expand Down
17 changes: 17 additions & 0 deletions packages/overlay/src/utils/remvoveURLSuffix.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
export function removeURLSuffix(url: string, suffix: string): string {
try {
const parsedUrl = new URL(url);

if (parsedUrl.pathname.endsWith(suffix)) {
parsedUrl.pathname = parsedUrl.pathname.slice(0, -suffix.length);
}

return parsedUrl.toString();
} catch (error: unknown) {
if (error instanceof TypeError) {
throw new Error(`Invalid URL provided: ${url}. Error: ${error.message}`);
} else {
throw new Error(`An unexpected error occurred: ${String(error)}`);
}
}
}
13 changes: 10 additions & 3 deletions packages/spotlight/src/vite-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const serverOptions = new Set(['importPath', 'integrationNames', 'port']);

export function buildClientInit(options: SpotlightInitOptions) {
const clientOptions = Object.fromEntries(
Object.entries(options).filter(([key, _value]) => !key.startsWith('_') && !serverOptions.has(key)),
Object.entries(options).filter(([key]) => !key.startsWith('_') && !serverOptions.has(key)),
);
let initOptions = JSON.stringify({
...clientOptions,
Expand Down Expand Up @@ -72,6 +72,13 @@ async function sendErrorToSpotlight(err: ErrorPayload['err'], spotlightUrl: stri
const errorLineInContext = contextLines?.indexOf(errorLine);
const event_id = randomBytes(16).toString('hex');
const timestamp = new Date();

const parsedUrl = new URL(spotlightUrl);
let spotlightErrorStreamUrl: string = spotlightUrl;
if (!parsedUrl.pathname.endsWith('/stream')) {
spotlightErrorStreamUrl = new URL('/stream', spotlightUrl).href;
}

const envelope = [
{ event_id, sent_at: timestamp.toISOString() },
{ type: 'event' },
Expand Down Expand Up @@ -117,7 +124,7 @@ async function sendErrorToSpotlight(err: ErrorPayload['err'], spotlightUrl: stri
]
.map(p => JSON.stringify(p))
.join('\n');
return await fetch(spotlightUrl, {
return await fetch(spotlightErrorStreamUrl, {
method: 'POST',
body: envelope,
headers: { 'Content-Type': 'application/x-sentry-envelope' },
Expand Down Expand Up @@ -151,7 +158,7 @@ export default function spotlight(options: SpotlightInitOptions = {}): PluginOpt
res: ServerResponse,
next: Connect.NextFunction,
) {
await sendErrorToSpotlight(err);
await sendErrorToSpotlight(err, options.sidecarUrl);

// The following part is per https://expressjs.com/en/guide/error-handling.html#the-default-error-handler
if (res.headersSent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Set this option if you have the sidecar running on another URL than the default

```ts
init({
sidecarUrl: 'http://localhost:8969/stream',
sidecarUrl: 'http://localhost:8969/sidecar',
});
```

Expand Down
Loading