Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

## Unreleased

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
- feat(browser): Enable Spotlight via `SENTRY_SPOTLIGHT` env var ([#18050](https://github.com/getsentry/sentry-javascript/pull/18050))
- Truthy values (`true`, `t`, `y`, `yes`, `on`, `1`) enable Spotlight with the default URL
- Any other non-falsy string value is used as a custom Spotlight Sidecar URL
- Falsy values (`false`, `f`, `n`, `no`, `off`, `0`) disable Spotlight

Work in this release was contributed by @hanseo0507. Thank you for your contribution!

Expand Down
12 changes: 12 additions & 0 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ type BrowserSpecificOptions = BrowserClientReplayOptions &
* @default false
*/
propagateTraceparent?: boolean;

/**
* If you use Spotlight by Sentry during development, use
* this option to forward captured Sentry events to Spotlight.
*
* Either set it to true, or provide a specific Spotlight Sidecar URL.
*
* More details: https://spotlightjs.com/
*
* IMPORTANT: Only set this option to `true` while developing, not in production!
*/
spotlight?: boolean | string;
};
/**
* Configuration options for the Sentry Browser SDK.
Expand Down
24 changes: 24 additions & 0 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Client, Integration, Options } from '@sentry/core';
import {
dedupeIntegration,
envToBool,
functionToStringIntegration,
getIntegrationsToSetup,
inboundFiltersIntegration,
Expand All @@ -15,6 +16,7 @@ import { browserSessionIntegration } from './integrations/browsersession';
import { globalHandlersIntegration } from './integrations/globalhandlers';
import { httpContextIntegration } from './integrations/httpcontext';
import { linkedErrorsIntegration } from './integrations/linkederrors';
import { INTEGRATION_NAME as SPOTLIGHT_INTEGRATION_NAME, spotlightBrowserIntegration } from './integrations/spotlight';
import { defaultStackParser } from './stack-parsers';
import { makeFetchTransport } from './transports/fetch';
import { checkAndWarnIfIsEmbeddedBrowserExtension } from './utils/detectBrowserExtension';
Expand Down Expand Up @@ -90,6 +92,17 @@ export function init(options: BrowserOptions = {}): Client | undefined {
const shouldDisableBecauseIsBrowserExtenstion =
!options.skipBrowserExtensionCheck && checkAndWarnIfIsEmbeddedBrowserExtension();

// Read spotlight config from env var with graceful fallback
// This will be replaced at build time by bundlers like webpack/vite
let spotlightFromEnv: boolean | string | undefined;
if (typeof process !== 'undefined' && process.env && process.env.SENTRY_SPOTLIGHT !== undefined) {
const envValue = process.env.SENTRY_SPOTLIGHT;
const parsedEnvValue = envToBool(envValue, { strict: true });
spotlightFromEnv = parsedEnvValue ?? envValue;
}
Comment on lines +95 to +102
Copy link
Member

@Lms24 Lms24 Oct 29, 2025

Choose a reason for hiding this comment

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

So, process.env will almost never* be replaced by bundlers, which means that this code path will almost never work. The only bundler that straight up adds support for process.env.X seems to be Parcel.

Webpack requires users to additionally add a replacement plugin that hard-replaces strings at build time. Rollup requires the same treatment (just with its own plugin architecture, or something like rollup-plugin-replace). In either case, this current code would fail because while process might be undefined, process.env.SENTRY_SPOTLIGHT could be "defined" (in the sense of the whole thing being hard-replaced at build time). See my code suggestion for a tentative workaround.

Vite (the one I care most about, next to webpack), can dynamically replace any env variable that is prefixed with VITE_. This is a security measure to avoid users leaking keys into their public browser bundles (actually a good thing IMHO). So users would have to define VITE_SENTRY_SPOTLIGHT instead of SENTRY_SPOTLIGHT. As I understand it, this again requires a "special treatment" for Spotlight in browser which I think you want to avoid.

Now the delightful JS ecosystem can't even agree on this VITE_ prefix, so Vite allows users to customize their public prefix via envPrefix.

In a similar manner, some frameworks built their own abstractions around env variables, for example NextJS but it requires a NEXT_PUBLIC_ prefix. The good news is that process.env.NEXT_PUBLIC_SENTRY_SPOTLIGHT would work. SvelteKit allows env variables prefixed with PUBLIC_ to be replaced in its browser bundles but they must be explicitly imported. Nuxt seems to prefer defining env variables explicitly in a runtime config file.

Older frameworks like Angular or Ember don't have env variable support at all but instead recommend using environment.(prod|dev|...).ts files with the respective variables defined in an environment object.

So, as you can see, there's no universal solution that will make this "just work". So we have two choices:
a) apply a fail-safe best-effort check and document that setting SENTRY_SPOTLIGHT might sometimes work depending on framework and bundler but other times it won't in which case users need to fall back to setting the option themselves.
b) not bother with this and the bytes this costs us but simply make it explicit for users to set spotlight: true|false on their terms, with the check they prefer.

In case you want to go with a (which I don't recommend), here's an idea how this could be done, trying to keep the bundle size waste as low as possible:

// this will need a bunch of ts-ignores

// wherever we need to check this
const enableSpotlight = options.spotlight ?? getSpotlightFromEnv();

// ...
function getSpotlightFromEnv() {
  try {
    const processEnvVar = process.env.SENTRY_SPOTLIGHT || process.env.NEXT_PUBLIC_SENTRY_SPOTLIGHT;
    return envToBool(processEnv , { strict: true });
  } catch { /* ignore */ }
  
  try {
    // this still feels scary to add to the SDK because I don't know how bundlers other than Vite react to `import.meta` mentioned in the code. They could throw a build error. We should at least test this with webpack.
    return envToBool(import.meta.env.VITE_SENTRY_SPOTLIGHT, { strict: true });
  } catch { /* ignore */ }

  return false;
}

