ref(browser): Streamline browser init() checks (#16340)

Extracted this out of
#15307

This PR:

1. Moves things around in the `sdk.ts` file a bit, moving types & consts
to the top of the file
2. Removes an "unecessary" log around fetch not being supported: This
log has two problems. First, it is not actually logged, because we do
`logger.log()` before running `init()`, which would actually set the
logger up. So the log would never show up. Second, the log is
superfluous, if the (default) fetch transport fails to send because
fetch is not available, you get a warning anyhow for this.
3. Streamlines the browser extension check code slightly.

Closes #16284

Author: mydea

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

@@ -23,7 +23,7 @@ sentryTest(
     if (hasDebugLogs()) {
       expect(errorLogs.length).toEqual(1);
       expect(errorLogs[0]).toEqual(
-        '[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
+        '[Sentry] You cannot use Sentry.init() in a browser extension, see: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
       );
     } else {
       expect(errorLogs.length).toEqual(0);

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

@@ -21,7 +21,7 @@ sentryTest('should not initialize when inside a Chrome browser extension', async
   if (hasDebugLogs()) {
     expect(errorLogs.length).toEqual(1);
     expect(errorLogs[0]).toEqual(
-      '[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
+      '[Sentry] You cannot use Sentry.init() in a browser extension, see: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
     );
   } else {
     expect(errorLogs.length).toEqual(0);

packages/browser/src/sdk.ts

@@ -7,9 +7,7 @@ import {
   getLocationHref,
   inboundFiltersIntegration,
   initAndBind,
-  logger,
   stackParserFromStackParserOptions,
-  supportsFetch,
 } from '@sentry/core';
 import type { BrowserClientOptions, BrowserOptions } from './client';
 import { BrowserClient } from './client';
@@ -24,6 +22,22 @@ 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;
+
 /** Get the default integrations for the browser SDK. */
 export function getDefaultIntegrations(_options: Options): Integration[] {
   /**
@@ -79,49 +93,6 @@ function dropTopLevelUndefinedKeys<T extends object>(obj: T): Partial<T> {
   return mutatetedObj;
 }
 
-type ExtensionProperties = {
-  chrome?: Runtime;
-  browser?: Runtime;
-  nw?: unknown;
-};
-type Runtime = {
-  runtime?: {
-    id?: string;
-  };
-};
-
-function shouldShowBrowserExtensionError(): boolean {
-  const windowWithMaybeExtension =
-    typeof WINDOW.window !== 'undefined' && (WINDOW as typeof WINDOW & ExtensionProperties);
-  if (!windowWithMaybeExtension) {
-    // No need to show the error if we're not in a browser window environment (e.g. service workers)
-    return false;
-  }
-
-  const extensionKey = windowWithMaybeExtension.chrome ? 'chrome' : 'browser';
-  const extensionObject = windowWithMaybeExtension[extensionKey];
-
-  const runtimeId = extensionObject?.runtime?.id;
-  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 =
-    !!runtimeId && WINDOW === WINDOW.top && extensionProtocols.some(protocol => href.startsWith(`${protocol}//`));
-
-  // 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
-  const isNWjs = typeof windowWithMaybeExtension.nw !== 'undefined';
-
-  return !!runtimeId && !isDedicatedExtensionPage && !isNWjs;
-}
-
-/**
- * A magic string that build tooling can leverage in order to inject a release value into the SDK.
- */
-declare const __SENTRY_RELEASE__: string | undefined;
-
 /**
  * The Sentry Browser SDK Client.
  *
@@ -169,25 +140,11 @@ declare const __SENTRY_RELEASE__: string | undefined;
  * @see {@link BrowserOptions} for documentation on configuration options.
  */
 export function init(browserOptions: BrowserOptions = {}): Client | undefined {
-  const options = applyDefaultOptions(browserOptions);
-
-  if (!options.skipBrowserExtensionCheck && shouldShowBrowserExtensionError()) {
-    if (DEBUG_BUILD) {
-      consoleSandbox(() => {
-        // eslint-disable-next-line no-console
-        console.error(
-          '[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
-        );
-      });
-    }
+  if (!browserOptions.skipBrowserExtensionCheck && _checkForBrowserExtension()) {
     return;
   }
 
-  if (DEBUG_BUILD && !supportsFetch()) {
-    logger.warn(
-      'No Fetch API detected. The Sentry SDK requires a Fetch API compatible environment to send events. Please add a Fetch API polyfill.',
-    );
-  }
+  const options = applyDefaultOptions(browserOptions);
   const clientOptions: BrowserClientOptions = {
     ...options,
     stackParser: stackParserFromStackParserOptions(options.stackParser || defaultStackParser),
@@ -213,3 +170,48 @@ 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/test/sdk.test.ts

@@ -149,7 +149,7 @@ describe('init', () => {
 
       expect(consoleErrorSpy).toBeCalledTimes(1);
       expect(consoleErrorSpy).toHaveBeenCalledWith(
-        '[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
+        '[Sentry] You cannot use Sentry.init() in a browser extension, see: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
       );
 
       consoleErrorSpy.mockRestore();
@@ -164,7 +164,7 @@ describe('init', () => {
 
       expect(consoleErrorSpy).toBeCalledTimes(1);
       expect(consoleErrorSpy).toHaveBeenCalledWith(
-        '[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
+        '[Sentry] You cannot use Sentry.init() in a browser extension, see: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
       );
 
       consoleErrorSpy.mockRestore();

packages/core/src/index.ts

@@ -203,9 +203,11 @@ export {
   supportsDOMError,
   supportsDOMException,
   supportsErrorEvent,
+  // eslint-disable-next-line deprecation/deprecation
   supportsFetch,
   supportsHistory,
   supportsNativeFetch,
+  // eslint-disable-next-line deprecation/deprecation
   supportsReferrerPolicy,
   supportsReportingObserver,
 } from './utils-hoist/supports';

packages/core/src/utils-hoist/supports.ts

@@ -69,8 +69,11 @@ export function supportsHistory(): boolean {
  * {@link supportsFetch}.
  *
  * @returns Answer to the given question.
+ * @deprecated This is no longer used and will be removed in a future major version.
  */
-export function supportsFetch(): boolean {
+export const supportsFetch = _isFetchSupported;
+
+function _isFetchSupported(): boolean {
   if (!('fetch' in WINDOW)) {
     return false;
   }
@@ -104,7 +107,7 @@ export function supportsNativeFetch(): boolean {
     return true;
   }
 
-  if (!supportsFetch()) {
+  if (!_isFetchSupported()) {
     return false;
   }
 
@@ -153,14 +156,15 @@ export function supportsReportingObserver(): boolean {
  * {@link supportsReferrerPolicy}.
  *
  * @returns Answer to the given question.
+ * @deprecated This is no longer used and will be removed in a future major version.
  */
 export function supportsReferrerPolicy(): boolean {
   // Despite all stars in the sky saying that Edge supports old draft syntax, aka 'never', 'always', 'origin' and 'default'
   // (see https://caniuse.com/#feat=referrer-policy),
   // it doesn't. And it throws an exception instead of ignoring this parameter...
   // REF: https://github.com/getsentry/raven-js/issues/1233
 
-  if (!supportsFetch()) {
+  if (!_isFetchSupported()) {
     return false;
   }