-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(browser): Add environment variable support for Spotlight configuration #18198
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
Open
BYK
wants to merge
46
commits into
develop
Choose a base branch
from
feat/spotlight-environment-variable-support
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,989
−251
Open
Changes from all commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
6cb5513
feat(browser): Add environment variable support for Spotlight configu…
BYK f16380f
change priority order for SENTRY_SPOTLIGHT
BYK fda3d61
fixup changelog
BYK bcff50d
some more clean up
BYK cd42ebd
address issues raised by agents
BYK 0ac2828
Fix test failure in CI (#18210)
BYK 0915965
Merge branch 'develop' into feat/spotlight-environment-variable-support
BYK 6ea8d90
Process and resolve pull request comments (#18235)
BYK 870be3b
Merge branch 'master' into feat/spotlight-environment-variable-support
BYK 1cf7f7f
Merge branch 'develop' into feat/spotlight-environment-variable-support
BYK b6fda2b
some tests etc
BYK 469940b
fix tests
BYK be6f1b5
add env2bool export back just in case
BYK c9b13e3
fix lint
BYK d129a79
Merge branch 'develop' into feat/spotlight-environment-variable-support
BYK f542f62
fix ts errors
BYK 4c28587
fix prettier
BYK f9b555a
lint fixes
BYK e12b3fc
fix tests
BYK ab56171
fix prettification
BYK 6cbd1fd
type error
BYK 3bc5309
remove module field to fix webpack 4
BYK c78da67
remove unused ts-ignore
BYK fb3f764
fix maybe?
BYK d556863
fixes
BYK 64c4692
works now?
BYK 75f2b49
sigh
BYK 9f1e28f
really?
BYK a23d53a
fix(e2e): explicitly pass spotlight env var to init in production builds
BYK 90eca4e
fix(e2e): use development SDK exports for Spotlight env var tests
BYK f16915a
fix(e2e): use vite dev server for realistic Spotlight testing
BYK c7c9b95
feat(e2e): add Next.js Spotlight dev mode test application
BYK cb298e2
prettification
BYK 5fdcf7d
fix(e2e): configure Next.js to use development export conditions
BYK c12edfc
feat(nextjs): auto-enable development export conditions in dev mode
BYK 2ea8c45
fix(e2e): revert Vite app to use production build with dev SDK exports
BYK 433c0ac
fix(e2e): update errors.test.ts to handle dev-mode filenames
BYK 212a8bb
fix(nextjs): add conditionNames to WebpackConfigObject resolve type
BYK 516ede0
fix(e2e): fix worker filename regex patterns
BYK 5df223a
fix(nextjs): revert automatic development conditions
BYK 102dc61
fix(nextjs): preserve existing conditionNames when adding development
BYK d4662e5
fix(nextjs): handle case where conditionNames doesn't exist
BYK 7cffb67
fix(nextjs): add browser/node condition for client/server bundles
BYK dde88ca
fix(nextjs): use resolve.alias to point @sentry/browser to dev build
BYK 9c6d19a
change approach
BYK de9943d
hmm
BYK File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
199 changes: 199 additions & 0 deletions
199
dev-packages/e2e-tests/test-applications/SPOTLIGHT_ENV_TESTS.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,199 @@ | ||
| # Spotlight Environment Variable E2E Tests | ||
|
|
||
| This document describes the E2E tests for Spotlight environment variable handling across different bundlers and module formats. | ||
|
|
||
| ## Overview | ||
|
|
||
| The Sentry JavaScript SDK supports Spotlight configuration via environment variables. The implementation must handle: | ||
|
|
||
| 1. **Multiple environment variable prefixes** for different frameworks (e.g., `NEXT_PUBLIC_*`, `VITE_*`) | ||
| 2. **Different module formats** (ESM vs CJS) | ||
| 3. **Different environment variable access methods** (`process.env` vs `import.meta.env`) | ||
| 4. **Empty string filtering** (empty strings should be treated as undefined) | ||
|
|
||
| ## Test Coverage | ||
|
|
||
| ### Next.js Tests (`nextjs-15/tests/spotlight-env.test.ts`) | ||
|
|
||
| Tests the **CJS build** scenario where `import.meta` must be stripped to avoid syntax errors. | ||
|
|
||
| **Test Page**: `/spotlight-env-test` | ||
| **Source**: `nextjs-15/app/spotlight-env-test/page.tsx` | ||
|
|
||
| #### Tests: | ||
|
|
||
| 1. **`respects NEXT_PUBLIC_SENTRY_SPOTLIGHT environment variable`** | ||
| - Verifies that `NEXT_PUBLIC_SENTRY_SPOTLIGHT=true` enables Spotlight | ||
| - Checks that the Spotlight integration is registered | ||
|
|
||
| 2. **`NEXT_PUBLIC_SENTRY_SPOTLIGHT takes precedence over SENTRY_SPOTLIGHT`** | ||
| - Verifies that `SENTRY_SPOTLIGHT` (backend-only) is not accessible in browser | ||
| - Ensures framework-specific vars have priority | ||
|
|
||
| 3. **`handles empty string environment variables correctly`** | ||
| - Documents expected behavior: empty strings should disable Spotlight | ||
| - Tests that `resolveSpotlightOptions` filters empty strings | ||
|
|
||
| 4. **`process.env check works without errors in CJS build`** | ||
| - **Critical test**: Verifies no `import.meta` syntax errors in CJS build | ||
| - Checks that the rollup plugin successfully stripped ESM-only code | ||
| - Monitors console for syntax errors | ||
|
|
||
| #### Environment Setup: | ||
|
|
||
| ```bash | ||
| NEXT_PUBLIC_SENTRY_SPOTLIGHT=true | ||
| # SENTRY_SPOTLIGHT can be set for backend, but won't be exposed to browser | ||
| ``` | ||
|
|
||
| ### Vite Tests (`browser-webworker-vite/tests/spotlight-env.test.ts`) | ||
|
|
||
| Tests the **ESM build** scenario where `import.meta` should be present and functional. | ||
|
|
||
| **Test Page**: `/spotlight-env-test.html` | ||
| **Source**: `browser-webworker-vite/src/spotlight-env-test.ts` | ||
|
|
||
| #### Tests: | ||
|
|
||
| 1. **`respects VITE_SENTRY_SPOTLIGHT environment variable`** | ||
| - Verifies that `VITE_SENTRY_SPOTLIGHT=true` enables Spotlight | ||
| - Checks that the Spotlight integration is registered | ||
|
|
||
| 2. **`import.meta.env is available in ESM build`** | ||
| - **Critical test**: Verifies `import.meta` is present in ESM builds | ||
| - Checks that `import.meta.env.VITE_SENTRY_SPOTLIGHT` is accessible | ||
| - Confirms the build format is ESM | ||
|
|
||
| 3. **`process.env also works via Vite transformation`** | ||
| - Verifies that Vite transforms `process.env` references | ||
| - Both `process.env` and `import.meta.env` should work | ||
|
|
||
| 4. **`handles empty string environment variables correctly`** | ||
| - Documents expected behavior for empty strings | ||
| - Tests that `resolveSpotlightOptions` filters empty strings | ||
|
|
||
| 5. **`no syntax errors from import.meta in ESM build`** | ||
| - Verifies no syntax errors when using `import.meta` | ||
| - Monitors console for errors | ||
|
|
||
| 6. **`getEnvValue function works with import.meta.env`** | ||
| - Tests the `getEnvValue` utility function | ||
| - Verifies it successfully reads from `import.meta.env` | ||
|
|
||
| #### Environment Setup: | ||
|
|
||
| ```bash | ||
| VITE_SENTRY_SPOTLIGHT=true | ||
| ``` | ||
|
|
||
| ## Implementation Details | ||
|
|
||
| ### Rollup Plugins | ||
|
|
||
| Two rollup plugins handle module-format-specific code: | ||
|
|
||
| 1. **`makeStripEsmPlugin()`** - Strips ESM-only code from CJS builds | ||
| - Removes code between `/* rollup-esm-only */` and `/* rollup-esm-only-end */` | ||
| - Applied to all CJS builds | ||
|
|
||
| 2. **`makeStripCjsPlugin()`** - Strips CJS-only code from ESM builds | ||
| - Removes code between `/* rollup-cjs-only */` and `/* rollup-cjs-only-end */` | ||
| - Applied to all ESM builds | ||
|
|
||
| ### Source Code | ||
|
|
||
| **File**: `packages/browser/src/utils/env.ts` | ||
|
|
||
| The `import.meta.env` check is wrapped in special comments: | ||
|
|
||
| ```typescript | ||
| /* rollup-esm-only */ | ||
| // Check import.meta.env (Vite, Astro, SvelteKit, etc.) | ||
| try { | ||
| if (typeof import.meta !== 'undefined' && import.meta.env) { | ||
| const value = import.meta.env[key]; | ||
| if (value !== undefined) { | ||
| return value; | ||
| } | ||
| } | ||
| } catch (e) { | ||
| // Silently ignore | ||
| } | ||
| /* rollup-esm-only-end */ | ||
| ``` | ||
|
|
||
| This code is: | ||
|
|
||
| - **Included** in ESM builds (Vite, Astro, SvelteKit) | ||
| - **Stripped** from CJS builds (Next.js, Webpack, etc.) | ||
|
|
||
| ### Empty String Handling | ||
|
|
||
| **File**: `packages/core/src/utils/resolveSpotlightOptions.ts` | ||
|
|
||
| The shared `resolveSpotlightOptions` function filters empty/whitespace strings: | ||
|
|
||
| ```typescript | ||
| // Treat empty/whitespace-only strings as undefined | ||
| const envUrl = typeof envSpotlight === 'string' && envSpotlight.trim() !== '' ? envSpotlight : undefined; | ||
| ``` | ||
|
|
||
| This ensures: | ||
|
|
||
| - Empty strings never enable Spotlight | ||
| - Whitespace-only strings are treated as undefined | ||
| - No invalid URL connections are attempted | ||
|
|
||
| ## Running the Tests | ||
|
|
||
| ### Next.js Tests | ||
|
|
||
| ```bash | ||
| cd dev-packages/e2e-tests/test-applications/nextjs-15 | ||
| NEXT_PUBLIC_SENTRY_SPOTLIGHT=true pnpm test tests/spotlight-env.test.ts | ||
| ``` | ||
|
|
||
| ### Vite Tests | ||
|
|
||
| ```bash | ||
| cd dev-packages/e2e-tests/test-applications/browser-webworker-vite | ||
| VITE_SENTRY_SPOTLIGHT=true pnpm test tests/spotlight-env.test.ts | ||
| ``` | ||
|
|
||
| ## Expected Outcomes | ||
|
|
||
| ### Next.js (CJS) | ||
|
|
||
| - ✅ `process.env.NEXT_PUBLIC_SENTRY_SPOTLIGHT` accessible | ||
| - ✅ `SENTRY_SPOTLIGHT` NOT accessible (backend-only) | ||
| - ✅ No `import.meta` syntax in output | ||
| - ✅ No syntax errors | ||
| - ✅ Spotlight integration enabled | ||
|
|
||
| ### Vite (ESM) | ||
|
|
||
| - ✅ `import.meta.env.VITE_SENTRY_SPOTLIGHT` accessible | ||
| - ✅ `process.env.VITE_SENTRY_SPOTLIGHT` NOT accessible (Vite only exposes import.meta.env) | ||
| - ✅ `import.meta` syntax present in output | ||
| - ✅ No syntax errors | ||
| - ✅ Spotlight integration enabled | ||
|
|
||
| ## Troubleshooting | ||
|
|
||
| ### Syntax Error: Cannot use import.meta outside a module | ||
|
|
||
| - **Cause**: `import.meta` code not stripped from CJS build | ||
| - **Fix**: Verify `makeStripEsmPlugin()` is applied to CJS builds | ||
| - **Check**: Look for `/* rollup-esm-only */` comments in source | ||
|
|
||
| ### Spotlight not enabled despite env var set | ||
|
|
||
| - **Cause**: Empty string or wrong prefix | ||
| - **Fix**: Use correct prefix (`NEXT_PUBLIC_*` for Next.js, `VITE_*` for Vite) | ||
| - **Check**: Verify `resolveSpotlightOptions` receives non-empty string | ||
|
|
||
| ### import.meta.env returns undefined in Vite | ||
|
|
||
| - **Cause**: `import.meta` code stripped from ESM build | ||
| - **Fix**: Verify `makeStripEsmPlugin()` is NOT applied to ESM builds | ||
| - **Check**: ESM builds should only use `makeStripCjsPlugin()` |
20 changes: 10 additions & 10 deletions
20
dev-packages/e2e-tests/test-applications/angular-17/src/index.html
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,13 @@ | ||
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>Angular17</title> | ||
| <base href="/"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1"> | ||
| <link rel="icon" type="image/x-icon" href="favicon.ico"> | ||
| </head> | ||
| <body> | ||
| <app-root></app-root> | ||
| </body> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>Angular17</title> | ||
| <base href="/" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
| <link rel="icon" type="image/x-icon" href="favicon.ico" /> | ||
| </head> | ||
| <body> | ||
| <app-root></app-root> | ||
| </body> | ||
| </html> |
20 changes: 10 additions & 10 deletions
20
dev-packages/e2e-tests/test-applications/angular-18/src/index.html
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,13 @@ | ||
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>Angular 18</title> | ||
| <base href="/"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1"> | ||
| <link rel="icon" type="image/x-icon" href="favicon.ico"> | ||
| </head> | ||
| <body> | ||
| <app-root></app-root> | ||
| </body> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>Angular 18</title> | ||
| <base href="/" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
| <link rel="icon" type="image/x-icon" href="favicon.ico" /> | ||
| </head> | ||
| <body> | ||
| <app-root></app-root> | ||
| </body> | ||
| </html> |
20 changes: 10 additions & 10 deletions
20
dev-packages/e2e-tests/test-applications/angular-19/src/index.html
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,13 @@ | ||
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>Angular19</title> | ||
| <base href="/"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1"> | ||
| <link rel="icon" type="image/x-icon" href="favicon.ico"> | ||
| </head> | ||
| <body> | ||
| <app-root></app-root> | ||
| </body> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>Angular19</title> | ||
| <base href="/" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
| <link rel="icon" type="image/x-icon" href="favicon.ico" /> | ||
| </head> | ||
| <body> | ||
| <app-root></app-root> | ||
| </body> | ||
| </html> |
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
3 changes: 2 additions & 1 deletion
3
dev-packages/e2e-tests/test-applications/browser-webworker-vite/playwright.config.mjs
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
38 changes: 38 additions & 0 deletions
38
dev-packages/e2e-tests/test-applications/browser-webworker-vite/spotlight-env-test.html
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Spotlight Env Test - Vite</title> | ||
| </head> | ||
| <body> | ||
| <h1>Spotlight Environment Variable Test (Vite)</h1> | ||
|
|
||
| <div id="env-vars" data-testid="env-vars"> | ||
| <h2>Environment Variables</h2> | ||
| <div data-testid="vite-spotlight">VITE_SENTRY_SPOTLIGHT: <span id="vite-spotlight-value">Loading...</span></div> | ||
| <div data-testid="process-env-spotlight"> | ||
| process.env.SENTRY_SPOTLIGHT: <span id="process-env-value">Loading...</span> | ||
| </div> | ||
| <div data-testid="import-meta-env-spotlight"> | ||
| import.meta.env.VITE_SENTRY_SPOTLIGHT: <span id="import-meta-value">Loading...</span> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div id="spotlight-status" data-testid="spotlight-status"> | ||
| <h2>Spotlight Integration Status</h2> | ||
| <div data-testid="spotlight-integration-found"> | ||
| Spotlight Integration: <span id="spotlight-status-value">Loading...</span> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div id="build-format" data-testid="build-format"> | ||
| <h2>Build Format Check</h2> | ||
| <div data-testid="import-meta-available"> | ||
| import.meta available: <span id="import-meta-available-value">Loading...</span> | ||
| </div> | ||
| </div> | ||
|
|
||
| <script type="module" src="/src/spotlight-env-test.ts"></script> | ||
| </body> | ||
| </html> |
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.