From 69e96ba34c112ff15715521b281c394572be6e5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Tue, 9 Mar 2021 17:20:24 +0100 Subject: [PATCH] fix: Prevent fetch errors loops with invalid fetch implementations --- packages/browser/src/transports/fetch.ts | 78 +++++++++++++++++-- .../test/unit/transports/fetch.test.ts | 28 ++++--- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index 64d6e18fda3f..c16ab3a4a588 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -1,13 +1,82 @@ import { eventToSentryRequest, sessionToSentryRequest } from '@sentry/core'; -import { Event, Response, SentryRequest, Session } from '@sentry/types'; -import { getGlobalObject, supportsReferrerPolicy, SyncPromise } from '@sentry/utils'; +import { Event, Response, SentryRequest, Session, TransportOptions } from '@sentry/types'; +import { getGlobalObject, logger, supportsReferrerPolicy, SyncPromise } from '@sentry/utils'; import { BaseTransport } from './base'; -const global = getGlobalObject(); +type FetchImpl = typeof fetch; + +/** + * A special usecase for incorrectly wrapped Fetch APIs in conjunction with ad-blockers. + * Whenever someone wraps the Fetch API and returns the wrong promise chain, + * this chain becomes orphaned and there is no possible way to capture it's rejections + * other than allowing it bubble up to this very handler. eg. + * + * const f = window.fetch; + * window.fetch = function () { + * const p = f.apply(this, arguments); + * + * p.then(function() { + * console.log('hi.'); + * }); + * + * return p; + * } + * + * `p.then(function () { ... })` is producing a completely separate promise chain, + * however, what's returned is `p` - the result of original `fetch` call. + * + * This mean, that whenever we use the Fetch API to send our own requests, _and_ + * some ad-blocker blocks it, this orphaned chain will _always_ reject, + * effectively causing another event to be captured. + * This makes a whole process become an infinite loop, which we need to somehow + * deal with, and break it in one way or another. + * + * To deal with this issue, we are making sure that we _always_ use the real + * browser Fetch API, instead of relying on what `window.fetch` exposes. + * The only downside to this would be missing our own requests as breadcrumbs, + * but because we are already not doing this, it should be just fine. + * + * Possible failed fetch error messages per-browser: + * + * Chrome: Failed to fetch + * Edge: Failed to Fetch + * Firefox: NetworkError when attempting to fetch resource + * Safari: resource blocked by content blocker + */ +function getNativeFetchImplementation(): FetchImpl { + // Make sure that the fetch we use is always the native one. + const global = getGlobalObject(); + const document = global.document; + // 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); + } + 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); +} /** `fetch` based transport */ export class FetchTransport extends BaseTransport { + /** + * Fetch API reference which always points to native browser implementation. + */ + private _fetch: typeof fetch; + + constructor(options: TransportOptions, fetchImpl: FetchImpl = getNativeFetchImplementation()) { + super(options); + this._fetch = fetchImpl; + } + /** * @inheritDoc */ @@ -54,8 +123,7 @@ export class FetchTransport extends BaseTransport { return this._buffer.add( new SyncPromise((resolve, reject) => { - global - .fetch(sentryRequest.url, options) + this._fetch(sentryRequest.url, options) .then(response => { const headers = { 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index 2442e3f13618..29c8c4eea337 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -19,7 +19,7 @@ let transport: Transports.BaseTransport; describe('FetchTransport', () => { beforeEach(() => { fetch = (stub(window, 'fetch') as unknown) as SinonStub; - transport = new Transports.FetchTransport({ dsn: testDsn }); + transport = new Transports.FetchTransport({ dsn: testDsn }, window.fetch); }); afterEach(() => { @@ -83,12 +83,15 @@ describe('FetchTransport', () => { }); it('passes in headers', async () => { - transport = new Transports.FetchTransport({ - dsn: testDsn, - headers: { - Authorization: 'Basic GVzdDp0ZXN0Cg==', + transport = new Transports.FetchTransport( + { + dsn: testDsn, + headers: { + Authorization: 'Basic GVzdDp0ZXN0Cg==', + }, }, - }); + window.fetch, + ); const response = { status: 200, headers: new Headers() }; fetch.returns(Promise.resolve(response)); @@ -109,12 +112,15 @@ describe('FetchTransport', () => { }); it('passes in fetch parameters', async () => { - transport = new Transports.FetchTransport({ - dsn: testDsn, - fetchParameters: { - credentials: 'include', + transport = new Transports.FetchTransport( + { + dsn: testDsn, + fetchParameters: { + credentials: 'include', + }, }, - }); + window.fetch, + ); const response = { status: 200, headers: new Headers() }; fetch.returns(Promise.resolve(response));