Skip to content

Commit d50bd8d

Browse files
authored
Merge pull request Expensify#103459 from software-mansion-labs/google-translate-sentry-filter
[NoQA] Drop Google Translate stack-overflow noise from Sentry
2 parents 4475dcd + d1317ae commit d50bd8d

10 files changed

Lines changed: 255 additions & 36 deletions

‎src/CONST/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2370,6 +2370,8 @@ const CONST = {
23702370
MFA_SCENARIO: 'mfa_scenario',
23712371
MFA_ERROR_REASON: 'mfa_error_reason',
23722372
BUILD_TYPE: 'build_type',
2373+
/** Written by Sentry's `thirdPartyErrorFilterIntegration` on events whose every frame is foreign to our bundle, read by the noise filters. */
2374+
THIRD_PARTY_CODE: 'third_party_code',
23732375
},
23742376
EXPENSE_ERROR_TYPE: {
23752377
REPORT_CREATION_FAILED: 'report_creation_failed',

‎src/libs/telemetry/integrations/classCallCheckNoiseFilter.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1+
import CONST from '@src/CONST';
2+
13
import type {Event, Integration} from '@sentry/core';
24

35
/** Message Babel's `_classCallCheck` throws when a transpiled class is called without `new`, from inside the class, never the caller. */
46
const CLASS_CALL_CHECK_MESSAGE = 'Cannot call a class as a function';
57

6-
/** Tag `thirdPartyErrorFilterIntegration` sets on events whose every frame is foreign to our bundle. */
7-
const THIRD_PARTY_CODE_TAG = 'third_party_code';
8-
98
/**
109
* Filenames that mean "this frame has no URL".
1110
*
@@ -39,7 +38,7 @@ const ANONYMOUS_FILENAMES = new Set(['app:///', '', '[native code]', 'native', '
3938
* (APP-J2J is a genuine Convert-experiment bug).
4039
*/
4140
function isClassCallCheckNoise(event: Event): boolean {
42-
if (event.tags?.[THIRD_PARTY_CODE_TAG] !== true) {
41+
if (event.tags?.[CONST.TELEMETRY.TAGS.THIRD_PARTY_CODE] !== true) {
4342
return false;
4443
}
4544

@@ -69,4 +68,4 @@ const classCallCheckNoiseFilterIntegration: Integration = {
6968
};
7069

7170
export default classCallCheckNoiseFilterIntegration;
72-
export {isClassCallCheckNoise, CLASS_CALL_CHECK_MESSAGE, THIRD_PARTY_CODE_TAG};
71+
export {isClassCallCheckNoise, CLASS_CALL_CHECK_MESSAGE};
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import CONST from '@src/CONST';
2+
3+
import type {Event, Integration, StackFrame} from '@sentry/core';
4+
5+
/** WebKit ends the message with a period, V8 does not. */
6+
const STACK_OVERFLOW_MESSAGE = /^Maximum call stack size exceeded\.?$/;
7+
8+
/**
9+
* `app:///` plus a basename with no dot, after `createReactNativeRewriteFrames`: the page URL (`app:///home`,
10+
* `app:///`) or a dot-less script URL such as Translate's own `app:///m=el_main`. Our chunks all have a dot.
11+
*/
12+
const PAGE_URL_FILENAME = /^app:\/\/\/[^./<]*$/;
13+
14+
/** Synthesized by the SDK for an `onerror` event with no stack (APP-ZF), which cannot be told apart from a real error. */
15+
const SYNTHESIZED_FILENAME = 'app:///undefined';
16+
17+
/**
18+
* A wasm frame compiled without a URL has no line number and, after the rewrite, a dot-less filename such as
19+
* `app:///00b2a5aa:wasm-function[38]:0x1b6c`. Translate frames always carry a line, so requiring one keeps a stack
20+
* overflow in our own wasm code.
21+
*/
22+
function isPageUrlFrame({filename = '', lineno}: StackFrame): boolean {
23+
return lineno !== undefined && PAGE_URL_FILENAME.test(filename) && filename !== SYNTHESIZED_FILENAME;
24+
}
25+
26+
/**
27+
* True for the Google Translate stack overflow from https://github.com/Expensify/App/issues/102044: Chrome (iOS
28+
* and desktop) injects Translate twice and two of its functions call each other until the stack overflows.
29+
* Requires a third-party tag, a single stack-overflow `RangeError`, and only dot-less frames with a line number.
30+
*
31+
* Wider than Translate on purpose: it also drops a stack overflow from any inline script, since inline scripts
32+
* report under the page URL. That covers the Ketch and GTM loaders in `web/index.html` and any GTM Custom HTML
33+
* tag. None of them is App code, so there is nothing to fix on our side. The line of the loop is not checked,
34+
* because Translate builds move it (226 in APP-M6P, 228 in APP-KFE).
35+
*/
36+
function isGoogleTranslateRecursionNoise(event: Event): boolean {
37+
if (event.tags?.[CONST.TELEMETRY.TAGS.THIRD_PARTY_CODE] !== true) {
38+
return false;
39+
}
40+
41+
const values = event.exception?.values ?? [];
42+
if (values.length !== 1) {
43+
return false;
44+
}
45+
46+
const [exception] = values;
47+
if (exception.type !== 'RangeError' || !STACK_OVERFLOW_MESSAGE.test(exception.value ?? '')) {
48+
return false;
49+
}
50+
51+
const frames = exception.stacktrace?.frames ?? [];
52+
return frames.length > 0 && frames.every(isPageUrlFrame);
53+
}
54+
55+
/** Must run after `thirdPartyErrorFilterIntegration`, which writes the tag this reads. */
56+
const googleTranslateRecursionNoiseFilterIntegration: Integration = {
57+
name: 'GoogleTranslateRecursionNoiseFilter',
58+
processEvent: (event) => (isGoogleTranslateRecursionNoise(event) ? null : event),
59+
};
60+
61+
export default googleTranslateRecursionNoiseFilterIntegration;
62+
export {isGoogleTranslateRecursionNoise};

‎src/libs/telemetry/integrations/index.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ const thirdPartyErrorFilterIntegration = undefined;
1818
// reads a tag only the web integration above sets. Stub for export shape parity; filtered out of the list here.
1919
const classCallCheckNoiseFilterIntegration = undefined;
2020

21+
// Web-only, stub for export shape parity.
22+
const googleTranslateRecursionNoiseFilterIntegration = undefined;
23+
2124
export {
2225
navigationIntegration,
2326
tracingIntegration,
@@ -27,4 +30,5 @@ export {
2730
reportingObserverIntegration,
2831
thirdPartyErrorFilterIntegration,
2932
classCallCheckNoiseFilterIntegration,
33+
googleTranslateRecursionNoiseFilterIntegration,
3034
};

‎src/libs/telemetry/integrations/index.web.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import * as SentryReact from '@sentry/react';
44

55
import classCallCheckNoiseFilterIntegration from './classCallCheckNoiseFilter';
66
import {breadcrumbsIntegration, browserProfilingIntegration, consoleIntegration, navigationIntegration, shouldCreateSpanForRequest} from './common';
7+
import googleTranslateRecursionNoiseFilterIntegration from './googleTranslateRecursionNoiseFilter';
78

89
/**
910
* `typeof` guard rather than a bare read: the define is absent from bundles that do not go through
@@ -35,9 +36,9 @@ const reportingObserverIntegration = SentryReact.reportingObserverIntegration({
3536
});
3637

3738
/**
38-
* Tags errors whose stack holds no frame from our own bundle with `third_party_code: true`, so noise
39-
* thrown by injected code (consent tools, tag managers, browser extensions) can be told apart from our
40-
* own errors in issue search (GH #93837).
39+
* Tags errors whose stack holds no frame from our own bundle with `third_party_code: true`
40+
* (`CONST.TELEMETRY.TAGS.THIRD_PARTY_CODE`), so noise thrown by injected code (consent tools, tag managers,
41+
* browser extensions) can be told apart from our own errors in issue search (GH #93837).
4142
*
4243
* Our bundle is recognized by the application key `@sentry/webpack-plugin` embeds in every chunk.
4344
* Without a stamped key every frame looks foreign and *all* our errors get mislabeled, so the guard
@@ -59,4 +60,5 @@ export {
5960
reportingObserverIntegration,
6061
thirdPartyErrorFilterIntegration,
6162
classCallCheckNoiseFilterIntegration,
63+
googleTranslateRecursionNoiseFilterIntegration,
6264
};

‎src/setup/telemetry/setupSentry.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
browserProfilingIntegration,
55
classCallCheckNoiseFilterIntegration,
66
consoleIntegration,
7+
googleTranslateRecursionNoiseFilterIntegration,
78
navigationIntegration,
89
reportingObserverIntegration,
910
thirdPartyErrorFilterIntegration,
@@ -27,12 +28,12 @@ import makeDebugTransport from './debugTransport';
2728
const EXTENSION_DENY_URLS = [/^chrome-extension:\/\//i, /^moz-extension:\/\//i, /^safari-extension:\/\//i, /^safari-web-extension:\/\//i];
2829

2930
/**
30-
* Ordered pair, not two independent entries: Sentry runs each integration's `processEvent` in list order, and
31-
* `classCallCheckNoiseFilterIntegration` reads the `third_party_code` tag `thirdPartyErrorFilterIntegration`
32-
* writes. Swapped, the filter goes inert with no type error to catch it - hence one constant that reorders as a
33-
* unit, with the order pinned by `tests/unit/setupSentryIntegrationOrderTest.ts`.
31+
* Ordered list, not independent entries: Sentry runs each integration's `processEvent` in list order, and the
32+
* noise filters read the `third_party_code` tag `thirdPartyErrorFilterIntegration` writes. Moved ahead of it, a
33+
* filter goes inert with no type error to catch it - hence one constant that reorders as a unit, with the order
34+
* pinned by `tests/unit/setupSentryIntegrationOrderTest.ts`.
3435
*/
35-
const THIRD_PARTY_NOISE_INTEGRATIONS = [thirdPartyErrorFilterIntegration, classCallCheckNoiseFilterIntegration];
36+
const THIRD_PARTY_NOISE_INTEGRATIONS = [thirdPartyErrorFilterIntegration, classCallCheckNoiseFilterIntegration, googleTranslateRecursionNoiseFilterIntegration];
3637

3738
function setupSentry(): void {
3839
const {semanticVersion, buildNumber} = getAppVersion(pkg.version);

‎tests/unit/classCallCheckNoiseFilterTest.ts‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,18 @@
1-
import classCallCheckNoiseFilterIntegration, {CLASS_CALL_CHECK_MESSAGE, isClassCallCheckNoise, THIRD_PARTY_CODE_TAG} from '@libs/telemetry/integrations/classCallCheckNoiseFilter';
1+
import classCallCheckNoiseFilterIntegration, {CLASS_CALL_CHECK_MESSAGE, isClassCallCheckNoise} from '@libs/telemetry/integrations/classCallCheckNoiseFilter';
22
import {classCallCheckNoiseFilterIntegration as webClassCallCheckNoiseFilterIntegration} from '@libs/telemetry/integrations/index.web';
33

4-
import type {Client, ErrorEvent, Exception, StackFrame} from '@sentry/core';
4+
import type {ErrorEvent, StackFrame} from '@sentry/core';
55

6-
const THIRD_PARTY_TAGS: ErrorEvent['tags'] = {[THIRD_PARTY_CODE_TAG]: true};
6+
import {buildErrorEvent, sentryClientStub, THIRD_PARTY_TAGS} from '../utils/SentryEventTestUtils';
77

88
/** The exact frames Sentry received for APP-JY0 on release 9.4.53-10. */
99
const APP_JY0_FRAMES: StackFrame[] = [
1010
{filename: 'app:///', lineno: 156, colno: 387215, function: 'r'},
1111
{filename: 'app:///', lineno: 156, colno: 384862, function: 'G'},
1212
];
1313

14-
/** `type: undefined` is what marks an error event in the SDK types, as opposed to `'transaction'`. */
15-
function buildEvent(values: Exception[], tags: ErrorEvent['tags'] = THIRD_PARTY_TAGS): ErrorEvent {
16-
return {type: undefined, tags, exception: {values}};
17-
}
18-
1914
function buildClassCallCheckEvent(frames: StackFrame[], tags: ErrorEvent['tags'] = THIRD_PARTY_TAGS): ErrorEvent {
20-
return buildEvent([{type: 'TypeError', value: CLASS_CALL_CHECK_MESSAGE, stacktrace: {frames}}], tags);
15+
return buildErrorEvent([{type: 'TypeError', value: CLASS_CALL_CHECK_MESSAGE, stacktrace: {frames}}], tags);
2116
}
2217

2318
describe('classCallCheckNoiseFilter', () => {
@@ -47,7 +42,7 @@ describe('classCallCheckNoiseFilter', () => {
4742
});
4843

4944
it('matches a chained error when every value is the signature and every frame is anonymous', () => {
50-
const event = buildEvent([
45+
const event = buildErrorEvent([
5146
{type: 'TypeError', value: CLASS_CALL_CHECK_MESSAGE, stacktrace: {frames: [{filename: 'app:///'}]}},
5247
{type: 'TypeError', value: CLASS_CALL_CHECK_MESSAGE, stacktrace: {frames: [{filename: '<anonymous>'}]}},
5348
]);
@@ -57,7 +52,7 @@ describe('classCallCheckNoiseFilter', () => {
5752

5853
describe('leaves everything else alone', () => {
5954
it('keeps a different error even when it is anonymous and tagged third-party', () => {
60-
const event = buildEvent([{type: 'TypeError', value: "Cannot read properties of undefined (reading 'se')", stacktrace: {frames: [{filename: 'app:///'}]}}]);
55+
const event = buildErrorEvent([{type: 'TypeError', value: "Cannot read properties of undefined (reading 'se')", stacktrace: {frames: [{filename: 'app:///'}]}}]);
6156
expect(isClassCallCheckNoise(event)).toBe(false);
6257
});
6358

@@ -81,7 +76,7 @@ describe('classCallCheckNoiseFilter', () => {
8176
});
8277

8378
it('keeps a chained error when only one value is the signature', () => {
84-
const event = buildEvent([
79+
const event = buildErrorEvent([
8580
{type: 'TypeError', value: CLASS_CALL_CHECK_MESSAGE, stacktrace: {frames: [{filename: 'app:///'}]}},
8681
{type: 'Error', value: 'something real', stacktrace: {frames: [{filename: 'app:///'}]}},
8782
]);
@@ -93,7 +88,7 @@ describe('classCallCheckNoiseFilter', () => {
9388
});
9489

9590
it('keeps the signature when it carries no frames at all', () => {
96-
expect(isClassCallCheckNoise(buildEvent([{type: 'TypeError', value: CLASS_CALL_CHECK_MESSAGE}]))).toBe(false);
91+
expect(isClassCallCheckNoise(buildErrorEvent([{type: 'TypeError', value: CLASS_CALL_CHECK_MESSAGE}]))).toBe(false);
9792
});
9893

9994
it('keeps a transaction event, which never carries exception values', () => {
@@ -102,10 +97,7 @@ describe('classCallCheckNoiseFilter', () => {
10297
});
10398

10499
describe('as a Sentry integration', () => {
105-
// `processEvent` ignores its client argument, so an empty stub satisfies the signature without stubbing the SDK.
106-
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- the filter never reads the client, this only satisfies the hook signature
107-
const client = Object.create(null) as Client;
108-
const processEvent = (event: ErrorEvent) => classCallCheckNoiseFilterIntegration.processEvent?.(event, {}, client);
100+
const processEvent = (event: ErrorEvent) => classCallCheckNoiseFilterIntegration.processEvent?.(event, {}, sentryClientStub);
109101

110102
it('is named so it can be identified in the integrations list', () => {
111103
expect(classCallCheckNoiseFilterIntegration.name).toBe('ClassCallCheckNoiseFilter');
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import googleTranslateRecursionNoiseFilterIntegration, {isGoogleTranslateRecursionNoise} from '@libs/telemetry/integrations/googleTranslateRecursionNoiseFilter';
2+
import {googleTranslateRecursionNoiseFilterIntegration as webGoogleTranslateRecursionNoiseFilterIntegration} from '@libs/telemetry/integrations/index.web';
3+
4+
import type {ErrorEvent, StackFrame} from '@sentry/core';
5+
6+
import {buildErrorEvent, sentryClientStub, THIRD_PARTY_TAGS} from '../utils/SentryEventTestUtils';
7+
8+
const WEBKIT_MESSAGE = 'Maximum call stack size exceeded.';
9+
const V8_MESSAGE = 'Maximum call stack size exceeded';
10+
11+
type Position = [lineno: number, colno: number];
12+
13+
function frame(filename: string, [lineno, colno]: Position): StackFrame {
14+
return {filename, lineno, colno};
15+
}
16+
17+
function callers(filename: string, positions: Position[]): StackFrame[] {
18+
return positions.map((position) => frame(filename, position));
19+
}
20+
21+
/** `count` frames alternating between two positions, `innermost` last. */
22+
function cycle(filename: string, innermost: Position, other: Position, count: number): StackFrame[] {
23+
return Array.from({length: count}, (_, index) => frame(filename, (count - 1 - index) % 2 === 0 ? innermost : other));
24+
}
25+
26+
function buildStackOverflowEvent(frames: StackFrame[], tags: ErrorEvent['tags'] = THIRD_PARTY_TAGS, value = WEBKIT_MESSAGE): ErrorEvent {
27+
return buildErrorEvent([{type: 'RangeError', value, stacktrace: {frames}}], tags);
28+
}
29+
30+
/** APP-M6P, Chrome iOS. */
31+
const APP_M6P_FRAMES: StackFrame[] = [
32+
...callers('app:///home', [
33+
[190, 70],
34+
[197, 363],
35+
[190, 41],
36+
[198, 237],
37+
[226, 382],
38+
]),
39+
...cycle('app:///home', [226, 63], [226, 408], 45),
40+
];
41+
42+
/** APP-KQF: the cycle sits under a different route than its callers. */
43+
const APP_KQF_FRAMES: StackFrame[] = [
44+
...callers('app:///home', [
45+
[190, 70],
46+
[197, 363],
47+
[190, 41],
48+
[198, 237],
49+
]),
50+
frame('app:///transition', [226, 382]),
51+
...cycle('app:///transition', [226, 63], [226, 408], 45),
52+
];
53+
54+
/** APP-M27: a page URL with no basename. */
55+
const APP_M27_FRAMES: StackFrame[] = APP_M6P_FRAMES.map((entry) => ({...entry, filename: 'app:///'}));
56+
57+
/** APP-MTE, desktop Chrome: the script reported under its own URL. */
58+
const APP_MTE_FRAMES: StackFrame[] = [
59+
...callers('app:///m=el_main', [
60+
[197, 128],
61+
[190, 39],
62+
[198, 235],
63+
[226, 380],
64+
]),
65+
...cycle('app:///m=el_main', [226, 406], [226, 61], 46),
66+
];
67+
68+
/** APP-KFE, an older Translate build: the loop sits on line 228, not 226. */
69+
const APP_KFE_FRAMES: StackFrame[] = [
70+
...callers('app:///', [
71+
[190, 70],
72+
[197, 363],
73+
[190, 41],
74+
[198, 237],
75+
[228, 382],
76+
]),
77+
...cycle('app:///', [228, 63], [228, 408], 45),
78+
];
79+
80+
describe('googleTranslateRecursionNoiseFilter', () => {
81+
it.each([
82+
['APP-M6P', APP_M6P_FRAMES, WEBKIT_MESSAGE],
83+
['APP-KQF', APP_KQF_FRAMES, WEBKIT_MESSAGE],
84+
['APP-M27', APP_M27_FRAMES, WEBKIT_MESSAGE],
85+
['APP-MTE', APP_MTE_FRAMES, V8_MESSAGE],
86+
['APP-KFE', APP_KFE_FRAMES, WEBKIT_MESSAGE],
87+
])('drops %s', (_, frames, message) => {
88+
// Given the exact frames Sentry recorded for one Translate build, because each build moves the loop and renames its functions
89+
const event = buildStackOverflowEvent(frames, THIRD_PARTY_TAGS, message);
90+
91+
// When the predicate runs on the recorded shape
92+
// Then it is dropped, because the filter must recognize every Translate build seen so far without pinning any one of them
93+
expect(isGoogleTranslateRecursionNoise(event)).toBe(true);
94+
});
95+
96+
it.each([
97+
['an untagged event', buildStackOverflowEvent(APP_M6P_FRAMES, {})],
98+
['a different error type', buildErrorEvent([{type: 'TypeError', value: WEBKIT_MESSAGE, stacktrace: {frames: APP_M6P_FRAMES}}])],
99+
[
100+
'a chained error',
101+
buildErrorEvent([
102+
{type: 'RangeError', value: WEBKIT_MESSAGE, stacktrace: {frames: APP_M6P_FRAMES}},
103+
{type: 'RangeError', value: WEBKIT_MESSAGE},
104+
]),
105+
],
106+
['a frame in one of our chunks', buildStackOverflowEvent([frame('app:///76-f662df2d477d1a4f.bundle.js', [1, 1]), ...APP_M6P_FRAMES])],
107+
['an <anonymous> script', buildStackOverflowEvent(cycle('app:///<anonymous>', [16, 9], [4, 13], 45))],
108+
['APP-ZF, a synthesized frame with no stack', buildStackOverflowEvent([frame('app:///undefined', [192, 70])])],
109+
['APP-2NH, a vendor script under its own dotted URL', buildStackOverflowEvent(cycle('app:///10042537-100413459.js', [4541, 17291], [4541, 17291], 50))],
110+
['a stack overflow with no frames', buildStackOverflowEvent([])],
111+
['a RangeError that is not a stack overflow', buildStackOverflowEvent(APP_M6P_FRAMES, THIRD_PARTY_TAGS, 'Invalid array length')],
112+
['a wasm frame compiled without a URL, which has no line number', buildStackOverflowEvent([{filename: 'app:///00b2a5aa:wasm-function[38]:0x1b6c'}, ...APP_M6P_FRAMES])],
113+
])('keeps %s', (_, event) => {
114+
// Given an event that satisfies all but one condition of the signature
115+
// When the predicate runs
116+
// Then it is kept, because one missed condition is enough to mean this could be an App error we would otherwise lose
117+
expect(isGoogleTranslateRecursionNoise(event)).toBe(false);
118+
});
119+
120+
it('drops the noise and passes anything else through as an integration', () => {
121+
// Given the integration and one matching and one non-matching event, because Sentry calls `processEvent`, not the predicate
122+
const kept = buildStackOverflowEvent(APP_M6P_FRAMES, {});
123+
124+
// When each goes through `processEvent`
125+
// Then the noise maps to `null`, which is how the SDK drops an event, and the other comes back as the same object so nothing downstream sees a copy
126+
expect(googleTranslateRecursionNoiseFilterIntegration.processEvent?.(buildStackOverflowEvent(APP_M6P_FRAMES), {}, sentryClientStub)).toBeNull();
127+
expect(googleTranslateRecursionNoiseFilterIntegration.processEvent?.(kept, {}, sentryClientStub)).toBe(kept);
128+
});
129+
130+
it('is exported from the web index', () => {
131+
// Given that the native index ships an `undefined` stub and `setupSentryIntegrationOrderTest` mocks the whole module
132+
// When the unmocked web index export is read
133+
// Then it is the real filter, because web is the only platform this runs on and nothing else checks the wiring
134+
expect(webGoogleTranslateRecursionNoiseFilterIntegration).toBe(googleTranslateRecursionNoiseFilterIntegration);
135+
});
136+
});

0 commit comments

Comments
 (0)