ref(core): Remove redundant iframe check in supportsNativeFetch

packages/browser-utils/src/getNativeImplementation.ts

@@ -1,5 +1,4 @@
-import { debug, isNativeFunction } from '@sentry/core';
-import { DEBUG_BUILD } from './debug-build';
+import { getNativeImplementationFromIframe, isNativeFunction } from '@sentry/core';
 import { WINDOW } from './types';
 
 /**
@@ -32,38 +31,34 @@ export function getNativeImplementation<T extends keyof CacheableImplementations
     return cached;
   }
 
-  let impl = WINDOW[name] as CacheableImplementations[T];
+  const cacheAndReturn = (impl: CacheableImplementations[T]) => (cachedImplementations[name] = impl.bind(WINDOW));
+
+  const windowImpl = WINDOW[name] as CacheableImplementations[T];
 
   // Fast path to avoid DOM I/O
-  if (isNativeFunction(impl)) {
-    return (cachedImplementations[name] = impl.bind(WINDOW) as CacheableImplementations[T]);
+  if (isNativeFunction(windowImpl)) {
+    return cacheAndReturn(windowImpl);
   }
 
-  const document = WINDOW.document;
-  // eslint-disable-next-line deprecation/deprecation
-  if (document && typeof document.createElement === 'function') {
-    try {
-      const sandbox = document.createElement('iframe');
-      sandbox.hidden = true;
-      document.head.appendChild(sandbox);
-      const contentWindow = sandbox.contentWindow;
-      if (contentWindow?.[name]) {
-        impl = contentWindow[name] as CacheableImplementations[T];
-      }
-      document.head.removeChild(sandbox);
-    } catch (e) {
-      // Could not create sandbox iframe, just use window.xxx
-      DEBUG_BUILD && debug.warn(`Could not create sandbox iframe for ${name} check, bailing to window.${name}: `, e);
-    }
+  const iframeImpl = getNativeImplementationFromIframe(name);
+  if (iframeImpl) {
+    return cacheAndReturn(iframeImpl);
   }
 
-  // Sanity check: This _should_ not happen, but if it does, we just skip caching...
-  // This can happen e.g. in tests where fetch may not be available in the env, or similar.
-  if (!impl) {
-    return impl;
+  // This is a really weird fallback but here's what's going on:
+  // We're just being extra careful here. According to types, windowImpl is _always_ defined.
+  // However, in some very rare cases (for example test environments), it may in fact not be defined.
+  // In exactly this case, if we fail to get an iframeImpl, AND no windowImpl either,
+  // we skip caching and just return effectively undefined (despite types saying it's always defined)
+  // This basically tricks TS into thinking this function never returns `undefined` which
+  // for the most part is true.
+  if (!windowImpl) {
+    return windowImpl; // but actually return undefined
   }
 
-  return (cachedImplementations[name] = impl.bind(WINDOW) as CacheableImplementations[T]);
+  // If _only_ iframeImpl is undefined and windowImpl is defined and not not native, we end up here
+  // In this case, we deliberately cache the windowImpl.
+  return cacheAndReturn(windowImpl);
 }
 
 /** Clear a cached implementation. */

packages/browser-utils/src/index.ts

@@ -24,7 +24,7 @@ export { addClickKeypressInstrumentationHandler } from './instrument/dom';
 
 export { addHistoryInstrumentationHandler } from './instrument/history';
 
-export { fetch, setTimeout, clearCachedImplementation, getNativeImplementation } from './getNativeImplementation';
+export { fetch, setTimeout, getNativeImplementation, clearCachedImplementation } from './getNativeImplementation';
 
 export { addXhrInstrumentationHandler, SENTRY_XHR_DATA_KEY } from './instrument/xhr';
 

packages/core/src/index.ts

@@ -216,7 +216,9 @@ export {
   isSyntheticEvent,
   isThenable,
   isVueViewModel,
+  isNativeFunction,
 } from './utils/is';
+export { getNativeImplementationFromIframe } from './utils/getNativeImplementationFromIframe';
 export { isBrowser } from './utils/isBrowser';
 export { CONSOLE_LEVELS, consoleSandbox, debug, originalConsoleMethods } from './utils/debug-logger';
 export type { SentryDebugLogger } from './utils/debug-logger';
