Skip to content
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

ref: Add fast-path to fetchImpl and cleanup redundant iframe #3341

Merged
merged 2 commits into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 13 additions & 4 deletions packages/browser/src/transports/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { eventToSentryRequest, sessionToSentryRequest } from '@sentry/core';
import { Event, Response, SentryRequest, Session, TransportOptions } from '@sentry/types';
import { getGlobalObject, logger, supportsReferrerPolicy, SyncPromise } from '@sentry/utils';
import { getGlobalObject, isNativeFetch, logger, supportsReferrerPolicy, SyncPromise } from '@sentry/utils';

import { BaseTransport } from './base';

Expand Down Expand Up @@ -45,24 +45,33 @@ type FetchImpl = typeof fetch;
* Safari: resource blocked by content blocker
*/
function getNativeFetchImplementation(): FetchImpl {
// Make sure that the fetch we use is always the native one.
/* eslint-disable @typescript-eslint/unbound-method */

// Fast path to avoid DOM I/O
const global = getGlobalObject<Window>();
if (isNativeFetch(global.fetch)) {
return global.fetch.bind(global);
}

const document = global.document;
let fetchImpl = global.fetch;
// eslint-disable-next-line deprecation/deprecation
if (typeof document?.createElement === `function`) {
try {
const sandbox = document.createElement('iframe');
sandbox.hidden = true;
document.head.appendChild(sandbox);
if (sandbox.contentWindow?.fetch) {
return sandbox.contentWindow.fetch.bind(global);
fetchImpl = sandbox.contentWindow.fetch;
}
document.head.removeChild(sandbox);
} catch (e) {
logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', e);
}
}
return global.fetch.bind(global);

return fetchImpl.bind(global);
/* eslint-enable @typescript-eslint/unbound-method */
}

/** `fetch` based transport */
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/supports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function supportsFetch(): boolean {
* isNativeFetch checks if the given function is a native implementation of fetch()
*/
// eslint-disable-next-line @typescript-eslint/ban-types
function isNativeFetch(func: Function): boolean {
export function isNativeFetch(func: Function): boolean {
return func && /^function fetch\(\)\s+\{\s+\[native code\]\s+\}$/.test(func.toString());
}

Expand Down