Skip to content

Commit c8ca286

Browse files
BYKcursoragent
andauthored
fix(node): Fix Spotlight configuration precedence to match specification (#18195)
## Problem The Spotlight configuration logic had a precedence bug where when `spotlight: true` was set in config AND the `SENTRY_SPOTLIGHT` environment variable contained a URL string, the SDK would incorrectly use `true` instead of the URL from the environment variable. According to the [Spotlight specification](https://raw.githubusercontent.com/getsentry/sentry-docs/b38e3b307f900665a348f855559ac1d1c58914cc/develop-docs/sdk/expected-features/spotlight.mdx), when `spotlight: true` is set and the env var contains a URL, the URL from the env var should be used to allow developers to override the Spotlight URL via environment variables. **Previous behavior:** ```typescript // Config: spotlight: true // Env: SENTRY_SPOTLIGHT=http://custom:3000/stream // Result: spotlight = true ❌ (incorrect) ``` **Expected behavior per spec:** ```typescript // Config: spotlight: true // Env: SENTRY_SPOTLIGHT=http://custom:3000/stream // Result: spotlight = "http://custom:3000/stream" ✅ (correct) ``` ## Solution Fixed the precedence logic in `getClientOptions()` to properly implement the specification: 1. `spotlight: false` → Always disabled (overrides env var) 2. `spotlight: string` → Uses the config URL (ignores env var) 3. `spotlight: true` + env var URL → **Uses the env var URL** (the bug fix) 4. `spotlight: true` + env var truthy → Uses default URL 5. No config + env var → Parses and uses env var The implementation reuses the existing `envToBool()` utility to avoid code duplication. ## Changes - Fixed Spotlight precedence logic in `packages/node-core/src/sdk/index.ts` - Added 12 comprehensive test cases covering all precedence scenarios in `packages/node-core/test/sdk/init.test.ts` - Updated CHANGELOG.md ## Test Coverage The new tests cover: - ✅ Env var only: truthy values, falsy values, URL strings - ✅ Config only: `true`, `false`, URL string - ✅ Precedence: config `false` overrides env var (URL, truthy, falsy) - ✅ Precedence: config URL overrides env var - ✅ Precedence: config `true` + env var URL uses env var URL (the fix) - ✅ Precedence: config `true` + env var truthy uses default URL ## Related - Original Spotlight implementation: #13325 - Spotlight specification: https://spotlightjs.com/ --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 7efb1d2 commit c8ca286

File tree

4 files changed

+128
-4
lines changed

4 files changed

+128
-4
lines changed

.size-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ module.exports = [
240240
import: createImport('init'),
241241
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
242242
gzip: true,
243-
limit: '158 KB',
243+
limit: '160 KB',
244244
},
245245
{
246246
name: '@sentry/node - without tracing',

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

0 commit comments

Comments
 (0)