@@ -259,7 +261,6 @@ export {
 export { filenameIsInApp, node, nodeStackLineParser } from './utils/node-stack-trace';
 export { isMatchingPattern, safeJoin, snipLine, stringMatchesSomePattern, truncate } from './utils/string';
 export {
-  isNativeFunction,
   supportsDOMError,
   supportsDOMException,
   supportsErrorEvent,

packages/core/src/utils/getNativeImplementationFromIframe.ts

@@ -0,0 +1,32 @@
+import { DEBUG_BUILD } from '../debug-build';
+import { GLOBAL_OBJ } from './worldwide';
+import { debug } from './debug-logger';
+
+const WINDOW = GLOBAL_OBJ as unknown as Window;
+
+interface CacheableImplementations {
+  setTimeout: typeof WINDOW.setTimeout;
+  fetch: typeof WINDOW.fetch;
+}
+
+export function getNativeImplementationFromIframe<T extends keyof CacheableImplementations>(name: T) {
+  let impl = undefined;
+  const document = WINDOW.document;
+  // eslint-disable-next-line deprecation/deprecation
+  if (document && typeof document.createElement === 'function') {
+    try {
+      const sandbox = document.createElement('iframe');
+      sandbox.hidden = true;
+      document.head.appendChild(sandbox);
+      const contentWindow = sandbox.contentWindow;
+      if (contentWindow?.[name]) {
+        impl = contentWindow[name] as CacheableImplementations[T];
+      }
+      document.head.removeChild(sandbox);
+    } catch (e) {
+      // Could not create sandbox iframe, just use window.xxx
+      DEBUG_BUILD && debug.warn(`Could not create sandbox iframe for ${name} check, bailing to window.${name}: `, e);
+    }
+  }
+  return impl;
+}

packages/core/src/utils/is.ts

@@ -214,3 +214,11 @@ export function isVueViewModel(wat: unknown): wat is VueViewModel | VNode {
 export function isRequest(request: unknown): request is Request {
   return typeof Request !== 'undefined' && isInstanceOf(request, Request);
 }
+
+/**
+ * isNative checks if the given function is a native implementation
+ */
+// eslint-disable-next-line @typescript-eslint/ban-types
+export function isNativeFunction(func: Function): boolean {
+  return func && /^function\s+\w+\(\)\s+\{\s+\[native code\]\s+\}$/.test(func.toString());
+}

packages/core/src/utils/supports.ts

@@ -1,5 +1,5 @@
-import { DEBUG_BUILD } from '../debug-build';
-import { debug } from './debug-logger';
+import { getNativeImplementationFromIframe } from './getNativeImplementationFromIframe';
+import { isNativeFunction } from './is';
 import { GLOBAL_OBJ } from './worldwide';
 
 const WINDOW = GLOBAL_OBJ as unknown as Window;
@@ -89,14 +89,6 @@ function _isFetchSupported(): boolean {
   }
 }
 
-/**
- * isNative checks if the given function is a native implementation
- */
-// eslint-disable-next-line @typescript-eslint/ban-types
-export function isNativeFunction(func: Function): boolean {
-  return func && /^function\s+\w+\(\)\s+\{\s+\[native code\]\s+\}$/.test(func.toString());
-}
-
 /**
  * Tells whether current environment supports Fetch API natively
  * {@link supportsNativeFetch}.
@@ -118,27 +110,9 @@ export function supportsNativeFetch(): boolean {
     return true;
   }
 
-  // window.fetch is implemented, but is polyfilled or already wrapped (e.g: by a chrome extension)
-  // so create a "pure" iframe to see if that has native fetch
-  let result = false;
-  const doc = WINDOW.document;
-  // eslint-disable-next-line deprecation/deprecation
-  if (doc && typeof (doc.createElement as unknown) === 'function') {
-    try {
-      const sandbox = doc.createElement('iframe');
-      sandbox.hidden = true;
-      doc.head.appendChild(sandbox);
-      if (sandbox.contentWindow?.fetch) {
-        // eslint-disable-next-line @typescript-eslint/unbound-method
-        result = isNativeFunction(sandbox.contentWindow.fetch);
-      }
-      doc.head.removeChild(sandbox);
-    } catch (err) {
-      DEBUG_BUILD && debug.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', err);
-    }
-  }
+  const nativeImpl = getNativeImplementationFromIframe('fetch');
 
-  return result;
+  return nativeImpl ? isNativeFunction(nativeImpl) : false;
 }
 
 /**