Uh oh!
There was an error while loading. Please reload this page.
fix(node): Fix Spotlight configuration precedence to match specification - #18195
Merged
Conversation
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.
Contributor
size-limit report 📦
|
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
Before submitting a pull request, please take a look at our [Contributing](https://github.com/getsentry/sentry-javascript/blob/master/CONTRIBUTING.md) guidelines and verify: - [x] If you've added code that should be tested, please add tests. - [x] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`). Fixes a test failure where `process.env.SENTRY_SPOTLIGHT` was not cleaned up after a test, causing environment variable pollution for subsequent tests. The failing test expected `spotlight: true` to resolve to `true`, but due to the lingering environment variable, it received the URL from `SENTRY_SPOTLIGHT` instead. This PR adds an `afterEach` hook to the `spotlight configuration` describe block to ensure `process.env.SENTRY_SPOTLIGHT` is deleted after each test, preventing cross-test contamination. --- <a href="https://cursor.com/background-agent?bcId=bc-9f9696f9-3843-4420-b2e9-26aedf9b0e2e"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-cursor-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-cursor-light.svg"><img alt="Open in Cursor" src="https://cursor.com/open-in-cursor.svg"></picture></a> <a href="https://cursor.com/agents?id=bc-9f9696f9-3843-4420-b2e9-26aedf9b0e2e"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-web-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-web-light.svg"><img alt="Open in Web" src="https://cursor.com/open-in-web.svg"></picture></a> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
![https://cursor.com/open-in-cursor-dark.svg"><source](https://cursor.com/open-in-cursor-dark.svg"><source){kind=link}
![https://cursor.com/open-in-cursor-light.svg"><img](https://cursor.com/open-in-cursor-light.svg"><img){kind=link}
![https://cursor.com/open-in-cursor.svg"></picture></a> <a](https://cursor.com/open-in-cursor.svg"></picture></a> <a){kind=link}
![https://cursor.com/open-in-web-dark.svg"><source](https://cursor.com/open-in-web-dark.svg"><source){kind=link}
![https://cursor.com/open-in-web-light.svg"><img](https://cursor.com/open-in-web-light.svg"><img){kind=link}
![https://cursor.com/open-in-web.svg"></picture></a](https://cursor.com/open-in-web.svg"></picture></a){kind=link}
Lms24
approved these changes
Nov 13, 2025
Member
There was a problem hiding this comment.
LGTM! Thanks!
you can bump the size limit config for the failing entry here:
sentry-javascript/.size-limit.js
Lines 237 to 244 in 2deb000
Feel free to go with 160 or so (I already have that as a limit on a branch of mine, so we'll get there anyway).
BYK
enabled auto-merge (squash)
November 13, 2025 15:30
Uh oh!
There was an error while loading. Please reload this page.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Spotlight configuration logic had a precedence bug where when
spotlight: truewas set in config AND theSENTRY_SPOTLIGHTenvironment variable contained a URL string, the SDK would incorrectly usetrueinstead of the URL from the environment variable.According to the Spotlight specification, when
spotlight: trueis 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:
Expected behavior per spec:
Solution
Fixed the precedence logic in
getClientOptions()to properly implement the specification:spotlight: false→ Always disabled (overrides env var)spotlight: string→ Uses the config URL (ignores env var)spotlight: true+ env var URL → Uses the env var URL (the bug fix)spotlight: true+ env var truthy → Uses default URLThe implementation reuses the existing
envToBool()utility to avoid code duplication.Changes
packages/node-core/src/sdk/index.tspackages/node-core/test/sdk/init.test.tsTest Coverage
The new tests cover:
true,false, URL stringfalseoverrides env var (URL, truthy, falsy)true+ env var URL uses env var URL (the fix)true+ env var truthy uses default URLRelated