Skip to content

Commit f82f852

Browse files
committed
fix(node): Fix Spotlight configuration precedence to match specification
The Spotlight configuration logic had a precedence bug where when 'spotlight: true' was set AND the 'SENTRY_SPOTLIGHT' env var contained a URL string, the SDK would use 'true' instead of the URL from the env var. According to the Spotlight specification, when 'spotlight: true' is set and the env var contains a URL, the URL should be used. Changes: - Fixed precedence logic in getClientOptions() to properly handle the case where config is 'true' and env var is a URL string - Added 12 comprehensive test cases covering all precedence scenarios - Reused existing envToBool() utility for env var parsing Fixes the issue where developers couldn't override the Spotlight URL via environment variable when using 'spotlight: true' in config.
1 parent 7efb1d2 commit f82f852

File tree

3 files changed

+123
-3
lines changed

3 files changed

+123
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## Unreleased
44

5-
- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
5+
- fix(node): Fix Spotlight configuration precedence to match specification (#18160)
66

77
## 10.25.0
88

packages/node-core/src/sdk/index.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,24 @@ function getClientOptions(
182182
getDefaultIntegrationsImpl: (options: Options) => Integration[],
183183
): NodeClientOptions {
184184
const release = getRelease(options.release);
185-
const spotlight =
186-
options.spotlight ?? envToBool(process.env.SENTRY_SPOTLIGHT, { strict: true }) ?? process.env.SENTRY_SPOTLIGHT;
185+
186+
// Parse spotlight configuration with proper precedence per spec
187+
let spotlight: boolean | string | undefined;
188+
if (options.spotlight === false) {
189+
spotlight = false;
190+
} else if (typeof options.spotlight === 'string') {
191+
spotlight = options.spotlight;
192+
} else {
193+
// options.spotlight is true or undefined
194+
const envBool = envToBool(process.env.SENTRY_SPOTLIGHT, { strict: true });
195+
const envUrl = envBool === null && process.env.SENTRY_SPOTLIGHT ? process.env.SENTRY_SPOTLIGHT : undefined;
196+
197+
spotlight =
198+
options.spotlight === true
199+
? (envUrl ?? true) // true: use env URL if present, otherwise true
200+
: (envBool ?? envUrl); // undefined: use env var (bool or URL)
201+
}
202+
187203
const tracesSampleRate = getTracesSampleRate(options.tracesSampleRate);
188204

189205
const mergedOptions = {

packages/node-core/test/sdk/init.test.ts

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,110 @@ describe('init()', () => {
216216
}),
217217
);
218218
});
219+
220+
describe('spotlight configuration', () => {
221+
it('enables spotlight with default URL from `SENTRY_SPOTLIGHT` env variable (truthy value)', () => {
222+
process.env.SENTRY_SPOTLIGHT = 'true';
223+
224+
const client = init({ dsn: PUBLIC_DSN });
225+
226+
expect(client?.getOptions().spotlight).toBe(true);
227+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(true);
228+
});
229+
230+
it('disables spotlight from `SENTRY_SPOTLIGHT` env variable (falsy value)', () => {
231+
process.env.SENTRY_SPOTLIGHT = 'false';
232+
233+
const client = init({ dsn: PUBLIC_DSN });
234+
235+
expect(client?.getOptions().spotlight).toBe(false);
236+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(false);
237+
});
238+
239+
it('enables spotlight with custom URL from `SENTRY_SPOTLIGHT` env variable', () => {
240+
process.env.SENTRY_SPOTLIGHT = 'http://localhost:3000/stream';
241+
242+
const client = init({ dsn: PUBLIC_DSN });
243+
244+
expect(client?.getOptions().spotlight).toBe('http://localhost:3000/stream');
245+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(true);
246+
});
247+
248+
it('enables spotlight with default URL from config `true`', () => {
249+
const client = init({ dsn: PUBLIC_DSN, spotlight: true });
250+
251+
expect(client?.getOptions().spotlight).toBe(true);
252+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(true);
253+
});
254+
255+
it('disables spotlight from config `false`', () => {
256+
const client = init({ dsn: PUBLIC_DSN, spotlight: false });
257+
258+
expect(client?.getOptions().spotlight).toBe(false);
259+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(false);
260+
});
261+
262+
it('enables spotlight with custom URL from config', () => {
263+
const client = init({ dsn: PUBLIC_DSN, spotlight: 'http://custom:8888/stream' });
264+
265+
expect(client?.getOptions().spotlight).toBe('http://custom:8888/stream');
266+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(true);
267+
});
268+
269+
it('config `false` overrides `SENTRY_SPOTLIGHT` env variable URL', () => {
270+
process.env.SENTRY_SPOTLIGHT = 'http://localhost:3000/stream';
271+
272+
const client = init({ dsn: PUBLIC_DSN, spotlight: false });
273+
274+
expect(client?.getOptions().spotlight).toBe(false);
275+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(false);
276+
});
277+
278+
it('config `false` overrides `SENTRY_SPOTLIGHT` env variable truthy value', () => {
279+
process.env.SENTRY_SPOTLIGHT = 'true';
280+
281+
const client = init({ dsn: PUBLIC_DSN, spotlight: false });
282+
283+
expect(client?.getOptions().spotlight).toBe(false);
284+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(false);
285+
});
286+
287+
it('config `false` with `SENTRY_SPOTLIGHT` env variable falsy value keeps spotlight disabled', () => {
288+
process.env.SENTRY_SPOTLIGHT = 'false';
289+
290+
const client = init({ dsn: PUBLIC_DSN, spotlight: false });
291+
292+
expect(client?.getOptions().spotlight).toBe(false);
293+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(false);
294+
});
295+
296+
it('config URL overrides `SENTRY_SPOTLIGHT` env variable URL', () => {
297+
process.env.SENTRY_SPOTLIGHT = 'http://env:3000/stream';
298+
299+
const client = init({ dsn: PUBLIC_DSN, spotlight: 'http://config:8888/stream' });
300+
301+
expect(client?.getOptions().spotlight).toBe('http://config:8888/stream');
302+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(true);
303+
});
304+
305+
it('config `true` with env var URL uses env var URL', () => {
306+
process.env.SENTRY_SPOTLIGHT = 'http://localhost:3000/stream';
307+
308+
const client = init({ dsn: PUBLIC_DSN, spotlight: true });
309+
310+
expect(client?.getOptions().spotlight).toBe('http://localhost:3000/stream');
311+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(true);
312+
});
313+
314+
it('config `true` with env var truthy value uses default URL', () => {
315+
process.env.SENTRY_SPOTLIGHT = 'true';
316+
317+
const client = init({ dsn: PUBLIC_DSN, spotlight: true });
318+
319+
expect(client?.getOptions().spotlight).toBe(true);
320+
expect(client?.getOptions().integrations.some(integration => integration.name === 'Spotlight')).toBe(true);
321+
});
322+
});
219323
});
220324
});
221325

0 commit comments

Comments
 (0)