feat(browser): Disable client when browser extension is detected in init() (#16354)

Similar to #16353,
this changes how default options for BrowserClient are handled. Instead
of building this in `init`, we now do (some) of it in BrowserClient.

Additionally, this also adjusts what we do if we detect a browser
extension: Instead of skipping the setup, we now just disable the
client. This streamlines this a bit and also ensures that we actually
always return a `Client` from init.

It also fixes the type for `BrowserOption` to actually allow to
configure `cdnBaseUrl`, which was actually only set for the
ClientOptions, oops.

The reason for this is to streamline the `init` code, making it easier
to extend/adjust it. Right now, there is a lot going on in different
places there. By moving as much as we can (and makes sense) to
`BrowserClient` this becomes a bit easier. While playing with different
ways to handle the browser extension stuff, I ended up landing on just
disabling the SDK in this scenario.

Author: mydea

dev-packages/browser-integration-tests/suites/manual-client/skip-init-browser-extension/test.ts

@@ -18,7 +18,12 @@ sentryTest(
       return !!(window as any).Sentry.isInitialized();
     });
 
-    expect(isInitialized).toEqual(false);
+    const isEnabled = await page.evaluate(() => {
+      return !!(window as any).Sentry.getClient()?.getOptions().enabled;
+    });
+
+    expect(isInitialized).toEqual(true);
+    expect(isEnabled).toEqual(false);
 
     if (hasDebugLogs()) {
       expect(errorLogs.length).toEqual(1);

dev-packages/browser-integration-tests/suites/manual-client/skip-init-chrome-extension/test.ts

@@ -16,7 +16,12 @@ sentryTest('should not initialize when inside a Chrome browser extension', async
     return !!(window as any).Sentry.isInitialized();
   });
 
