Skip to content

Commit 7e4d6b9

Browse files
committed
fix(browser): Cancel the worker error event once it is forwarded
The page received every uncaught worker error twice: once forwarded with its stack and once bubbled to window.onerror without one. The worker now cancels its error event when, and only when, the forward succeeded, so the message-only copy never leaves the worker and nothing on the page has to correlate the two. A failed forward, a stopped listener or an older SDK all leave the bubbled report in place. A cancelled error prints nothing, so the worker logs it to keep it in DevTools. Cancelling also silences error listeners on the Worker object in the page, so the page replays the event for them with the error object attached. A dispatched event does not reach window.onerror. The e2e app also throws a primitive in the worker to show that the ErrorEvent position gives such an error a usable frame.
1 parent 612ef6e commit 7e4d6b9

6 files changed

Lines changed: 175 additions & 67 deletions

File tree

‎dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,8 @@
1717
<button id="trigger-error-3" type="button" style="background-color: #dc3545; color: white">
1818
Trigger Worker 3 (lazily added) Error
1919
</button>
20+
<button id="trigger-primitive-error" type="button" style="background-color: #dc3545; color: white">
21+
Trigger Worker Primitive Error
22+
</button>
2023
</body>
2124
</html>

‎dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ const worker2 = new MyWorker2();
2323
const webWorkerIntegration = Sentry.webWorkerIntegration({ worker: [worker, worker2] });
2424
Sentry.addIntegration(webWorkerIntegration);
2525

26+
worker.addEventListener('error', event => {
27+
// this is part of the test, do not delete
28+
(window as any).workerErrorEvents = [
29+
...((window as any).workerErrorEvents ?? []),
30+
{ message: event.message, hasError: !!event.error },
31+
];
32+
});
33+
2634
worker.addEventListener('message', event => {
2735
// this is part of the test, do not delete
2836
console.log('received message from worker:', event.data.msg);
@@ -34,6 +42,12 @@ document.querySelector<HTMLButtonElement>('#trigger-error')!.addEventListener('c
3442
});
3543
});
3644