The most important aspects of this function are:

  • we must never throw at runtime or cause a build error
  • we must always default to false rather than true because otherwise, we risk sending stuff to spotlight in a live prod build.

Overall, I'd still recommend going with b.


const spotlight = options.spotlight ?? spotlightFromEnv;

const clientOptions: BrowserClientOptions = {
...options,
enabled: shouldDisableBecauseIsBrowserExtenstion ? false : options.enabled,
Expand All @@ -100,7 +113,18 @@ export function init(options: BrowserOptions = {}): Client | undefined {
options.defaultIntegrations == null ? getDefaultIntegrations(options) : options.defaultIntegrations,
}),
transport: options.transport || makeFetchTransport,
spotlight,
};

// Auto-add spotlight integration if enabled and not already present
if (spotlight && !clientOptions.integrations.some(({ name }) => name === SPOTLIGHT_INTEGRATION_NAME)) {
clientOptions.integrations.push(
spotlightBrowserIntegration({
sidecarUrl: typeof spotlight === 'string' ? spotlight : undefined,
}),
);
}

return initAndBind(BrowserClient, clientOptions);
}

Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ export { flushIfServerless } from './utils/flushIfServerless';
export { SDK_VERSION } from './utils/version';
export { getDebugImagesForResources, getFilenameToDebugIdMap } from './utils/debug-ids';
export { escapeStringForRegex } from './vendor/escapeStringForRegex';
export { envToBool, TRUTHY_ENV_VALUES, FALSY_ENV_VALUES } from './utils/envToBool';
export type { BoolCastOptions, StrictBoolCast, LooseBoolCast } from './utils/envToBool';

export type { Attachment } from './types-hoist/attachment';
export type {
Expand Down
2 changes: 1 addition & 1 deletion packages/node-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export { initializeEsmLoader } from './sdk/esmLoader';
export { isCjs } from './utils/detection';
export { ensureIsWrapped } from './utils/ensureIsWrapped';
export { createMissingInstrumentationContext } from './utils/createMissingInstrumentationContext';
export { envToBool } from './utils/envToBool';
export { makeNodeTransport, type NodeTransportOptions } from './transports';
export type { HTTPModuleRequestIncomingMessage } from './transports/http-module';
export { NodeClient } from './sdk/client';
Expand Down Expand Up @@ -112,6 +111,7 @@ export {
startSession,
captureSession,
endSession,
envToBool,
addIntegration,
startSpan,
startSpanManual,
Expand Down
2 changes: 1 addition & 1 deletion packages/node-core/src/sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
consoleIntegration,
consoleSandbox,
debug,
envToBool,
functionToStringIntegration,
getCurrentScope,
getIntegrationsToSetup,
Expand Down Expand Up @@ -37,7 +38,6 @@ import { systemErrorIntegration } from '../integrations/systemError';
import { makeNodeTransport } from '../transports';
import type { NodeClientOptions, NodeOptions } from '../types';
import { isCjs } from '../utils/detection';
import { envToBool } from '../utils/envToBool';
import { defaultStackParser, getSentryRelease } from './api';
import { NodeClient } from './client';
import { initializeEsmLoader } from './esmLoader';
Expand Down
Loading