Skip to content

Commit 772269a

Browse files
billyvgmydea
andauthored
feat(replay): Use unwrapped setTimeout to avoid e.g. angular change detection (#11924)
This PR makes sure we use the native, unwrapped `setTimeout` implementation of the browser. Some environments, e.g. Angular, monkey patch this for their change detection, leading to performance issues (possibly). We have already changed this in rrweb, but we also have some usage of this in replay itself. This PR _should_ work fine, however all test fail today because we heavily use `jest.useFakeTimers()`, which basically monkey patches fetch too. So with this change, we do not use the patched timers, leading to everything blowing up 🤯 Based on #11864 Depends on #11899 --------- Co-authored-by: Francesco Novy <[email protected]>
1 parent 88e002d commit 772269a

40 files changed

+221
-154
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
import { logger } from '@sentry/utils';
2+
import { DEBUG_BUILD } from './debug-build';
3+
import { WINDOW } from './types';
4+
5+
/**
6+
* We generally want to use window.fetch / window.setTimeout.
7+
* However, in some cases this may be wrapped (e.g. by Zone.js for Angular),
8+
* so we try to get an unpatched version of this from a sandboxed iframe.
9+
*/
10+
11+
interface CacheableImplementations {
12+
setTimeout: typeof WINDOW.setTimeout;
13+
fetch: typeof WINDOW.fetch;
14+
}
15+
16+
const cachedImplementations: Partial<CacheableImplementations> = {};
17+
18+
/**
19+
* isNative checks if the given function is a native implementation
20+
*/
21+
// eslint-disable-next-line @typescript-eslint/ban-types
22+
function isNative(func: Function): boolean {
23+
return func && /^function\s+\w+\(\)\s+\{\s+\[native code\]\s+\}$/.test(func.toString());
24+
}
25+
26+
/**
27+
* Get the native implementation of a browser function.
28+
*
29+
* This can be used to ensure we get an unwrapped version of a function, in cases where a wrapped function can lead to problems.
30+
*
31+
* The following methods can be retrieved:
32+
* - `setTimeout`: This can be wrapped by e.g. Angular, causing change detection to be triggered.
33+
* - `fetch`: This can be wrapped by e.g. ad-blockers, causing an infinite loop when a request is blocked.
34+
*/
35+
export function getNativeImplementation<T extends keyof CacheableImplementations>(
36+
name: T,
37+
): CacheableImplementations[T] {
38+
const cached = cachedImplementations[name];
39+
if (cached) {
40+
return cached;
41+
}
42+
43+
let impl = WINDOW[name] as CacheableImplementations[T];
44+
45+
// Fast path to avoid DOM I/O
46+
if (isNative(impl)) {
47+
return (cachedImplementations[name] = impl.bind(WINDOW) as CacheableImplementations[T]);
48+
}
49+
50+
const document = WINDOW.document;
51+
// eslint-disable-next-line deprecation/deprecation
52+
if (document && typeof document.createElement === 'function') {
53+
try {
54+
const sandbox = document.createElement('iframe');
55+
sandbox.hidden = true;
56+
document.head.appendChild(sandbox);
57+
const contentWindow = sandbox.contentWindow;
58+
if (contentWindow && contentWindow[name]) {
59+
impl = contentWindow[name] as CacheableImplementations[T];
60+
}
61+
document.head.removeChild(sandbox);
62+
} catch (e) {
63+
// Could not create sandbox iframe, just use window.xxx
64+
DEBUG_BUILD && logger.warn(`Could not create sandbox iframe for ${name} check, bailing to window.${name}: `, e);
65+
}
66+
}
67+
68+
// Sanity check: This _should_ not happen, but if it does, we just skip caching...
69+
// This can happen e.g. in tests where fetch may not be available in the env, or similar.
70+
if (!impl) {
71+
return impl;
72+
}
73+
74+
return (cachedImplementations[name] = impl.bind(WINDOW) as CacheableImplementations[T]);
75+
}
76+
77+
/** Clear a cached implementation. */
78+
export function clearCachedImplementation(name: keyof CacheableImplementations): void {
79+
cachedImplementations[name] = undefined;
80+
}
81+
82+
/**
83+
* A special usecase for incorrectly wrapped Fetch APIs in conjunction with ad-blockers.
84+
* Whenever someone wraps the Fetch API and returns the wrong promise chain,
85+
* this chain becomes orphaned and there is no possible way to capture it's rejections
86+
* other than allowing it bubble up to this very handler. eg.
87+
*
88+
* const f = window.fetch;
89+
* window.fetch = function () {
90+
* const p = f.apply(this, arguments);
91+
*
92+
* p.then(function() {
93+
* console.log('hi.');
94+
* });
95+
*
96+
* return p;
97+
* }
98+
*
99+
* `p.then(function () { ... })` is producing a completely separate promise chain,
100+
* however, what's returned is `p` - the result of original `fetch` call.
101+
*
102+
* This mean, that whenever we use the Fetch API to send our own requests, _and_
103+
* some ad-blocker blocks it, this orphaned chain will _always_ reject,
104+
* effectively causing another event to be captured.
105+
* This makes a whole process become an infinite loop, which we need to somehow
106+
* deal with, and break it in one way or another.
107+
*
108+
* To deal with this issue, we are making sure that we _always_ use the real
109+
* browser Fetch API, instead of relying on what `window.fetch` exposes.
110+
* The only downside to this would be missing our own requests as breadcrumbs,
111+
* but because we are already not doing this, it should be just fine.
112+
*
113+
* Possible failed fetch error messages per-browser:
114+
*
115+
* Chrome: Failed to fetch
116+
* Edge: Failed to Fetch
117+
* Firefox: NetworkError when attempting to fetch resource
118+
* Safari: resource blocked by content blocker
119+
*/
120+
export function fetch(...rest: Parameters<typeof WINDOW.fetch>): ReturnType<typeof WINDOW.fetch> {
121+
return getNativeImplementation('fetch')(...rest);
122+
}
123+
124+
/**
125+
* Get an unwrapped `setTimeout` method.
126+
* This ensures that even if e.g. Angular wraps `setTimeout`, we get the native implementation,
127+
* avoiding triggering change detection.
128+
*/
129+
export function setTimeout(...rest: Parameters<typeof WINDOW.setTimeout>): ReturnType<typeof WINDOW.setTimeout> {
130+
return getNativeImplementation('setTimeout')(...rest);
131+
}

packages/browser-utils/src/index.ts

+3-4
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ export { addClickKeypressInstrumentationHandler } from './instrument/dom';
1818

1919
export { addHistoryInstrumentationHandler } from './instrument/history';
2020

21-
export {
22-
addXhrInstrumentationHandler,
23-
SENTRY_XHR_DATA_KEY,
24-
} from './instrument/xhr';
21+
export { fetch, setTimeout, clearCachedImplementation, getNativeImplementation } from './getNativeImplementation';
22+
23+
export { addXhrInstrumentationHandler, SENTRY_XHR_DATA_KEY } from './instrument/xhr';

packages/browser-utils/src/instrument/dom.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { HandlerDataDom } from '@sentry/types';
22

33
import { addHandler, addNonEnumerableProperty, fill, maybeInstrument, triggerHandlers, uuid4 } from '@sentry/utils';
4-
import { WINDOW } from '../metrics/types';
4+
import { WINDOW } from '../types';
55

66
type SentryWrappedTarget = HTMLElement & { _sentryId?: string };
77

packages/browser-utils/src/instrument/history.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { HandlerDataHistory } from '@sentry/types';
22
import { addHandler, fill, maybeInstrument, supportsHistory, triggerHandlers } from '@sentry/utils';
3-
import { WINDOW } from '../metrics/types';
3+
import { WINDOW } from '../types';
44

55
let lastHref: string | undefined;
66

packages/browser-utils/src/instrument/xhr.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, WrappedFunction } from '@sentry/types';
22