45+
document.querySelector<HTMLButtonElement>('#trigger-primitive-error')!.addEventListener('click', () => {
46+
worker.postMessage({
47+
msg: 'TRIGGER_PRIMITIVE_ERROR',
48+
});
49+
});
50+
3751
document.querySelector<HTMLButtonElement>('#trigger-error-2')!.addEventListener('click', () => {
3852
worker2.postMessage({
3953
msg: 'TRIGGER_ERROR',

‎dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,9 @@ self.addEventListener('message', event => {
1212
// This will throw an uncaught error in the worker
1313
throw new Error(`Uncaught error in worker`);
1414
}
15+
16+
if (event.data.msg === 'TRIGGER_PRIMITIVE_ERROR') {
17+
// A thrown primitive has no stack, so only the ErrorEvent knows where it came from
18+
throw 'Primitive thrown in worker';
19+
}
1520
});

‎dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,46 @@ test('emits exactly one event for an uncaught worker error', async ({ page }) =>
8787
await page.locator('#trigger-error').click();
8888
await firstErrorPromise;
8989

90-
// The bubbled copy of the first throw is queued right behind the forwarded
91-
// one, so the second worker's event arriving without it in between is the
92-
// signal that it was suppressed.
90+
// The worker cancels the native error event, so page listeners on the
91+
// worker object only see the replayed one, which carries the error object.
92+
expect(await page.evaluate(() => (window as any).workerErrorEvents)).toEqual([
93+
{ message: 'Uncaught Error: Uncaught error in worker', hasError: true },
94+
]);
95+
96+
// A bubbled copy of the first throw would have been reported before the
97+
// second worker's event, so its absence here shows it never happened.
9398
await page.locator('#trigger-error-2').click();
9499
await secondErrorPromise;
95100

96101
expect(mechanisms).toEqual([WORKER_MECHANISM, WORKER_MECHANISM]);
97102
});
98103

104+
test('locates a thrown primitive by its ErrorEvent position', async ({ page }) => {
105+
const errorEventPromise = waitForError('browser-webworker-vite', event => {
106+
return event.exception?.values?.[0]?.value === 'Primitive thrown in worker';
107+
});
108+
109+
await page.goto('/');
110+
111+
await page.locator('#trigger-primitive-error').click();
112+
113+
const errorEvent = await errorEventPromise;
114+
const exception = errorEvent.exception?.values?.[0];
115+
116+
expect(exception?.mechanism?.type).toBe(WORKER_MECHANISM);
117+
expect(exception?.stacktrace?.frames).toEqual([
118+
{
119+
filename: expect.stringMatching(/worker-.+\.js$/),
120+
lineno: expect.any(Number),
121+
colno: expect.any(Number),
122+
function: '?',
123+
in_app: true,
124+
},
125+
]);
126+
expect(exception?.stacktrace?.frames?.[0]?.lineno).toBeGreaterThan(0);
127+
expect(exception?.stacktrace?.frames?.[0]?.colno).toBeGreaterThan(0);
128+
});
129+
99130
test("user worker message handlers don't trigger for sentry messages", async ({ page }) => {
100131
const workerReadyPromise = new Promise<number>(resolve => {
101132
let workerMessageCount = 0;

‎packages/browser/src/integrations/webWorker.ts‎

Lines changed: 48 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { DebugImage, Integration, IntegrationFn } from '@sentry/core';
22
import {
33
addNonEnumerableProperty,
44
captureEvent,
5+
consoleSandbox,
56
debug,
67
defineIntegration,
78
getClient,
@@ -12,7 +13,7 @@ import {
1213
} from '@sentry/core';
1314
import { DEBUG_BUILD } from '../debug-build';
1415
import { eventFromUnknownInput, extractMessage, extractType } from '../eventbuilder';
15-
import { ignoreNextOnError, WINDOW } from '../helpers';
16+
import { WINDOW } from '../helpers';
1617
import {
1718
_enhanceEventWithInitialFrame,
1819
_eventFromRejectionWithPrimitive,
@@ -27,8 +28,6 @@ interface WebWorkerMessage {
2728
_sentryModuleMetadata?: Record<string, any>; // eslint-disable-line @typescript-eslint/no-explicit-any
2829
_sentryWorkerError?: SerializedWorkerError;
2930
_sentryWasmImages?: Array<DebugImage>;
30-
/** Sent by workers that forward uncaught errors, not only rejections. */
31-
_sentryForwardsErrors?: boolean;
3231
}
3332

3433
type WorkerErrorKind = 'error' | 'unhandledrejection';
@@ -40,6 +39,8 @@ interface SerializedWorkerError {
4039
kind?: WorkerErrorKind;
4140
/** Structured clone resets any name outside the built-in set to `Error`. */
4241
name?: string;
42+
/** The `ErrorEvent` message, replayed on the worker object for page listeners. */
43+
message?: string;
4344
/** Script the error was thrown in, which can differ from the worker script. */
4445
url?: string;
4546
lineno?: number;
@@ -138,27 +139,10 @@ export const webWorkerIntegration = defineIntegration(({ worker }: WebWorkerInte
138139
})) as IntegrationFn<WebWorkerIntegration>;
139140

140141
function listenForSentryMessages(worker: Worker): void {
141-
let forwardsErrors = false;
142-
143-
// An uncaught worker error fires `error` on the worker object and, unless
144-
// cancelled, is then reported to `window.onerror` in the same task. The
145-
// worker already forwarded it with a real stack, so the global handler
146-
// must skip the message-only copy. Not cancelling keeps the browser's own
147-
// console report.
148-
worker.addEventListener('error', () => {
149-
if (forwardsErrors) {
150-
ignoreNextOnError();
151-
}
152-
});
153-
154142
worker.addEventListener('message', event => {
155143
if (isSentryMessage(event.data)) {
156144
event.stopImmediatePropagation(); // other listeners should not receive this message
157145

158-
if (event.data._sentryForwardsErrors) {
159-
forwardsErrors = true;
160-
}
161-
162146
// Handle debug IDs
163147
if (event.data._sentryDebugIds) {
164148
DEBUG_BUILD && debug.log('Sentry debugId web worker message received', event.data);
@@ -201,21 +185,14 @@ function listenForSentryMessages(worker: Worker): void {
201185
// Handle errors and unhandled rejections forwarded from worker
202186
if (event.data._sentryWorkerError) {
203187
DEBUG_BUILD && debug.log('Sentry worker error message received', event.data._sentryWorkerError);
204-
handleForwardedWorkerError(event.data._sentryWorkerError);
188+
handleForwardedWorkerError(worker, event.data._sentryWorkerError);
205189
}
206190
}
207191
});
208192
}
209193

210-
function handleForwardedWorkerError(workerError: SerializedWorkerError): void {
211-
const client = getClient();
212-
if (!client) {
213-
return;
214-
}
215-
216-
const { stackParser, attachStacktrace } = client.getOptions();
217-
218-
const { reason, kind, name, filename, url, lineno, colno, plainError } = workerError;
194+
function handleForwardedWorkerError(worker: Worker, workerError: SerializedWorkerError): void {
195+
const { reason, kind, name, filename, url, lineno, colno, plainError, message } = workerError;
219196
// Older workers only ever forwarded rejections and send no `kind`.
220197
const isUnhandledRejection = kind !== 'error';
221198

@@ -225,6 +202,21 @@ function handleForwardedWorkerError(workerError: SerializedWorkerError): void {
225202
addNonEnumerableProperty(error, 'name', name);
226203
}
227204

205+
// The worker cancelled its native error event, which also silences
206+
// `error` listeners on the worker object in the page. Replay it for them
207+
// with the error object the native event never carries. A dispatched
208+
// event has no default action, so it does not reach `window.onerror`.
209+
if (!isUnhandledRejection && typeof ErrorEvent === 'function') {
210+
worker.dispatchEvent(new ErrorEvent('error', { message, filename: url || filename, lineno, colno, error }));
211+
}
212+
213+
const client = getClient();
214+
if (!client) {
215+
return;
216+
}
217+
218+
const { stackParser, attachStacktrace } = client.getOptions();
219+
228220
// Follow same pattern as globalHandlers for each source.
229221
// A thrown primitive is not a rejection, so the rejection-specific wording must not apply to it.
230222
const event =
@@ -319,33 +311,46 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void {
319311
_sentryMessage: true,
320312
_sentryDebugIds: self._sentryDebugIds ?? undefined,
321313
_sentryModuleMetadata: self._sentryModuleMetadata ?? undefined,
322-
_sentryForwardsErrors: true,
323314
});
324315

325-
const forward = (serializedError: Omit<SerializedWorkerError, 'filename' | 'name'>): void => {
316+
const forward = (serializedError: Omit<SerializedWorkerError, 'filename' | 'name'>): boolean => {
326317
const { reason } = serializedError;
327318

328-
postSerializedWorkerError(self, {
319+
DEBUG_BUILD && debug.log(`[Sentry Worker] Forwarding ${serializedError.kind} to parent`, serializedError);
320+
321+
return postSerializedWorkerError(self, {
329322
...serializedError,
330323
filename: self.location?.href,
331324
name: isError(reason) ? extractType(reason) : undefined,
332325
});
333-
334-
DEBUG_BUILD && debug.log(`[Sentry Worker] Forwarding ${serializedError.kind} to parent`, serializedError);
335326
};
336327

337328
// Uncaught errors bubble to the parent, but the propagated ErrorEvent
338329
// carries no error object. Forwarding the object keeps the real stack.
339330
self.addEventListener('error', (event: unknown) => {
340-
const { error, message, filename, lineno, colno } = event as {
331+
const errorEvent = event as {
341332
error?: unknown;
342333
message?: string;
343334
filename?: string;
344335
lineno?: number;
345336
colno?: number;
337+
preventDefault?: () => void;
346338
};
339+
const { error, message, filename, lineno, colno } = errorEvent;
340+
const reason = error ?? message;
341+
342+
if (!forward({ kind: 'error', reason, message, url: filename, lineno, colno })) {
343+
return;
344+
}
347345

348-
forward({ kind: 'error', reason: error ?? message, url: filename, lineno, colno });
346+
// The page now has the error with its stack, so the message-only copy
347+
// must not bubble there as well. A cancelled error prints nothing, so
348+
// log it to keep it visible in DevTools.
349+
errorEvent.preventDefault?.();
350+
consoleSandbox(() => {
351+
// eslint-disable-next-line no-console
352+
console.error(reason);
353+
});
349354
});
350355

351356
// Unhandled rejections do not bubble to the parent thread at all.
@@ -359,20 +364,19 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void {
359364
/**
360365
* `postMessage` structured-clones the reason. A `DataCloneError` must never
361366
* escape the worker's own error handler, so the forward is retried with
362-
* plain data. The page suppresses the bubbled copy of every error once the
363-
* worker has announced itself, so the retry must clone in every browser,
364-
* including ones that cannot clone `Error` at all.
367+
* plain data that clones in every browser, including ones that cannot clone
368+
* `Error` at all. Returns whether the page received the error.
365369
*/
366370
function postSerializedWorkerError(
367371
self: MinimalDedicatedWorkerGlobalScope,
368372
serializedError: SerializedWorkerError,
369-
): void {
373+
): boolean {
370374
try {
371375
self.postMessage({
372376
_sentryMessage: true,
373377
_sentryWorkerError: serializedError,
374378
});
375-
return;
379+
return true;
376380
} catch {
377381
// Not cloneable, fall through and send plain data instead.
378382
}
@@ -386,8 +390,10 @@ function postSerializedWorkerError(
386390
_sentryMessage: true,
387391
_sentryWorkerError: { ...serializedError, reason: plainReason, plainError },
388392
});
393+
return true;
389394
} catch {
390395
// Dropping the forward is better than throwing out of the worker's error handler.
396+
return false;
391397
}
392398
}
393399

0 commit comments

Comments
 (0)