-  expect(isInitialized).toEqual(false);
+  const isEnabled = await page.evaluate(() => {
+    return !!(window as any).Sentry.getClient()?.getOptions().enabled;
+  });
+
+  expect(isInitialized).toEqual(true);
+  expect(isEnabled).toEqual(false);
 
   if (hasDebugLogs()) {
     expect(errorLogs.length).toEqual(1);

packages/browser/src/client.ts

@@ -21,15 +21,24 @@ import { eventFromException, eventFromMessage } from './eventbuilder';
 import { WINDOW } from './helpers';
 import type { BrowserTransportOptions } from './transports/types';
 
+/**
+ * A magic string that build tooling can leverage in order to inject a release value into the SDK.
+ */
+declare const __SENTRY_RELEASE__: string | undefined;
+
 const DEFAULT_FLUSH_INTERVAL = 5000;
 
+type BrowserSpecificOptions = BrowserClientReplayOptions &
+  BrowserClientProfilingOptions & {
+    /** If configured, this URL will be used as base URL for lazy loading integration. */
+    cdnBaseUrl?: string;
+  };
 /**
  * Configuration options for the Sentry Browser SDK.
  * @see @sentry/core Options for more information.
  */
 export type BrowserOptions = Options<BrowserTransportOptions> &
-  BrowserClientReplayOptions &
-  BrowserClientProfilingOptions & {
+  BrowserSpecificOptions & {
     /**
      * Important: Only set this option if you know what you are doing!
      *
@@ -54,12 +63,7 @@ export type BrowserOptions = Options<BrowserTransportOptions> &
  * Configuration options for the Sentry Browser SDK Client class
  * @see BrowserClient for more information.
  */
-export type BrowserClientOptions = ClientOptions<BrowserTransportOptions> &
-  BrowserClientReplayOptions &
-  BrowserClientProfilingOptions & {
-    /** If configured, this URL will be used as base URL for lazy loading integration. */
-    cdnBaseUrl?: string;
-  };
+export type BrowserClientOptions = ClientOptions<BrowserTransportOptions> & BrowserSpecificOptions;
 
 /**
  * The Sentry Browser SDK Client.
@@ -75,11 +79,7 @@ export class BrowserClient extends Client<BrowserClientOptions> {
    * @param options Configuration options for this SDK.
    */
   public constructor(options: BrowserClientOptions) {
-    const opts = {
-      // We default this to true, as it is the safer scenario
-      parentSpanIsAlwaysRootSpan: true,
-      ...options,
-    };
+    const opts = applyDefaultOptions(options);
     const sdkSource = WINDOW.SENTRY_SDK_SOURCE || getSDKSource();
     applySdkMetadata(opts, 'browser', ['browser'], sdkSource);
 
@@ -155,3 +155,17 @@ export class BrowserClient extends Client<BrowserClientOptions> {
     return super._prepareEvent(event, hint, currentScope, isolationScope);
   }
 }
+
+/** Exported only for tests. */
+export function applyDefaultOptions<T extends Partial<BrowserClientOptions>>(optionsArg: T): T {
+  return {
+    release:
+      typeof __SENTRY_RELEASE__ === 'string' // This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value
+        ? __SENTRY_RELEASE__
+        : WINDOW.SENTRY_RELEASE?.id, // This supports the variable that sentry-webpack-plugin injects
+    sendClientReports: true,
+    // We default this to true, as it is the safer scenario
+    parentSpanIsAlwaysRootSpan: true,
+    ...optionsArg,
+  };
+}

packages/browser/src/sdk.ts

@@ -1,18 +1,14 @@
 import type { Client, Integration, Options } from '@sentry/core';
 import {
-  consoleSandbox,
   dedupeIntegration,
   functionToStringIntegration,
   getIntegrationsToSetup,
-  getLocationHref,
   inboundFiltersIntegration,
   initAndBind,
   stackParserFromStackParserOptions,
 } from '@sentry/core';
 import type { BrowserClientOptions, BrowserOptions } from './client';
 import { BrowserClient } from './client';
-import { DEBUG_BUILD } from './debug-build';
-import { WINDOW } from './helpers';
 import { breadcrumbsIntegration } from './integrations/breadcrumbs';
 import { browserApiErrorsIntegration } from './integrations/browserapierrors';
 import { browserSessionIntegration } from './integrations/browsersession';
@@ -21,22 +17,7 @@ import { httpContextIntegration } from './integrations/httpcontext';
 import { linkedErrorsIntegration } from './integrations/linkederrors';
 import { defaultStackParser } from './stack-parsers';
 import { makeFetchTransport } from './transports/fetch';
-
-type ExtensionProperties = {
-  chrome?: Runtime;
-  browser?: Runtime;
-  nw?: unknown;
-};
-type Runtime = {
-  runtime?: {
-    id?: string;
-  };
-};
-
-/**
- * A magic string that build tooling can leverage in order to inject a release value into the SDK.
- */
-declare const __SENTRY_RELEASE__: string | undefined;
+import { checkAndWarnIfIsEmbeddedBrowserExtension } from './utils/detectBrowserExtension';
 
 /** Get the default integrations for the browser SDK. */
 export function getDefaultIntegrations(_options: Options): Integration[] {
@@ -59,40 +40,6 @@ export function getDefaultIntegrations(_options: Options): Integration[] {
   ];
 }
 
-/** Exported only for tests. */
-export function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions {
-  const defaultOptions: BrowserOptions = {
-    defaultIntegrations: getDefaultIntegrations(optionsArg),
-    release:
-      typeof __SENTRY_RELEASE__ === 'string' // This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value
-        ? __SENTRY_RELEASE__
-        : WINDOW.SENTRY_RELEASE?.id, // This supports the variable that sentry-webpack-plugin injects
-    sendClientReports: true,
-  };
-
-  return {
-    ...defaultOptions,
-    ...dropTopLevelUndefinedKeys(optionsArg),
-  };
-}
-
-/**
- * In contrast to the regular `dropUndefinedKeys` method,
- * this one does not deep-drop keys, but only on the top level.
- */
-function dropTopLevelUndefinedKeys<T extends object>(obj: T): Partial<T> {
-  const mutatetedObj: Partial<T> = {};
-
-  for (const k of Object.getOwnPropertyNames(obj)) {
-    const key = k as keyof T;
-    if (obj[key] !== undefined) {
-      mutatetedObj[key] = obj[key];
-    }
-  }
-
-  return mutatetedObj;
-}
-
 /**
  * The Sentry Browser SDK Client.
  *
@@ -139,19 +86,21 @@ function dropTopLevelUndefinedKeys<T extends object>(obj: T): Partial<T> {
  *
  * @see {@link BrowserOptions} for documentation on configuration options.
  */
-export function init(browserOptions: BrowserOptions = {}): Client | undefined {
-  if (!browserOptions.skipBrowserExtensionCheck && _checkForBrowserExtension()) {
-    return;
-  }
+export function init(options: BrowserOptions = {}): Client | undefined {
+  const shouldDisableBecauseIsBrowserExtenstion =
+    !options.skipBrowserExtensionCheck && checkAndWarnIfIsEmbeddedBrowserExtension();
 
-  const options = applyDefaultOptions(browserOptions);
   const clientOptions: BrowserClientOptions = {
     ...options,
+    enabled: shouldDisableBecauseIsBrowserExtenstion ? false : options.enabled,
     stackParser: stackParserFromStackParserOptions(options.stackParser || defaultStackParser),
-    integrations: getIntegrationsToSetup(options),
+    integrations: getIntegrationsToSetup({
+      integrations: options.integrations,
+      defaultIntegrations:
+        options.defaultIntegrations == null ? getDefaultIntegrations(options) : options.defaultIntegrations,
+    }),
     transport: options.transport || makeFetchTransport,
   };
-
   return initAndBind(BrowserClient, clientOptions);
 }
 
@@ -170,48 +119,3 @@ export function forceLoad(): void {
 export function onLoad(callback: () => void): void {
   callback();
 }
-
-function _isEmbeddedBrowserExtension(): boolean {
-  if (typeof WINDOW.window === 'undefined') {
-    // No need to show the error if we're not in a browser window environment (e.g. service workers)
-    return false;
-  }
-
-  const _window = WINDOW as typeof WINDOW & ExtensionProperties;
-
-  // Running the SDK in NW.js, which appears like a browser extension but isn't, is also fine
-  // see: https://github.com/getsentry/sentry-javascript/issues/12668
-  if (_window.nw) {
-    return false;
-  }
-
-  const extensionObject = _window['chrome'] || _window['browser'];
-
-  if (!extensionObject?.runtime?.id) {
-    return false;
-  }
-
-  const href = getLocationHref();
-  const extensionProtocols = ['chrome-extension', 'moz-extension', 'ms-browser-extension', 'safari-web-extension'];
-
-  // Running the SDK in a dedicated extension page and calling Sentry.init is fine; no risk of data leakage
-  const isDedicatedExtensionPage =
-    WINDOW === WINDOW.top && extensionProtocols.some(protocol => href.startsWith(`${protocol}://`));
-
-  return !isDedicatedExtensionPage;
-}
-
-function _checkForBrowserExtension(): true | void {
-  if (_isEmbeddedBrowserExtension()) {
-    if (DEBUG_BUILD) {
-      consoleSandbox(() => {
-        // eslint-disable-next-line no-console
-        console.error(
-          '[Sentry] You cannot use Sentry.init() in a browser extension, see: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
-        );
-      });
-    }
-
-    return true;
-  }
-}

packages/browser/src/utils/detectBrowserExtension.ts

@@ -0,0 +1,65 @@
+import { consoleSandbox, getLocationHref } from '@sentry/core';
+import { DEBUG_BUILD } from '../debug-build';
+import { WINDOW } from '../helpers';
+
+type ExtensionRuntime = {
+  runtime?: {
+    id?: string;
+  };
+};
+type ExtensionProperties = {
+  chrome?: ExtensionRuntime;
+  browser?: ExtensionRuntime;
+  nw?: unknown;
+};
+
+/**
+ * Returns true if the SDK is running in an embedded browser extension.
+ * Stand-alone browser extensions (which do not share the same data as the main browser page) are fine.
+ */
+export function checkAndWarnIfIsEmbeddedBrowserExtension(): boolean {
+  if (_isEmbeddedBrowserExtension()) {
+    if (DEBUG_BUILD) {
+      consoleSandbox(() => {
+        // eslint-disable-next-line no-console
+        console.error(
+          '[Sentry] You cannot use Sentry.init() in a browser extension, see: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
+        );
+      });
+    }
+
+    return true;
+  }
+
+  return false;
+}
+
+function _isEmbeddedBrowserExtension(): boolean {
+  if (typeof WINDOW.window === 'undefined') {
+    // No need to show the error if we're not in a browser window environment (e.g. service workers)
+    return false;
+  }
+
+  const _window = WINDOW as typeof WINDOW & ExtensionProperties;
+
+  // Running the SDK in NW.js, which appears like a browser extension but isn't, is also fine
+  // see: https://github.com/getsentry/sentry-javascript/issues/12668
+  if (_window.nw) {
+    return false;
+  }
+
+  const extensionObject = _window['chrome'] || _window['browser'];
+
+  if (!extensionObject?.runtime?.id) {
+    return false;
+  }
+
+  const href = getLocationHref();
+  const extensionProtocols = ['chrome-extension', 'moz-extension', 'ms-browser-extension', 'safari-web-extension'];
+
+  // Running the SDK in a dedicated extension page and calling Sentry.init is fine; no risk of data leakage
+  const isDedicatedExtensionPage =
+    WINDOW === WINDOW.top && extensionProtocols.some(protocol => href.startsWith(`${protocol}://`));
+
+  return !isDedicatedExtensionPage;
+}