33
import { addHandler, fill, isString, maybeInstrument, timestampInSeconds, triggerHandlers } from '@sentry/utils';
4-
import { WINDOW } from '../metrics/types';
4+
import { WINDOW } from '../types';
55

66
export const SENTRY_XHR_DATA_KEY = '__sentry_xhr_v3__';
77

packages/browser-utils/src/metrics/browserMetrics.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ import { browserPerformanceTimeOrigin, getComponentName, htmlTreeAsString, logge
66

77
import { spanToJSON } from '@sentry/core';
88
import { DEBUG_BUILD } from '../debug-build';
9+
import { WINDOW } from '../types';
910
import {
1011
addClsInstrumentationHandler,
1112
addFidInstrumentationHandler,
1213
addLcpInstrumentationHandler,
1314
addPerformanceInstrumentationHandler,
1415
addTtfbInstrumentationHandler,
1516
} from './instrument';
16-
import { WINDOW } from './types';
1717
import { getBrowserPerformanceAPI, isMeasurementValue, msToSec, startAndEndSpan } from './utils';
1818
import { getNavigationEntry } from './web-vitals/lib/getNavigationEntry';
1919
import { getVisibilityWatcher } from './web-vitals/lib/getVisibilityWatcher';

packages/browser-utils/src/metrics/utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { SentrySpan } from '@sentry/core';
22
import { spanToJSON, startInactiveSpan, withActiveSpan } from '@sentry/core';
33
import type { Span, SpanTimeInput, StartSpanOptions } from '@sentry/types';
4-
import { WINDOW } from './types';
4+
import { WINDOW } from '../types';
55

66
/**
77
* Checks if a given value is a valid measurement value.

packages/browser-utils/src/metrics/web-vitals/getINP.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { WINDOW } from '../types';
17+
import { WINDOW } from '../../types';
1818
import { bindReporter } from './lib/bindReporter';
1919
import { initMetric } from './lib/initMetric';
2020
import { observe } from './lib/observe';

packages/browser-utils/src/metrics/web-vitals/getLCP.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { WINDOW } from '../types';
17+
import { WINDOW } from '../../types';
1818
import { bindReporter } from './lib/bindReporter';
1919
import { getActivationStart } from './lib/getActivationStart';
2020
import { getVisibilityWatcher } from './lib/getVisibilityWatcher';

packages/browser-utils/src/metrics/web-vitals/lib/getNavigationEntry.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { WINDOW } from '../../types';
17+
import { WINDOW } from '../../../types';
1818
import type { NavigationTimingPolyfillEntry } from '../types';
1919

2020
export const getNavigationEntry = (): PerformanceNavigationTiming | NavigationTimingPolyfillEntry | undefined => {

packages/browser-utils/src/metrics/web-vitals/lib/getVisibilityWatcher.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { WINDOW } from '../../types';
17+
import { WINDOW } from '../../../types';
1818

1919
let firstHiddenTime = -1;
2020

packages/browser-utils/src/metrics/web-vitals/lib/initMetric.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { WINDOW } from '../../types';
17+
import { WINDOW } from '../../../types';
1818
import type { MetricType } from '../types';
1919
import { generateUniqueID } from './generateUniqueID';
2020
import { getActivationStart } from './getActivationStart';

packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { WINDOW } from '../../types';
17+
import { WINDOW } from '../../../types';
1818

1919
export interface OnHiddenCallback {
2020
(event: Event): void;

packages/browser-utils/src/metrics/web-vitals/lib/whenActivated.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { WINDOW } from '../../types';
17+
import { WINDOW } from '../../../types';
1818

1919
export const whenActivated = (callback: () => void) => {
2020
if (WINDOW.document && WINDOW.document.prerendering) {

packages/browser-utils/src/metrics/web-vitals/onTTFB.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { WINDOW } from '../types';
17+
import { WINDOW } from '../../types';
1818
import { bindReporter } from './lib/bindReporter';
1919
import { getActivationStart } from './lib/getActivationStart';
2020
import { getNavigationEntry } from './lib/getNavigationEntry';

packages/browser-utils/test/browser/browserMetrics.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
import type { Span } from '@sentry/types';
1212
import type { ResourceEntry } from '../../src/metrics/browserMetrics';
1313
import { _addMeasureSpans, _addResourceSpans } from '../../src/metrics/browserMetrics';
14-
import { WINDOW } from '../../src/metrics/types';
14+
import { WINDOW } from '../../src/types';
1515
import { TestClient, getDefaultClientOptions } from '../utils/TestClient';
1616

1717
const mockWindowLocation = {

packages/browser/.eslintrc.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@ module.exports = {
22
env: {
33
browser: true,
44
},
5-
ignorePatterns: ['test/integration/**', 'src/loader.js'],
5+
ignorePatterns: ['test/integration/**', 'test/loader.js'],
66
extends: ['../../.eslintrc.js'],
77
};

packages/browser/src/transports/fetch.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1+
import { clearCachedImplementation, getNativeImplementation } from '@sentry-internal/browser-utils';
12
import { createTransport } from '@sentry/core';
23
import type { Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';
34
import { rejectedSyncPromise } from '@sentry/utils';
5+
import type { WINDOW } from '../helpers';
46

57
import type { BrowserTransportOptions } from './types';
6-
import type { FetchImpl } from './utils';
7-
import { clearCachedFetchImplementation, getNativeFetchImplementation } from './utils';
88

99
/**
1010
* Creates a Transport that uses the Fetch API to send events to Sentry.
1111
*/
1212
export function makeFetchTransport(
1313
options: BrowserTransportOptions,
14-
nativeFetch: FetchImpl | undefined = getNativeFetchImplementation(),
14+
nativeFetch: typeof WINDOW.fetch | undefined = getNativeImplementation('fetch'),
1515
): Transport {
1616
let pendingBodySize = 0;
1717
let pendingCount = 0;
@@ -42,7 +42,7 @@ export function makeFetchTransport(
4242
};
4343

4444
if (!nativeFetch) {
45-
clearCachedFetchImplementation();
45+
clearCachedImplementation('fetch');
4646
return rejectedSyncPromise('No fetch implementation available');
4747
}
4848

@@ -59,7 +59,7 @@ export function makeFetchTransport(
5959
};
6060
});
6161
} catch (e) {
62-
clearCachedFetchImplementation();
62+
clearCachedImplementation('fetch');
6363
pendingBodySize -= requestSize;
6464
pendingCount--;
6565
return rejectedSyncPromise(e);

0 commit comments

Comments
 (0)