From 725ad376703d9d5268e04955139e52f28590892a Mon Sep 17 00:00:00 2001 From: Ana-Maria Dumitrache Date: Tue, 8 Sep 2026 16:37:56 +0200 Subject: [PATCH 01/11] fix(browser): forward uncaught worker errors with their stack - Add an `error` listener in `registerWebWorker` that posts `event.error` (falling back to `event.message`) over the existing `_sentryWorkerError` channel - Add optional `kind` discriminator to `SerializedWorkerError`; a missing `kind` means rejection, so workers registered by an older SDK keep working - Rename `handleForwardedWorkerRejection` to `handleForwardedWorkerError` and branch on `kind` for both the mechanism and `eventFromUnknownInput`'s `isUnhandledRejection` argument - Report forwarded throws under the `auto.browser.web_worker.onerror` mechanism - Restrict `_eventFromRejectionWithPrimitive` to rejections so a thrown primitive is not labelled "Non-Error promise rejection" - Set `Error.stackTraceLimit = 50` in the worker, matching globalHandlersIntegration, since V8's default of 10 truncates stacks before they are forwarded - Wrap the forwarding `postMessage` so a non-cloneable reason is described instead of raising DataCloneError out of the worker's error handler - Correct the doc comment claiming globalHandlers already captures sync worker errors --- .../browser/src/integrations/webWorker.ts | 99 ++++++++++++++----- 1 file changed, 77 insertions(+), 22 deletions(-) diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index e9a8e338d287..5b2a4d1057ab 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -18,6 +18,8 @@ interface WebWorkerMessage { interface SerializedWorkerError { reason: unknown; filename?: string; + /** Absent on workers registered by an SDK version that only forwarded rejections. */ + kind?: 'error' | 'unhandledrejection'; } interface WebWorkerIntegrationOptions { @@ -153,31 +155,33 @@ function listenForSentryMessages(worker: Worker): void { ]; } - // Handle unhandled rejections forwarded from worker + // Handle errors and unhandled rejections forwarded from worker if (event.data._sentryWorkerError) { - DEBUG_BUILD && debug.log('Sentry worker rejection message received', event.data._sentryWorkerError); - handleForwardedWorkerRejection(event.data._sentryWorkerError); + DEBUG_BUILD && debug.log('Sentry worker error message received', event.data._sentryWorkerError); + handleForwardedWorkerError(event.data._sentryWorkerError); } } }); } -function handleForwardedWorkerRejection(workerError: SerializedWorkerError): void { +function handleForwardedWorkerError(workerError: SerializedWorkerError): void { const client = getClient(); if (!client) { return; } - const stackParser = client.getOptions().stackParser; - const attachStacktrace = client.getOptions().attachStacktrace; + const { stackParser, attachStacktrace } = client.getOptions(); const error = workerError.reason; + // Older workers only ever forwarded rejections and send no `kind`. + const isUnhandledRejection = workerError.kind !== 'error'; - // Follow same pattern as globalHandlers for unhandledrejection - // Handle both primitives and errors the same way - const event = isPrimitive(error) - ? _eventFromRejectionWithPrimitive(error) - : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, true); + // Follow same pattern as globalHandlers for each source. + // A thrown primitive is not a rejection, so the rejection-specific wording must not apply to it. + const event = + isUnhandledRejection && isPrimitive(error) + ? _eventFromRejectionWithPrimitive(error) + : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, isUnhandledRejection); event.level = 'error'; @@ -195,11 +199,11 @@ function handleForwardedWorkerRejection(workerError: SerializedWorkerError): voi originalException: error, mechanism: { handled: false, - type: 'auto.browser.web_worker.onunhandledrejection', + type: isUnhandledRejection ? 'auto.browser.web_worker.onunhandledrejection' : 'auto.browser.web_worker.onerror', }, }); - DEBUG_BUILD && debug.log('Captured worker unhandled rejection', error); + DEBUG_BUILD && debug.log(`Captured worker ${isUnhandledRejection ? 'unhandled rejection' : 'error'}`, error); } /** @@ -230,11 +234,12 @@ interface RegisterWebWorkerOptions { * This function will: * - Send debug IDs to the parent thread * - Send module metadata to the parent thread (for thirdPartyErrorFilterIntegration) - * - Set up a handler for unhandled rejections in the worker - * - Forward unhandled rejections to the parent thread for capture + * - Set up handlers for uncaught errors and unhandled rejections in the worker + * - Forward both to the parent thread for capture * - * Note: Synchronous errors in workers are already captured by globalHandlers. - * This only handles unhandled promise rejections which don't bubble to the parent. + * Note: uncaught errors do bubble to the parent, but the propagated `ErrorEvent` carries + * no `error` object, so globalHandlers can only build an event from the message string. + * Forwarding them here preserves the real stack, which matters most for wasm frames. * * @example * ```ts filename={worker.js} @@ -250,6 +255,10 @@ interface RegisterWebWorkerOptions { * - `self`: The worker instance you're calling this function from (self). */ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { + // Mirrors globalHandlersIntegration. The worker has no client of its own, so without this + // V8's default of 10 truncates stacks before they can be forwarded. + Error.stackTraceLimit = 50; + // Send debug IDs and raw module metadata to parent thread // The metadata will be parsed lazily on the main thread when needed self.postMessage({ @@ -258,6 +267,23 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { _sentryModuleMetadata: self._sentryModuleMetadata ?? undefined, }); + // Set up error handler inside the worker + // Uncaught errors bubble to the parent, but structured clone preserves `stack` while the + // propagated ErrorEvent does not, so forwarding is what gives the parent real frames + self.addEventListener('error', (event: unknown) => { + const { error, message } = event as { error?: unknown; message?: string }; + + const serializedError: SerializedWorkerError = { + reason: error ?? message, + filename: self.location?.href, + kind: 'error', + }; + + postSerializedWorkerError(self, serializedError); + + DEBUG_BUILD && debug.log('[Sentry Worker] Forwarding error to parent', serializedError); + }); + // Set up unhandledrejection handler inside the worker // Following the same pattern as globalHandlers // unhandled rejections don't bubble to the parent thread, so we need to handle them here @@ -269,18 +295,47 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { const serializedError: SerializedWorkerError = { reason: reason, filename: self.location?.href, + kind: 'unhandledrejection', }; - // Forward to parent thread + postSerializedWorkerError(self, serializedError); + + DEBUG_BUILD && debug.log('[Sentry Worker] Forwarding unhandled rejection to parent', serializedError); + }); + + DEBUG_BUILD && debug.log('[Sentry Worker] Registered worker with error and unhandled rejection handling'); +} + +/** + * `postMessage` structured-clones the reason. Errors clone well (`message`, `stack` and `cause` + * all survive), but exotic values raise `DataCloneError`, which must never escape the worker's + * own error handler. + */ +function postSerializedWorkerError( + self: MinimalDedicatedWorkerGlobalScope, + serializedError: SerializedWorkerError, +): void { + try { self.postMessage({ _sentryMessage: true, _sentryWorkerError: serializedError, }); + return; + } catch { + // Not cloneable, fall through and describe it instead. + } - DEBUG_BUILD && debug.log('[Sentry Worker] Forwarding unhandled rejection to parent', serializedError); - }); - - DEBUG_BUILD && debug.log('[Sentry Worker] Registered worker with unhandled rejection handling'); + try { + self.postMessage({ + _sentryMessage: true, + _sentryWorkerError: { + ...serializedError, + reason: `Worker error with non-cloneable reason: ${Object.prototype.toString.call(serializedError.reason)}`, + }, + }); + } catch { + // Dropping the forward is better than throwing out of the worker's error handler. + } } function isSentryMessage(eventData: unknown): eventData is WebWorkerMessage { From 5aa926ffe77dae1d951f29281c4743ba0f2ea37f Mon Sep 17 00:00:00 2001 From: Ana-Maria Dumitrache Date: Tue, 8 Sep 2026 16:52:09 +0200 Subject: [PATCH 02/11] test(browser): cover uncaught worker error forwarding --- .../tests/errors.test.ts | 43 ++-- .../test/integrations/webWorker.test.ts | 204 ++++++++++++++++++ 2 files changed, 234 insertions(+), 13 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts index a253c5ef4847..6126f0bf4b83 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts @@ -7,9 +7,14 @@ function waitForPageloadSpan() { }); } +// The throw still bubbles to the page after the worker forwards it, so globalHandlers emits a +// second, frameless event for the same error. Every test below selects the forwarded one by its +// mechanism, since that is the event carrying the real stack. +const WORKER_MECHANISM = 'auto.browser.web_worker.onerror'; + test('captures an error with debug ids and pageload trace context', async ({ page }) => { const errorEventPromise = waitForError('browser-webworker-vite', async event => { - return !event.type && !!event.exception?.values?.[0]; + return !event.type && event.exception?.values?.[0]?.mechanism?.type === WORKER_MECHANISM; }); const pageloadSpanPromise = waitForPageloadSpan(); @@ -24,9 +29,15 @@ test('captures an error with debug ids and pageload trace context', async ({ pag const pageloadSpan = await pageloadSpanPromise; expect(errorEvent.exception?.values).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught Error: Uncaught error in worker'); - expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames?.[0]?.filename).toMatch(/worker-.+\.js$/); + expect(errorEvent.exception?.values?.[0]?.type).toBe('Error'); + expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught error in worker'); + expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toEqual( + expect.arrayContaining([expect.objectContaining({ filename: expect.stringMatching(/worker-.+\.js$/) })]), + ); + + expect(errorEvent.contexts?.worker).toEqual({ + filename: expect.stringMatching(/worker-.+\.js$/), + }); expect(errorEvent.transaction).toBe('/'); expect(pageloadSpan.name).toBe('Pageload'); @@ -75,7 +86,7 @@ test("user worker message handlers don't trigger for sentry messages", async ({ test('captures an error from the second eagerly added worker', async ({ page }) => { const errorEventPromise = waitForError('browser-webworker-vite', async event => { - return !event.type && !!event.exception?.values?.[0]; + return !event.type && event.exception?.values?.[0]?.mechanism?.type === WORKER_MECHANISM; }); const pageloadSpanPromise = waitForPageloadSpan(); @@ -90,9 +101,10 @@ test('captures an error from the second eagerly added worker', async ({ page }) const pageloadSpan = await pageloadSpanPromise; expect(errorEvent.exception?.values).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught Error: Uncaught error in worker 2'); - expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames?.[0]?.filename).toMatch(/worker2-.+\.js$/); + expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught error in worker 2'); + expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toEqual( + expect.arrayContaining([expect.objectContaining({ filename: expect.stringMatching(/worker2-.+\.js$/) })]), + ); expect(errorEvent.transaction).toBe('/'); expect(pageloadSpan.name).toBe('Pageload'); @@ -120,7 +132,7 @@ test('captures an error from the second eagerly added worker', async ({ page }) test('captures an error from the third lazily added worker', async ({ page }) => { const errorEventPromise = waitForError('browser-webworker-vite', async event => { - return !event.type && !!event.exception?.values?.[0]; + return !event.type && event.exception?.values?.[0]?.mechanism?.type === WORKER_MECHANISM; }); const pageloadSpanPromise = waitForPageloadSpan(); @@ -135,9 +147,10 @@ test('captures an error from the third lazily added worker', async ({ page }) => const pageloadSpan = await pageloadSpanPromise; expect(errorEvent.exception?.values).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught Error: Uncaught error in worker 3'); - expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toHaveLength(1); - expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames?.[0]?.filename).toMatch(/worker3-.+\.js$/); + expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught error in worker 3'); + expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toEqual( + expect.arrayContaining([expect.objectContaining({ filename: expect.stringMatching(/worker3-.+\.js$/) })]), + ); expect(errorEvent.transaction).toBe('/'); expect(pageloadSpan.name).toBe('Pageload'); @@ -165,7 +178,11 @@ test('captures an error from the third lazily added worker', async ({ page }) => test('worker errors are not tagged as third-party when module metadata is present', async ({ page }) => { const errorEventPromise = waitForError('browser-webworker-vite', async event => { - return !event.type && event.exception?.values?.[0]?.value === 'Uncaught Error: Uncaught error in worker'; + return ( + !event.type && + event.exception?.values?.[0]?.mechanism?.type === WORKER_MECHANISM && + event.exception?.values?.[0]?.value === 'Uncaught error in worker' + ); }); await page.goto('/'); diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index c239e31bd638..d64958af7dc1 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -4,8 +4,11 @@ import * as SentryCore from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { BrowserClient } from '../../src/client'; import * as helpers from '../../src/helpers'; import { INTEGRATION_NAME, registerWebWorker, webWorkerIntegration } from '../../src/integrations/webWorker'; +import { defaultStackParser } from '../../src/stack-parsers'; +import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; // Mock @sentry/core vi.mock('@sentry/core', async importActual => { @@ -510,6 +513,92 @@ describe('registerWebWorker', () => { _sentryModuleMetadata: rawMetadata, }); }); + + describe('error forwarding', () => { + // registerWebWorker raises this globally, so every test in here has to put it back. + const originalStackTraceLimit = Error.stackTraceLimit; + + afterEach(() => { + Error.stackTraceLimit = originalStackTraceLimit; + }); + + function getListener(type: string): (event: unknown) => void { + const call = mockWorkerSelf.addEventListener.mock.calls.find(([eventType]) => eventType === type); + return call![1]; + } + + it('raises the stack trace limit so forwarded stacks are not truncated', () => { + Error.stackTraceLimit = 10; + + registerWebWorker({ self: mockWorkerSelf as any }); + + expect(Error.stackTraceLimit).toBe(50); + }); + + it('forwards an uncaught error with its stack and kind "error"', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + const error = new Error('boom'); + getListener('error')({ error, message: 'Uncaught Error: boom' }); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: { + reason: error, + filename: undefined, + kind: 'error', + }, + }); + }); + + it('falls back to the event message when there is no error object', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + getListener('error')({ error: null, message: 'Uncaught Error: boom' }); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: expect.objectContaining({ + reason: 'Uncaught Error: boom', + kind: 'error', + }), + }); + }); + + it('tags forwarded rejections with kind "unhandledrejection"', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + const reason = new Error('rejected'); + getListener('unhandledrejection')({ reason }); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: { + reason, + filename: undefined, + kind: 'unhandledrejection', + }, + }); + }); + + it('describes a non-cloneable reason instead of throwing out of the error handler', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + mockWorkerSelf.postMessage.mockImplementationOnce(() => { + throw new DOMException('could not be cloned', 'DataCloneError'); + }); + + expect(() => getListener('error')({ error: () => {} })).not.toThrow(); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: expect.objectContaining({ + reason: 'Worker error with non-cloneable reason: [object Function]', + kind: 'error', + }), + }); + }); + }); }); describe('registerWebWorker and webWorkerIntegration', () => { @@ -614,3 +703,118 @@ describe('registerWebWorker and webWorkerIntegration', () => { }); }); }); + +describe('forwarded worker errors', () => { + let client: BrowserClient; + let captureEventSpy: ReturnType; + let messageHandler: (event: any) => void; + + beforeEach(() => { + vi.clearAllMocks(); + + client = new BrowserClient({ + ...getDefaultBrowserClientOptions(), + stackParser: defaultStackParser, + }); + SentryCore.setCurrentClient(client); + client.init(); + captureEventSpy = vi.spyOn(client, 'captureEvent'); + + const mockWorker = { addEventListener: vi.fn(), postMessage: vi.fn() }; + const integration = webWorkerIntegration({ worker: mockWorker as any }); + integration.setupOnce!(); + messageHandler = mockWorker.addEventListener.mock.calls[0]![1]; + }); + + function forward(workerError: Record): void { + messageHandler({ + data: { _sentryMessage: true, _sentryWorkerError: workerError }, + stopImmediatePropagation: vi.fn(), + }); + } + + function capturedEvent(): SentryCore.Event { + return captureEventSpy.mock.lastCall![0] as SentryCore.Event; + } + + it('captures a forwarded error with the onerror mechanism', () => { + const error = new Error('boom'); + + forward({ reason: error, filename: 'http://localhost/worker.js', kind: 'error' }); + + expect(captureEventSpy).toHaveBeenCalledWith( + expect.objectContaining({ level: 'error' }), + expect.objectContaining({ + originalException: error, + mechanism: { handled: false, type: 'auto.browser.web_worker.onerror' }, + }), + expect.anything(), + ); + }); + + it('captures a forwarded rejection with the onunhandledrejection mechanism', () => { + const reason = new Error('rejected'); + + forward({ reason, kind: 'unhandledrejection' }); + + expect(captureEventSpy).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + mechanism: { handled: false, type: 'auto.browser.web_worker.onunhandledrejection' }, + }), + expect.anything(), + ); + }); + + it('treats a payload without kind as a rejection, for workers on an older SDK', () => { + forward({ reason: new Error('rejected') }); + + expect(captureEventSpy).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + mechanism: { handled: false, type: 'auto.browser.web_worker.onunhandledrejection' }, + }), + expect.anything(), + ); + }); + + it('does not apply promise-rejection wording to a thrown primitive', () => { + forward({ reason: 'just a string', kind: 'error' }); + + expect(capturedEvent().exception?.values?.[0]).toEqual( + expect.objectContaining({ + type: 'Error', + value: 'just a string', + }), + ); + }); + + it('keeps promise-rejection wording for a rejected primitive', () => { + forward({ reason: 'just a string', kind: 'unhandledrejection' }); + + expect(capturedEvent().exception?.values?.[0]).toEqual( + expect.objectContaining({ + type: 'UnhandledRejection', + value: 'Non-Error promise rejection captured with value: just a string', + }), + ); + }); + + it('parses the forwarded stack into frames, including wasm frames', () => { + const error = new Error('divide by zero'); + error.stack = [ + 'RuntimeError: divide by zero', + ' at trigger_crash (http://localhost:8080/maze.wasm:wasm-function[36]:0x2877)', + ' at runStepGame (http://localhost:8080/worker.js:12:9)', + ].join('\n'); + + forward({ reason: error, kind: 'error' }); + + expect(capturedEvent().exception?.values?.[0]?.stacktrace?.frames).toEqual( + expect.arrayContaining([ + expect.objectContaining({ filename: 'http://localhost:8080/maze.wasm:wasm-function[36]:0x2877' }), + expect.objectContaining({ filename: 'http://localhost:8080/worker.js' }), + ]), + ); + }); +}); From e1d3cae906e6a89a6b5299608e15fa271d1b2b23 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 11 Sep 2026 12:16:55 +0200 Subject: [PATCH 03/11] fix(browser): Dedupe forwarded worker errors and keep their name The parent already holds the Worker object, so it listens for its error event and tells globalHandlers to skip the frameless copy that bubbles to window.onerror. The event is not cancelled, so the browser still prints its own report. The skip only applies once the worker announced that it forwards errors, so workers on an older SDK keep the bubbled event. Structured clone resets any error name outside the built-in set, so the worker sends the name separately and the parent restores it. When the reason cannot be cloned, the worker retries with a fresh Error that keeps message and stack, or with a normalized value for anything else. Message-only errors get a frame from the ErrorEvent location, the same way globalHandlers does. --- .../tests/errors.test.ts | 41 ++- .../src/integrations/globalhandlers.ts | 5 +- .../browser/src/integrations/webWorker.ts | 140 +++++++--- .../test/integrations/webWorker.test.ts | 256 +++++++++++++----- 4 files changed, 312 insertions(+), 130 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts index 6126f0bf4b83..6b050b40a8f1 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts @@ -7,14 +7,14 @@ function waitForPageloadSpan() { }); } -// The throw still bubbles to the page after the worker forwards it, so globalHandlers emits a -// second, frameless event for the same error. Every test below selects the forwarded one by its -// mechanism, since that is the event carrying the real stack. +// The throw still bubbles to the page after the worker forwards it, but +// the integration makes globalHandlers skip that frameless copy. So the +// first error event to arrive must be the forwarded one. const WORKER_MECHANISM = 'auto.browser.web_worker.onerror'; test('captures an error with debug ids and pageload trace context', async ({ page }) => { const errorEventPromise = waitForError('browser-webworker-vite', async event => { - return !event.type && event.exception?.values?.[0]?.mechanism?.type === WORKER_MECHANISM; + return !event.type && !!event.exception?.values?.[0]; }); const pageloadSpanPromise = waitForPageloadSpan(); @@ -29,6 +29,7 @@ test('captures an error with debug ids and pageload trace context', async ({ pag const pageloadSpan = await pageloadSpanPromise; expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.mechanism?.type).toBe(WORKER_MECHANISM); expect(errorEvent.exception?.values?.[0]?.type).toBe('Error'); expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught error in worker'); expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toEqual( @@ -63,6 +64,26 @@ test('captures an error with debug ids and pageload trace context', async ({ pag }); }); +test('emits exactly one event for an uncaught worker error', async ({ page }) => { + const mechanisms: Array = []; + // Never resolves. It only records every error event that arrives, so + // the global handler's copy of the throw would show up here. + void waitForError('browser-webworker-vite', event => { + if (!event.type && event.exception?.values?.[0]) { + mechanisms.push(event.exception.values[0].mechanism?.type); + } + return false; + }); + + await page.goto('/'); + + await page.locator('#trigger-error').click(); + + await page.waitForTimeout(2000); + + expect(mechanisms).toEqual([WORKER_MECHANISM]); +}); + test("user worker message handlers don't trigger for sentry messages", async ({ page }) => { const workerReadyPromise = new Promise(resolve => { let workerMessageCount = 0; @@ -86,7 +107,7 @@ test("user worker message handlers don't trigger for sentry messages", async ({ test('captures an error from the second eagerly added worker', async ({ page }) => { const errorEventPromise = waitForError('browser-webworker-vite', async event => { - return !event.type && event.exception?.values?.[0]?.mechanism?.type === WORKER_MECHANISM; + return !event.type && !!event.exception?.values?.[0]; }); const pageloadSpanPromise = waitForPageloadSpan(); @@ -101,6 +122,7 @@ test('captures an error from the second eagerly added worker', async ({ page }) const pageloadSpan = await pageloadSpanPromise; expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.mechanism?.type).toBe(WORKER_MECHANISM); expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught error in worker 2'); expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toEqual( expect.arrayContaining([expect.objectContaining({ filename: expect.stringMatching(/worker2-.+\.js$/) })]), @@ -132,7 +154,7 @@ test('captures an error from the second eagerly added worker', async ({ page }) test('captures an error from the third lazily added worker', async ({ page }) => { const errorEventPromise = waitForError('browser-webworker-vite', async event => { - return !event.type && event.exception?.values?.[0]?.mechanism?.type === WORKER_MECHANISM; + return !event.type && !!event.exception?.values?.[0]; }); const pageloadSpanPromise = waitForPageloadSpan(); @@ -147,6 +169,7 @@ test('captures an error from the third lazily added worker', async ({ page }) => const pageloadSpan = await pageloadSpanPromise; expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.mechanism?.type).toBe(WORKER_MECHANISM); expect(errorEvent.exception?.values?.[0]?.value).toBe('Uncaught error in worker 3'); expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toEqual( expect.arrayContaining([expect.objectContaining({ filename: expect.stringMatching(/worker3-.+\.js$/) })]), @@ -178,11 +201,7 @@ test('captures an error from the third lazily added worker', async ({ page }) => test('worker errors are not tagged as third-party when module metadata is present', async ({ page }) => { const errorEventPromise = waitForError('browser-webworker-vite', async event => { - return ( - !event.type && - event.exception?.values?.[0]?.mechanism?.type === WORKER_MECHANISM && - event.exception?.values?.[0]?.value === 'Uncaught error in worker' - ); + return !event.type && event.exception?.values?.[0]?.value === 'Uncaught error in worker'; }); await page.goto('/'); diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index 0dbeb46a0fce..f258913a8e16 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -156,7 +156,10 @@ export function _eventFromRejectionWithPrimitive(reason: Primitive): Event { }; } -function _enhanceEventWithInitialFrame( +/** + * Adds a frame built from the error location when the event has none. + */ +export function _enhanceEventWithInitialFrame( event: Event, url: string | undefined, lineno: number | undefined, diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index 5b2a4d1057ab..23b67f1a2e71 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -1,9 +1,23 @@ import type { DebugImage, Integration, IntegrationFn } from '@sentry/core'; -import { captureEvent, debug, defineIntegration, getClient, isPlainObject, isPrimitive } from '@sentry/core'; +import { + addNonEnumerableProperty, + captureEvent, + debug, + defineIntegration, + getClient, + isError, + isPlainObject, + isPrimitive, + normalize, +} from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; -import { eventFromUnknownInput } from '../eventbuilder'; -import { WINDOW } from '../helpers'; -import { _eventFromRejectionWithPrimitive, _getUnhandledRejectionError } from './globalhandlers'; +import { eventFromUnknownInput, extractMessage, extractType } from '../eventbuilder'; +import { ignoreNextOnError, WINDOW } from '../helpers'; +import { + _enhanceEventWithInitialFrame, + _eventFromRejectionWithPrimitive, + _getUnhandledRejectionError, +} from './globalhandlers'; export const INTEGRATION_NAME = 'WebWorker' as const; @@ -13,13 +27,21 @@ interface WebWorkerMessage { _sentryModuleMetadata?: Record; // eslint-disable-line @typescript-eslint/no-explicit-any _sentryWorkerError?: SerializedWorkerError; _sentryWasmImages?: Array; + /** Sent by workers that forward uncaught errors, not only rejections. */ + _sentryForwardsErrors?: boolean; } +type WorkerErrorKind = 'error' | 'unhandledrejection'; + interface SerializedWorkerError { reason: unknown; filename?: string; /** Absent on workers registered by an SDK version that only forwarded rejections. */ - kind?: 'error' | 'unhandledrejection'; + kind?: WorkerErrorKind; + /** Structured clone resets any name outside the built-in set to `Error`. */ + name?: string; + lineno?: number; + colno?: number; } interface WebWorkerIntegrationOptions { @@ -112,10 +134,27 @@ export const webWorkerIntegration = defineIntegration(({ worker }: WebWorkerInte })) as IntegrationFn; function listenForSentryMessages(worker: Worker): void { + let forwardsErrors = false; + + // An uncaught worker error fires `error` on the worker object and, unless + // cancelled, is then reported to `window.onerror` in the same task. The + // worker already forwarded it with a real stack, so the global handler + // must skip the message-only copy. Not cancelling keeps the browser's own + // console report. + worker.addEventListener('error', () => { + if (forwardsErrors) { + ignoreNextOnError(); + } + }); + worker.addEventListener('message', event => { if (isSentryMessage(event.data)) { event.stopImmediatePropagation(); // other listeners should not receive this message + if (event.data._sentryForwardsErrors) { + forwardsErrors = true; + } + // Handle debug IDs if (event.data._sentryDebugIds) { DEBUG_BUILD && debug.log('Sentry debugId web worker message received', event.data); @@ -172,9 +211,13 @@ function handleForwardedWorkerError(workerError: SerializedWorkerError): void { const { stackParser, attachStacktrace } = client.getOptions(); - const error = workerError.reason; + const { reason: error, kind, name, filename, lineno, colno } = workerError; // Older workers only ever forwarded rejections and send no `kind`. - const isUnhandledRejection = workerError.kind !== 'error'; + const isUnhandledRejection = kind !== 'error'; + + if (name && isError(error) && error.name !== name) { + addNonEnumerableProperty(error, 'name', name); + } // Follow same pattern as globalHandlers for each source. // A thrown primitive is not a rejection, so the rejection-specific wording must not apply to it. @@ -183,14 +226,18 @@ function handleForwardedWorkerError(workerError: SerializedWorkerError): void { ? _eventFromRejectionWithPrimitive(error) : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, isUnhandledRejection); + if (!isUnhandledRejection) { + _enhanceEventWithInitialFrame(event, filename, lineno, colno); + } + event.level = 'error'; // Add worker-specific context - if (workerError.filename) { + if (filename) { event.contexts = { ...event.contexts, worker: { - filename: workerError.filename, + filename, }, }; } @@ -256,7 +303,7 @@ interface RegisterWebWorkerOptions { */ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { // Mirrors globalHandlersIntegration. The worker has no client of its own, so without this - // V8's default of 10 truncates stacks before they can be forwarded. + // V8's default of 10 truncates stacks before this code forwards them. Error.stackTraceLimit = 50; // Send debug IDs and raw module metadata to parent thread @@ -265,51 +312,46 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { _sentryMessage: true, _sentryDebugIds: self._sentryDebugIds ?? undefined, _sentryModuleMetadata: self._sentryModuleMetadata ?? undefined, + _sentryForwardsErrors: true, }); - // Set up error handler inside the worker - // Uncaught errors bubble to the parent, but structured clone preserves `stack` while the - // propagated ErrorEvent does not, so forwarding is what gives the parent real frames - self.addEventListener('error', (event: unknown) => { - const { error, message } = event as { error?: unknown; message?: string }; + const forward = (serializedError: Omit): void => { + const { reason } = serializedError; - const serializedError: SerializedWorkerError = { - reason: error ?? message, + postSerializedWorkerError(self, { + ...serializedError, filename: self.location?.href, - kind: 'error', - }; - - postSerializedWorkerError(self, serializedError); - - DEBUG_BUILD && debug.log('[Sentry Worker] Forwarding error to parent', serializedError); - }); + name: isError(reason) ? extractType(reason) : undefined, + }); - // Set up unhandledrejection handler inside the worker - // Following the same pattern as globalHandlers - // unhandled rejections don't bubble to the parent thread, so we need to handle them here - self.addEventListener('unhandledrejection', (event: unknown) => { - const reason = _getUnhandledRejectionError(event); + DEBUG_BUILD && debug.log(`[Sentry Worker] Forwarding ${serializedError.kind} to parent`, serializedError); + }; - // Forward the raw reason to parent thread - // The parent will handle primitives vs errors the same way globalHandlers does - const serializedError: SerializedWorkerError = { - reason: reason, - filename: self.location?.href, - kind: 'unhandledrejection', + // Uncaught errors bubble to the parent, but the propagated ErrorEvent + // carries no error object. Forwarding the object keeps the real stack. + self.addEventListener('error', (event: unknown) => { + const { error, message, lineno, colno } = event as { + error?: unknown; + message?: string; + lineno?: number; + colno?: number; }; - postSerializedWorkerError(self, serializedError); + forward({ kind: 'error', reason: error ?? message, lineno, colno }); + }); - DEBUG_BUILD && debug.log('[Sentry Worker] Forwarding unhandled rejection to parent', serializedError); + // Unhandled rejections do not bubble to the parent thread at all. + self.addEventListener('unhandledrejection', (event: unknown) => { + forward({ kind: 'unhandledrejection', reason: _getUnhandledRejectionError(event) }); }); DEBUG_BUILD && debug.log('[Sentry Worker] Registered worker with error and unhandled rejection handling'); } /** - * `postMessage` structured-clones the reason. Errors clone well (`message`, `stack` and `cause` - * all survive), but exotic values raise `DataCloneError`, which must never escape the worker's - * own error handler. + * `postMessage` structured-clones the reason. A `DataCloneError` must never + * escape the worker's own error handler, so the forward is retried with a + * cloneable stand-in that keeps as much of the original as possible. */ function postSerializedWorkerError( self: MinimalDedicatedWorkerGlobalScope, @@ -322,22 +364,30 @@ function postSerializedWorkerError( }); return; } catch { - // Not cloneable, fall through and describe it instead. + // Not cloneable, fall through and send a stand-in instead. } + const { reason } = serializedError; + // A fresh Error keeps message and stack but drops the `cause` that blocked + // the clone. `normalize` only produces cloneable output for everything else. + const cloneableReason = isError(reason) ? cloneableErrorFrom(reason) : normalize(reason); + try { self.postMessage({ _sentryMessage: true, - _sentryWorkerError: { - ...serializedError, - reason: `Worker error with non-cloneable reason: ${Object.prototype.toString.call(serializedError.reason)}`, - }, + _sentryWorkerError: { ...serializedError, reason: cloneableReason }, }); } catch { // Dropping the forward is better than throwing out of the worker's error handler. } } +function cloneableErrorFrom(error: Error): Error { + const clone = new Error(extractMessage(error)); + clone.stack = error.stack; + return clone; +} + function isSentryMessage(eventData: unknown): eventData is WebWorkerMessage { if (!isPlainObject(eventData) || eventData._sentryMessage !== true) { return false; diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index d64958af7dc1..bf38df7584a9 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -3,6 +3,7 @@ */ import * as SentryCore from '@sentry/core'; +import type { MockInstance } from 'vitest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { BrowserClient } from '../../src/client'; import * as helpers from '../../src/helpers'; @@ -30,8 +31,17 @@ vi.mock('../../src/helpers', () => ({ WINDOW: { _sentryDebugIds: undefined, }, + ignoreNextOnError: vi.fn(), })); +function getListener(addEventListener: ReturnType, type: string): (event: any) => void { + const call = addEventListener.mock.calls.find(([eventType]) => eventType === type); + if (!call) { + throw new Error(`No ${type} listener registered`); + } + return call[1]; +} + describe('webWorkerIntegration', () => { const mockDebugLog = SentryCore.debug.log as any; @@ -118,9 +128,7 @@ describe('webWorkerIntegration', () => { const integration = webWorkerIntegration({ worker: mockWorker as any }); integration.setupOnce!(); - // Extract the message handler from the addEventListener call - expect(mockWorker.addEventListener.mock.calls).toBeDefined(); - messageHandler = mockWorker.addEventListener.mock.calls[0]![1]; + messageHandler = getListener(mockWorker.addEventListener, 'message'); }); it('ignores non-Sentry messages', () => { @@ -412,6 +420,9 @@ describe('registerWebWorker', () => { _sentryModuleMetadata?: Record; }; + // registerWebWorker raises this globally, so every test has to put it back. + const originalStackTraceLimit = Error.stackTraceLimit; + beforeEach(() => { vi.clearAllMocks(); @@ -421,6 +432,10 @@ describe('registerWebWorker', () => { }; }); + afterEach(() => { + Error.stackTraceLimit = originalStackTraceLimit; + }); + it('posts message with _sentryMessage flag', () => { registerWebWorker({ self: mockWorkerSelf as any }); @@ -429,6 +444,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); }); @@ -448,6 +464,7 @@ describe('registerWebWorker', () => { 'worker-file2.js': 'debug-id-2', }, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); }); @@ -461,6 +478,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); }); @@ -478,6 +496,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: rawMetadata, + _sentryForwardsErrors: true, }); }); @@ -490,6 +509,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); }); @@ -511,20 +531,13 @@ describe('registerWebWorker', () => { 'worker-file.js': 'debug-id-1', }, _sentryModuleMetadata: rawMetadata, + _sentryForwardsErrors: true, }); }); describe('error forwarding', () => { - // registerWebWorker raises this globally, so every test in here has to put it back. - const originalStackTraceLimit = Error.stackTraceLimit; - - afterEach(() => { - Error.stackTraceLimit = originalStackTraceLimit; - }); - - function getListener(type: string): (event: unknown) => void { - const call = mockWorkerSelf.addEventListener.mock.calls.find(([eventType]) => eventType === type); - return call![1]; + function trigger(type: string, event: unknown): void { + getListener(mockWorkerSelf.addEventListener, type)(event); } it('raises the stack trace limit so forwarded stacks are not truncated', () => { @@ -535,11 +548,11 @@ describe('registerWebWorker', () => { expect(Error.stackTraceLimit).toBe(50); }); - it('forwards an uncaught error with its stack and kind "error"', () => { + it('forwards an uncaught error with its location, name and kind "error"', () => { registerWebWorker({ self: mockWorkerSelf as any }); const error = new Error('boom'); - getListener('error')({ error, message: 'Uncaught Error: boom' }); + trigger('error', { error, message: 'Uncaught Error: boom', lineno: 12, colno: 9 }); expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, @@ -547,20 +560,39 @@ describe('registerWebWorker', () => { reason: error, filename: undefined, kind: 'error', + name: 'Error', + lineno: 12, + colno: 9, }, }); }); + it('sends the error name separately because structured clone resets it', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + const error = new Error('divide by zero'); + error.name = 'RuntimeError'; + trigger('error', { error }); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: expect.objectContaining({ reason: error, name: 'RuntimeError' }), + }); + }); + it('falls back to the event message when there is no error object', () => { registerWebWorker({ self: mockWorkerSelf as any }); - getListener('error')({ error: null, message: 'Uncaught Error: boom' }); + trigger('error', { error: null, message: 'Uncaught Error: boom', lineno: 3, colno: 7 }); expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, _sentryWorkerError: expect.objectContaining({ reason: 'Uncaught Error: boom', kind: 'error', + name: undefined, + lineno: 3, + colno: 7, }), }); }); @@ -569,7 +601,7 @@ describe('registerWebWorker', () => { registerWebWorker({ self: mockWorkerSelf as any }); const reason = new Error('rejected'); - getListener('unhandledrejection')({ reason }); + trigger('unhandledrejection', { reason }); expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, @@ -577,25 +609,73 @@ describe('registerWebWorker', () => { reason, filename: undefined, kind: 'unhandledrejection', + name: 'Error', }, }); }); - it('describes a non-cloneable reason instead of throwing out of the error handler', () => { - registerWebWorker({ self: mockWorkerSelf as any }); + describe('when the reason cannot be structured-cloned', () => { + beforeEach(() => { + mockWorkerSelf.postMessage.mockImplementation(message => structuredClone(message)); + }); - mockWorkerSelf.postMessage.mockImplementationOnce(() => { - throw new DOMException('could not be cloned', 'DataCloneError'); + it('retries with a fresh error that keeps message and stack but drops the cause', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + const error = new Error('boom') as Error & { cause?: unknown }; + error.cause = () => {}; + expect(() => trigger('error', { error })).not.toThrow(); + + // The mocked postMessage clones for real, so a third call proves the + // retry no longer carries the function that blocked the first one. + expect(mockWorkerSelf.postMessage).toHaveBeenCalledTimes(3); + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: expect.objectContaining({ + reason: expect.objectContaining({ message: 'boom', stack: error.stack }), + name: 'Error', + kind: 'error', + }), + }); }); - expect(() => getListener('error')({ error: () => {} })).not.toThrow(); + it('keeps the message and stack of a WebAssembly.Exception', () => { + registerWebWorker({ self: mockWorkerSelf as any }); - expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ - _sentryMessage: true, - _sentryWorkerError: expect.objectContaining({ - reason: 'Worker error with non-cloneable reason: [object Function]', - kind: 'error', - }), + const tag = new WebAssembly.Tag({ parameters: [] }); + const exception = new WebAssembly.Exception(tag, [], { traceStack: true }); + trigger('error', { error: exception }); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: expect.objectContaining({ + reason: expect.objectContaining({ message: 'wasm exception', stack: exception.stack }), + name: 'WebAssembly.Exception', + }), + }); + }); + + it('normalizes a reason that is not an error', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + trigger('unhandledrejection', { reason: { retry: () => {} } }); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: expect.objectContaining({ + reason: { retry: '[Function: retry]' }, + kind: 'unhandledrejection', + }), + }); + }); + + it('does not throw out of the error handler when the retry fails as well', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + mockWorkerSelf.postMessage.mockImplementation(() => { + throw new DOMException('could not be cloned', 'DataCloneError'); + }); + + expect(() => trigger('error', { error: new Error('boom') })).not.toThrow(); }); }); }); @@ -667,6 +747,7 @@ describe('registerWebWorker and webWorkerIntegration', () => { _sentryMessage: true, _sentryDebugIds: mockWorker._sentryDebugIds, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); expect((helpers.WINDOW as any)._sentryDebugIds).toEqual({ @@ -688,6 +769,7 @@ describe('registerWebWorker and webWorkerIntegration', () => { _sentryMessage: true, _sentryDebugIds: mockWorker3._sentryDebugIds, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); expect((helpers.WINDOW as any)._sentryDebugIds).toEqual({ @@ -706,8 +788,8 @@ describe('registerWebWorker and webWorkerIntegration', () => { describe('forwarded worker errors', () => { let client: BrowserClient; - let captureEventSpy: ReturnType; - let messageHandler: (event: any) => void; + let captureEventSpy: MockInstance; + let mockWorker: { addEventListener: ReturnType; postMessage: ReturnType }; beforeEach(() => { vi.clearAllMocks(); @@ -720,21 +802,31 @@ describe('forwarded worker errors', () => { client.init(); captureEventSpy = vi.spyOn(client, 'captureEvent'); - const mockWorker = { addEventListener: vi.fn(), postMessage: vi.fn() }; + mockWorker = { addEventListener: vi.fn(), postMessage: vi.fn() }; const integration = webWorkerIntegration({ worker: mockWorker as any }); integration.setupOnce!(); - messageHandler = mockWorker.addEventListener.mock.calls[0]![1]; }); - function forward(workerError: Record): void { - messageHandler({ - data: { _sentryMessage: true, _sentryWorkerError: workerError }, + function receive(data: Record): void { + getListener( + mockWorker.addEventListener, + 'message', + )({ + data: { _sentryMessage: true, ...data }, stopImmediatePropagation: vi.fn(), }); } - function capturedEvent(): SentryCore.Event { - return captureEventSpy.mock.lastCall![0] as SentryCore.Event; + function forward(workerError: Record): void { + receive({ _sentryWorkerError: workerError }); + } + + function expectCapturedException(exception: Record): void { + expect(captureEventSpy).toHaveBeenCalledWith( + expect.objectContaining({ exception: { values: [expect.objectContaining(exception)] } }), + expect.anything(), + expect.anything(), + ); } it('captures a forwarded error with the onerror mechanism', () => { @@ -752,22 +844,12 @@ describe('forwarded worker errors', () => { ); }); - it('captures a forwarded rejection with the onunhandledrejection mechanism', () => { - const reason = new Error('rejected'); - - forward({ reason, kind: 'unhandledrejection' }); - - expect(captureEventSpy).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - mechanism: { handled: false, type: 'auto.browser.web_worker.onunhandledrejection' }, - }), - expect.anything(), - ); - }); - - it('treats a payload without kind as a rejection, for workers on an older SDK', () => { - forward({ reason: new Error('rejected') }); + it.each([ + ['kind "unhandledrejection"', 'unhandledrejection'], + // Workers registered by an older SDK only forwarded rejections and sent no kind. + ['no kind', undefined], + ])('captures a forwarded rejection with %s using the onunhandledrejection mechanism', (_, kind) => { + forward({ reason: new Error('rejected'), kind }); expect(captureEventSpy).toHaveBeenCalledWith( expect.anything(), @@ -781,40 +863,68 @@ describe('forwarded worker errors', () => { it('does not apply promise-rejection wording to a thrown primitive', () => { forward({ reason: 'just a string', kind: 'error' }); - expect(capturedEvent().exception?.values?.[0]).toEqual( - expect.objectContaining({ - type: 'Error', - value: 'just a string', - }), - ); + expectCapturedException({ type: 'Error', value: 'just a string' }); }); it('keeps promise-rejection wording for a rejected primitive', () => { forward({ reason: 'just a string', kind: 'unhandledrejection' }); - expect(capturedEvent().exception?.values?.[0]).toEqual( - expect.objectContaining({ - type: 'UnhandledRejection', - value: 'Non-Error promise rejection captured with value: just a string', - }), - ); + expectCapturedException({ + type: 'UnhandledRejection', + value: 'Non-Error promise rejection captured with value: just a string', + }); }); - it('parses the forwarded stack into frames, including wasm frames', () => { + it('restores the name that structured clone dropped and parses the forwarded stack', () => { const error = new Error('divide by zero'); + error.name = 'RuntimeError'; error.stack = [ 'RuntimeError: divide by zero', ' at trigger_crash (http://localhost:8080/maze.wasm:wasm-function[36]:0x2877)', ' at runStepGame (http://localhost:8080/worker.js:12:9)', ].join('\n'); + const cloned = structuredClone(error); + expect(cloned.name).toBe('Error'); + + forward({ reason: cloned, name: 'RuntimeError', kind: 'error' }); + + expectCapturedException({ + type: 'RuntimeError', + value: 'divide by zero', + stacktrace: { + frames: expect.arrayContaining([ + expect.objectContaining({ filename: 'http://localhost:8080/maze.wasm:wasm-function[36]:0x2877' }), + expect.objectContaining({ filename: 'http://localhost:8080/worker.js' }), + ]), + }, + }); + }); - forward({ reason: error, kind: 'error' }); + it('adds a frame from the error location when a message-only error has no stack', () => { + forward({ + reason: 'Uncaught Error: boom', + kind: 'error', + filename: 'http://localhost/worker.js', + lineno: 12, + colno: 9, + }); - expect(capturedEvent().exception?.values?.[0]?.stacktrace?.frames).toEqual( - expect.arrayContaining([ - expect.objectContaining({ filename: 'http://localhost:8080/maze.wasm:wasm-function[36]:0x2877' }), - expect.objectContaining({ filename: 'http://localhost:8080/worker.js' }), - ]), - ); + expectCapturedException({ + value: 'Uncaught Error: boom', + stacktrace: { + frames: [expect.objectContaining({ filename: 'http://localhost/worker.js', lineno: 12, colno: 9 })], + }, + }); + }); + + it('skips the global onerror copy only once the worker announced that it forwards errors', () => { + const onWorkerError = getListener(mockWorker.addEventListener, 'error'); + + onWorkerError({}); + expect(helpers.ignoreNextOnError).not.toHaveBeenCalled(); + + receive({ _sentryDebugIds: undefined, _sentryForwardsErrors: true }); + onWorkerError({}); + expect(helpers.ignoreNextOnError).toHaveBeenCalledTimes(1); }); }); From 62239dd8e5b127a6ccda8537d468e2b8f3fc4fed Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 11 Sep 2026 12:38:58 +0200 Subject: [PATCH 04/11] fix(browser): Use the throwing script for the worker fallback frame The ErrorEvent filename can differ from the worker script when the error comes from an imported module, so the worker forwards it and the page builds the fallback frame from it. The single-event e2e test now waits for a second worker's event instead of sleeping. The bubbled copy of the first throw is queued right behind the forwarded one, so it would arrive before that event. --- .../tests/errors.test.ts | 18 ++++++++++++--- .../browser/src/integrations/webWorker.ts | 11 +++++---- .../test/integrations/webWorker.test.ts | 23 ++++++++++++++----- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts index 6b050b40a8f1..8a6f4ab60910 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts @@ -67,7 +67,7 @@ test('captures an error with debug ids and pageload trace context', async ({ pag test('emits exactly one event for an uncaught worker error', async ({ page }) => { const mechanisms: Array = []; // Never resolves. It only records every error event that arrives, so - // the global handler's copy of the throw would show up here. + // the global handler's copy of a throw would show up here. void waitForError('browser-webworker-vite', event => { if (!event.type && event.exception?.values?.[0]) { mechanisms.push(event.exception.values[0].mechanism?.type); @@ -75,13 +75,25 @@ test('emits exactly one event for an uncaught worker error', async ({ page }) => return false; }); + const firstErrorPromise = waitForError('browser-webworker-vite', event => { + return event.exception?.values?.[0]?.value === 'Uncaught error in worker'; + }); + const secondErrorPromise = waitForError('browser-webworker-vite', event => { + return event.exception?.values?.[0]?.value === 'Uncaught error in worker 2'; + }); + await page.goto('/'); await page.locator('#trigger-error').click(); + await firstErrorPromise; - await page.waitForTimeout(2000); + // The bubbled copy of the first throw is queued right behind the forwarded + // one, so the second worker's event arriving without it in between is the + // signal that it was suppressed. + await page.locator('#trigger-error-2').click(); + await secondErrorPromise; - expect(mechanisms).toEqual([WORKER_MECHANISM]); + expect(mechanisms).toEqual([WORKER_MECHANISM, WORKER_MECHANISM]); }); test("user worker message handlers don't trigger for sentry messages", async ({ page }) => { diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index 23b67f1a2e71..2d09106ba80e 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -40,6 +40,8 @@ interface SerializedWorkerError { kind?: WorkerErrorKind; /** Structured clone resets any name outside the built-in set to `Error`. */ name?: string; + /** Script the error was thrown in, which can differ from the worker script. */ + url?: string; lineno?: number; colno?: number; } @@ -211,7 +213,7 @@ function handleForwardedWorkerError(workerError: SerializedWorkerError): void { const { stackParser, attachStacktrace } = client.getOptions(); - const { reason: error, kind, name, filename, lineno, colno } = workerError; + const { reason: error, kind, name, filename, url, lineno, colno } = workerError; // Older workers only ever forwarded rejections and send no `kind`. const isUnhandledRejection = kind !== 'error'; @@ -227,7 +229,7 @@ function handleForwardedWorkerError(workerError: SerializedWorkerError): void { : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, isUnhandledRejection); if (!isUnhandledRejection) { - _enhanceEventWithInitialFrame(event, filename, lineno, colno); + _enhanceEventWithInitialFrame(event, url ?? filename, lineno, colno); } event.level = 'error'; @@ -330,14 +332,15 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { // Uncaught errors bubble to the parent, but the propagated ErrorEvent // carries no error object. Forwarding the object keeps the real stack. self.addEventListener('error', (event: unknown) => { - const { error, message, lineno, colno } = event as { + const { error, message, filename, lineno, colno } = event as { error?: unknown; message?: string; + filename?: string; lineno?: number; colno?: number; }; - forward({ kind: 'error', reason: error ?? message, lineno, colno }); + forward({ kind: 'error', reason: error ?? message, url: filename, lineno, colno }); }); // Unhandled rejections do not bubble to the parent thread at all. diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index bf38df7584a9..836ecb165ec7 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -418,6 +418,7 @@ describe('registerWebWorker', () => { addEventListener: ReturnType; _sentryDebugIds?: Record; _sentryModuleMetadata?: Record; + location?: { href?: string }; }; // registerWebWorker raises this globally, so every test has to put it back. @@ -551,16 +552,24 @@ describe('registerWebWorker', () => { it('forwards an uncaught error with its location, name and kind "error"', () => { registerWebWorker({ self: mockWorkerSelf as any }); + mockWorkerSelf.location = { href: 'http://localhost/worker.js' }; const error = new Error('boom'); - trigger('error', { error, message: 'Uncaught Error: boom', lineno: 12, colno: 9 }); + trigger('error', { + error, + message: 'Uncaught Error: boom', + filename: 'http://localhost/chunk.js', + lineno: 12, + colno: 9, + }); expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, _sentryWorkerError: { reason: error, - filename: undefined, + filename: 'http://localhost/worker.js', kind: 'error', name: 'Error', + url: 'http://localhost/chunk.js', lineno: 12, colno: 9, }, @@ -900,20 +909,22 @@ describe('forwarded worker errors', () => { }); }); - it('adds a frame from the error location when a message-only error has no stack', () => { + it.each([ + ['the script that threw', 'http://localhost/chunk.js', 'http://localhost/chunk.js'], + ['the worker script when the event has no url', undefined, 'http://localhost/worker.js'], + ])('adds a frame at %s when a message-only error has no stack', (_, url, frameFilename) => { forward({ reason: 'Uncaught Error: boom', kind: 'error', filename: 'http://localhost/worker.js', + url, lineno: 12, colno: 9, }); expectCapturedException({ value: 'Uncaught Error: boom', - stacktrace: { - frames: [expect.objectContaining({ filename: 'http://localhost/worker.js', lineno: 12, colno: 9 })], - }, + stacktrace: { frames: [expect.objectContaining({ filename: frameFilename, lineno: 12, colno: 9 })] }, }); }); From c845e9da7ac56a476a5df58dfaf426d8ff263d05 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 11 Sep 2026 14:15:59 +0200 Subject: [PATCH 05/11] fix(browser): Treat an empty ErrorEvent filename as unknown An ErrorEvent reports an unknown script as an empty string, which skipped the worker script and left the fallback frame on the page URL. --- packages/browser/src/integrations/webWorker.ts | 3 ++- packages/browser/test/integrations/webWorker.test.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index 2d09106ba80e..2e08fa054e9a 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -229,7 +229,8 @@ function handleForwardedWorkerError(workerError: SerializedWorkerError): void { : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, isUnhandledRejection); if (!isUnhandledRejection) { - _enhanceEventWithInitialFrame(event, url ?? filename, lineno, colno); + // An ErrorEvent reports an unknown script as an empty string. + _enhanceEventWithInitialFrame(event, url || filename, lineno, colno); } event.level = 'error'; diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index 836ecb165ec7..96b3f89462c6 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -912,6 +912,7 @@ describe('forwarded worker errors', () => { it.each([ ['the script that threw', 'http://localhost/chunk.js', 'http://localhost/chunk.js'], ['the worker script when the event has no url', undefined, 'http://localhost/worker.js'], + ['the worker script when the event url is empty', '', 'http://localhost/worker.js'], ])('adds a frame at %s when a message-only error has no stack', (_, url, frameFilename) => { forward({ reason: 'Uncaught Error: boom', From 37dd41bd16f08871db6f0ce60271d0f906fd0950 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 11 Sep 2026 14:55:52 +0200 Subject: [PATCH 06/11] fix(browser): Retry a failed worker forward with plain data The page suppresses the bubbled copy of every error once the worker has announced itself, so a retry that posts another Error loses the error completely in browsers that cannot clone Error at all. The retry now sends a plain message and stack copy and the page rebuilds the Error. --- .../browser/src/integrations/webWorker.ts | 29 +++++++----- .../test/integrations/webWorker.test.ts | 44 +++++++++++++++++-- 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index 2e08fa054e9a..72748a01a450 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -44,6 +44,8 @@ interface SerializedWorkerError { url?: string; lineno?: number; colno?: number; + /** Set when `reason` is a plain `{ message, stack }` copy of an error that did not clone. */ + plainError?: boolean; } interface WebWorkerIntegrationOptions { @@ -213,10 +215,12 @@ function handleForwardedWorkerError(workerError: SerializedWorkerError): void { const { stackParser, attachStacktrace } = client.getOptions(); - const { reason: error, kind, name, filename, url, lineno, colno } = workerError; + const { reason, kind, name, filename, url, lineno, colno, plainError } = workerError; // Older workers only ever forwarded rejections and send no `kind`. const isUnhandledRejection = kind !== 'error'; + const error = plainError && isPlainObject(reason) ? errorFromPlain(reason) : reason; + if (name && isError(error) && error.name !== name) { addNonEnumerableProperty(error, 'name', name); } @@ -354,8 +358,10 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { /** * `postMessage` structured-clones the reason. A `DataCloneError` must never - * escape the worker's own error handler, so the forward is retried with a - * cloneable stand-in that keeps as much of the original as possible. + * escape the worker's own error handler, so the forward is retried with + * plain data. The page suppresses the bubbled copy of every error once the + * worker has announced itself, so the retry must clone in every browser, + * including ones that cannot clone `Error` at all. */ function postSerializedWorkerError( self: MinimalDedicatedWorkerGlobalScope, @@ -368,28 +374,27 @@ function postSerializedWorkerError( }); return; } catch { - // Not cloneable, fall through and send a stand-in instead. + // Not cloneable, fall through and send plain data instead. } const { reason } = serializedError; - // A fresh Error keeps message and stack but drops the `cause` that blocked - // the clone. `normalize` only produces cloneable output for everything else. - const cloneableReason = isError(reason) ? cloneableErrorFrom(reason) : normalize(reason); + const plainError = isError(reason); + const plainReason = plainError ? { message: extractMessage(reason), stack: reason.stack } : normalize(reason); try { self.postMessage({ _sentryMessage: true, - _sentryWorkerError: { ...serializedError, reason: cloneableReason }, + _sentryWorkerError: { ...serializedError, reason: plainReason, plainError }, }); } catch { // Dropping the forward is better than throwing out of the worker's error handler. } } -function cloneableErrorFrom(error: Error): Error { - const clone = new Error(extractMessage(error)); - clone.stack = error.stack; - return clone; +function errorFromPlain(plain: Record): Error { + const error = new Error(String(plain.message)); + error.stack = typeof plain.stack === 'string' ? plain.stack : undefined; + return error; } function isSentryMessage(eventData: unknown): eventData is WebWorkerMessage { diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index 96b3f89462c6..6d1dc4cc4997 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -5,6 +5,7 @@ import * as SentryCore from '@sentry/core'; import type { MockInstance } from 'vitest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { isError } from '@sentry/core'; import { BrowserClient } from '../../src/client'; import * as helpers from '../../src/helpers'; import { INTEGRATION_NAME, registerWebWorker, webWorkerIntegration } from '../../src/integrations/webWorker'; @@ -628,7 +629,7 @@ describe('registerWebWorker', () => { mockWorkerSelf.postMessage.mockImplementation(message => structuredClone(message)); }); - it('retries with a fresh error that keeps message and stack but drops the cause', () => { + it('retries with a plain copy that keeps message and stack but drops the cause', () => { registerWebWorker({ self: mockWorkerSelf as any }); const error = new Error('boom') as Error & { cause?: unknown }; @@ -641,13 +642,34 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, _sentryWorkerError: expect.objectContaining({ - reason: expect.objectContaining({ message: 'boom', stack: error.stack }), + reason: { message: 'boom', stack: error.stack }, + plainError: true, name: 'Error', kind: 'error', }), }); }); + it('sends only plain data when the browser cannot clone errors at all', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + mockWorkerSelf.postMessage.mockImplementation(message => { + if (isError(message._sentryWorkerError?.reason)) { + throw new DOMException('could not be cloned', 'DataCloneError'); + } + }); + + const error = new Error('boom'); + trigger('error', { error }); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: expect.objectContaining({ + reason: { message: 'boom', stack: error.stack }, + plainError: true, + }), + }); + }); + it('keeps the message and stack of a WebAssembly.Exception', () => { registerWebWorker({ self: mockWorkerSelf as any }); @@ -658,7 +680,8 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, _sentryWorkerError: expect.objectContaining({ - reason: expect.objectContaining({ message: 'wasm exception', stack: exception.stack }), + reason: { message: 'wasm exception', stack: exception.stack }, + plainError: true, name: 'WebAssembly.Exception', }), }); @@ -673,6 +696,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryWorkerError: expect.objectContaining({ reason: { retry: '[Function: retry]' }, + plainError: false, kind: 'unhandledrejection', }), }); @@ -909,6 +933,20 @@ describe('forwarded worker errors', () => { }); }); + it('rebuilds an error from a plain copy and restores its name', () => { + const stack = ['RuntimeError: divide by zero', ' at runStepGame (http://localhost:8080/worker.js:12:9)'].join( + '\n', + ); + + forward({ reason: { message: 'divide by zero', stack }, plainError: true, name: 'RuntimeError', kind: 'error' }); + + expectCapturedException({ + type: 'RuntimeError', + value: 'divide by zero', + stacktrace: { frames: [expect.objectContaining({ filename: 'http://localhost:8080/worker.js', lineno: 12 })] }, + }); + }); + it.each([ ['the script that threw', 'http://localhost/chunk.js', 'http://localhost/chunk.js'], ['the worker script when the event has no url', undefined, 'http://localhost/worker.js'], From 7e4d6b967cbd3bc1909fc981f735783c616ff0d8 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 17 Sep 2026 11:51:30 +0100 Subject: [PATCH 07/11] 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. --- .../browser-webworker-vite/index.html | 3 + .../browser-webworker-vite/src/main.ts | 14 +++ .../browser-webworker-vite/src/worker.ts | 5 + .../tests/errors.test.ts | 37 +++++++- .../browser/src/integrations/webWorker.ts | 90 +++++++++--------- .../test/integrations/webWorker.test.ts | 93 ++++++++++++++----- 6 files changed, 175 insertions(+), 67 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html index 0ebc79719432..5dd7ddcc323e 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html @@ -17,5 +17,8 @@ + diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts index 238ec062663a..2f8f9b83c07e 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts @@ -23,6 +23,14 @@ const worker2 = new MyWorker2(); const webWorkerIntegration = Sentry.webWorkerIntegration({ worker: [worker, worker2] }); Sentry.addIntegration(webWorkerIntegration); +worker.addEventListener('error', event => { + // this is part of the test, do not delete + (window as any).workerErrorEvents = [ + ...((window as any).workerErrorEvents ?? []), + { message: event.message, hasError: !!event.error }, + ]; +}); + worker.addEventListener('message', event => { // this is part of the test, do not delete console.log('received message from worker:', event.data.msg); @@ -34,6 +42,12 @@ document.querySelector('#trigger-error')!.addEventListener('c }); }); +document.querySelector('#trigger-primitive-error')!.addEventListener('click', () => { + worker.postMessage({ + msg: 'TRIGGER_PRIMITIVE_ERROR', + }); +}); + document.querySelector('#trigger-error-2')!.addEventListener('click', () => { worker2.postMessage({ msg: 'TRIGGER_ERROR', diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker.ts index 6ed994e9006b..7d37932d7302 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker.ts @@ -12,4 +12,9 @@ self.addEventListener('message', event => { // This will throw an uncaught error in the worker throw new Error(`Uncaught error in worker`); } + + if (event.data.msg === 'TRIGGER_PRIMITIVE_ERROR') { + // A thrown primitive has no stack, so only the ErrorEvent knows where it came from + throw 'Primitive thrown in worker'; + } }); diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts index 8a6f4ab60910..5e0e2d4f4ae1 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts @@ -87,15 +87,46 @@ test('emits exactly one event for an uncaught worker error', async ({ page }) => await page.locator('#trigger-error').click(); await firstErrorPromise; - // The bubbled copy of the first throw is queued right behind the forwarded - // one, so the second worker's event arriving without it in between is the - // signal that it was suppressed. + // The worker cancels the native error event, so page listeners on the + // worker object only see the replayed one, which carries the error object. + expect(await page.evaluate(() => (window as any).workerErrorEvents)).toEqual([ + { message: 'Uncaught Error: Uncaught error in worker', hasError: true }, + ]); + + // A bubbled copy of the first throw would have been reported before the + // second worker's event, so its absence here shows it never happened. await page.locator('#trigger-error-2').click(); await secondErrorPromise; expect(mechanisms).toEqual([WORKER_MECHANISM, WORKER_MECHANISM]); }); +test('locates a thrown primitive by its ErrorEvent position', async ({ page }) => { + const errorEventPromise = waitForError('browser-webworker-vite', event => { + return event.exception?.values?.[0]?.value === 'Primitive thrown in worker'; + }); + + await page.goto('/'); + + await page.locator('#trigger-primitive-error').click(); + + const errorEvent = await errorEventPromise; + const exception = errorEvent.exception?.values?.[0]; + + expect(exception?.mechanism?.type).toBe(WORKER_MECHANISM); + expect(exception?.stacktrace?.frames).toEqual([ + { + filename: expect.stringMatching(/worker-.+\.js$/), + lineno: expect.any(Number), + colno: expect.any(Number), + function: '?', + in_app: true, + }, + ]); + expect(exception?.stacktrace?.frames?.[0]?.lineno).toBeGreaterThan(0); + expect(exception?.stacktrace?.frames?.[0]?.colno).toBeGreaterThan(0); +}); + test("user worker message handlers don't trigger for sentry messages", async ({ page }) => { const workerReadyPromise = new Promise(resolve => { let workerMessageCount = 0; diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index 72748a01a450..f84b83f92ea6 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -2,6 +2,7 @@ import type { DebugImage, Integration, IntegrationFn } from '@sentry/core'; import { addNonEnumerableProperty, captureEvent, + consoleSandbox, debug, defineIntegration, getClient, @@ -12,7 +13,7 @@ import { } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; import { eventFromUnknownInput, extractMessage, extractType } from '../eventbuilder'; -import { ignoreNextOnError, WINDOW } from '../helpers'; +import { WINDOW } from '../helpers'; import { _enhanceEventWithInitialFrame, _eventFromRejectionWithPrimitive, @@ -27,8 +28,6 @@ interface WebWorkerMessage { _sentryModuleMetadata?: Record; // eslint-disable-line @typescript-eslint/no-explicit-any _sentryWorkerError?: SerializedWorkerError; _sentryWasmImages?: Array; - /** Sent by workers that forward uncaught errors, not only rejections. */ - _sentryForwardsErrors?: boolean; } type WorkerErrorKind = 'error' | 'unhandledrejection'; @@ -40,6 +39,8 @@ interface SerializedWorkerError { kind?: WorkerErrorKind; /** Structured clone resets any name outside the built-in set to `Error`. */ name?: string; + /** The `ErrorEvent` message, replayed on the worker object for page listeners. */ + message?: string; /** Script the error was thrown in, which can differ from the worker script. */ url?: string; lineno?: number; @@ -138,27 +139,10 @@ export const webWorkerIntegration = defineIntegration(({ worker }: WebWorkerInte })) as IntegrationFn; function listenForSentryMessages(worker: Worker): void { - let forwardsErrors = false; - - // An uncaught worker error fires `error` on the worker object and, unless - // cancelled, is then reported to `window.onerror` in the same task. The - // worker already forwarded it with a real stack, so the global handler - // must skip the message-only copy. Not cancelling keeps the browser's own - // console report. - worker.addEventListener('error', () => { - if (forwardsErrors) { - ignoreNextOnError(); - } - }); - worker.addEventListener('message', event => { if (isSentryMessage(event.data)) { event.stopImmediatePropagation(); // other listeners should not receive this message - if (event.data._sentryForwardsErrors) { - forwardsErrors = true; - } - // Handle debug IDs if (event.data._sentryDebugIds) { DEBUG_BUILD && debug.log('Sentry debugId web worker message received', event.data); @@ -201,21 +185,14 @@ function listenForSentryMessages(worker: Worker): void { // Handle errors and unhandled rejections forwarded from worker if (event.data._sentryWorkerError) { DEBUG_BUILD && debug.log('Sentry worker error message received', event.data._sentryWorkerError); - handleForwardedWorkerError(event.data._sentryWorkerError); + handleForwardedWorkerError(worker, event.data._sentryWorkerError); } } }); } -function handleForwardedWorkerError(workerError: SerializedWorkerError): void { - const client = getClient(); - if (!client) { - return; - } - - const { stackParser, attachStacktrace } = client.getOptions(); - - const { reason, kind, name, filename, url, lineno, colno, plainError } = workerError; +function handleForwardedWorkerError(worker: Worker, workerError: SerializedWorkerError): void { + const { reason, kind, name, filename, url, lineno, colno, plainError, message } = workerError; // Older workers only ever forwarded rejections and send no `kind`. const isUnhandledRejection = kind !== 'error'; @@ -225,6 +202,21 @@ function handleForwardedWorkerError(workerError: SerializedWorkerError): void { addNonEnumerableProperty(error, 'name', name); } + // The worker cancelled its native error event, which also silences + // `error` listeners on the worker object in the page. Replay it for them + // with the error object the native event never carries. A dispatched + // event has no default action, so it does not reach `window.onerror`. + if (!isUnhandledRejection && typeof ErrorEvent === 'function') { + worker.dispatchEvent(new ErrorEvent('error', { message, filename: url || filename, lineno, colno, error })); + } + + const client = getClient(); + if (!client) { + return; + } + + const { stackParser, attachStacktrace } = client.getOptions(); + // Follow same pattern as globalHandlers for each source. // A thrown primitive is not a rejection, so the rejection-specific wording must not apply to it. const event = @@ -319,33 +311,46 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { _sentryMessage: true, _sentryDebugIds: self._sentryDebugIds ?? undefined, _sentryModuleMetadata: self._sentryModuleMetadata ?? undefined, - _sentryForwardsErrors: true, }); - const forward = (serializedError: Omit): void => { + const forward = (serializedError: Omit): boolean => { const { reason } = serializedError; - postSerializedWorkerError(self, { + DEBUG_BUILD && debug.log(`[Sentry Worker] Forwarding ${serializedError.kind} to parent`, serializedError); + + return postSerializedWorkerError(self, { ...serializedError, filename: self.location?.href, name: isError(reason) ? extractType(reason) : undefined, }); - - DEBUG_BUILD && debug.log(`[Sentry Worker] Forwarding ${serializedError.kind} to parent`, serializedError); }; // Uncaught errors bubble to the parent, but the propagated ErrorEvent // carries no error object. Forwarding the object keeps the real stack. self.addEventListener('error', (event: unknown) => { - const { error, message, filename, lineno, colno } = event as { + const errorEvent = event as { error?: unknown; message?: string; filename?: string; lineno?: number; colno?: number; + preventDefault?: () => void; }; + const { error, message, filename, lineno, colno } = errorEvent; + const reason = error ?? message; + + if (!forward({ kind: 'error', reason, message, url: filename, lineno, colno })) { + return; + } - forward({ kind: 'error', reason: error ?? message, url: filename, lineno, colno }); + // The page now has the error with its stack, so the message-only copy + // must not bubble there as well. A cancelled error prints nothing, so + // log it to keep it visible in DevTools. + errorEvent.preventDefault?.(); + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.error(reason); + }); }); // Unhandled rejections do not bubble to the parent thread at all. @@ -359,20 +364,19 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { /** * `postMessage` structured-clones the reason. A `DataCloneError` must never * escape the worker's own error handler, so the forward is retried with - * plain data. The page suppresses the bubbled copy of every error once the - * worker has announced itself, so the retry must clone in every browser, - * including ones that cannot clone `Error` at all. + * plain data that clones in every browser, including ones that cannot clone + * `Error` at all. Returns whether the page received the error. */ function postSerializedWorkerError( self: MinimalDedicatedWorkerGlobalScope, serializedError: SerializedWorkerError, -): void { +): boolean { try { self.postMessage({ _sentryMessage: true, _sentryWorkerError: serializedError, }); - return; + return true; } catch { // Not cloneable, fall through and send plain data instead. } @@ -386,8 +390,10 @@ function postSerializedWorkerError( _sentryMessage: true, _sentryWorkerError: { ...serializedError, reason: plainReason, plainError }, }); + return true; } catch { // Dropping the forward is better than throwing out of the worker's error handler. + return false; } } diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index 6d1dc4cc4997..1a2189a43fa0 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -32,7 +32,6 @@ vi.mock('../../src/helpers', () => ({ WINDOW: { _sentryDebugIds: undefined, }, - ignoreNextOnError: vi.fn(), })); function getListener(addEventListener: ReturnType, type: string): (event: any) => void { @@ -48,12 +47,14 @@ describe('webWorkerIntegration', () => { let mockWorker: { addEventListener: ReturnType; + dispatchEvent: ReturnType; postMessage: ReturnType; _sentryDebugIds?: Record; }; let mockWorker2: { addEventListener: ReturnType; + dispatchEvent: ReturnType; postMessage: ReturnType; _sentryDebugIds?: Record; }; @@ -72,11 +73,13 @@ describe('webWorkerIntegration', () => { // Setup mock worker mockWorker = { addEventListener: vi.fn(), + dispatchEvent: vi.fn(), postMessage: vi.fn(), }; mockWorker2 = { addEventListener: vi.fn(), + dispatchEvent: vi.fn(), postMessage: vi.fn(), }; @@ -417,6 +420,7 @@ describe('registerWebWorker', () => { let mockWorkerSelf: { postMessage: ReturnType; addEventListener: ReturnType; + dispatchEvent: ReturnType; _sentryDebugIds?: Record; _sentryModuleMetadata?: Record; location?: { href?: string }; @@ -431,6 +435,7 @@ describe('registerWebWorker', () => { mockWorkerSelf = { postMessage: vi.fn(), addEventListener: vi.fn(), + dispatchEvent: vi.fn(), }; }); @@ -446,7 +451,6 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); }); @@ -466,7 +470,6 @@ describe('registerWebWorker', () => { 'worker-file2.js': 'debug-id-2', }, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); }); @@ -480,7 +483,6 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); }); @@ -498,7 +500,6 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: rawMetadata, - _sentryForwardsErrors: true, }); }); @@ -511,7 +512,6 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); }); @@ -533,13 +533,24 @@ describe('registerWebWorker', () => { 'worker-file.js': 'debug-id-1', }, _sentryModuleMetadata: rawMetadata, - _sentryForwardsErrors: true, }); }); describe('error forwarding', () => { - function trigger(type: string, event: unknown): void { - getListener(mockWorkerSelf.addEventListener, type)(event); + let consoleErrorSpy: MockInstance; + + beforeEach(() => { + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + }); + + function trigger(type: string, event: Record): { preventDefault: ReturnType } { + const fullEvent = { preventDefault: vi.fn(), ...event }; + getListener(mockWorkerSelf.addEventListener, type)(fullEvent); + return fullEvent; } it('raises the stack trace limit so forwarded stacks are not truncated', () => { @@ -555,7 +566,7 @@ describe('registerWebWorker', () => { mockWorkerSelf.location = { href: 'http://localhost/worker.js' }; const error = new Error('boom'); - trigger('error', { + const event = trigger('error', { error, message: 'Uncaught Error: boom', filename: 'http://localhost/chunk.js', @@ -570,11 +581,26 @@ describe('registerWebWorker', () => { filename: 'http://localhost/worker.js', kind: 'error', name: 'Error', + message: 'Uncaught Error: boom', url: 'http://localhost/chunk.js', lineno: 12, colno: 9, }, }); + expect(event.preventDefault).toHaveBeenCalledOnce(); + expect(consoleErrorSpy).toHaveBeenCalledExactlyOnceWith(error); + }); + + it('lets the error bubble when the forward failed, so the page still reports it', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + mockWorkerSelf.postMessage.mockImplementation(() => { + throw new DOMException('could not be cloned', 'DataCloneError'); + }); + + const event = trigger('error', { error: new Error('boom') }); + + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(consoleErrorSpy).not.toHaveBeenCalled(); }); it('sends the error name separately because structured clone resets it', () => { @@ -611,7 +637,8 @@ describe('registerWebWorker', () => { registerWebWorker({ self: mockWorkerSelf as any }); const reason = new Error('rejected'); - trigger('unhandledrejection', { reason }); + const event = trigger('unhandledrejection', { reason }); + expect(event.preventDefault).not.toHaveBeenCalled(); expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, @@ -780,7 +807,6 @@ describe('registerWebWorker and webWorkerIntegration', () => { _sentryMessage: true, _sentryDebugIds: mockWorker._sentryDebugIds, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); expect((helpers.WINDOW as any)._sentryDebugIds).toEqual({ @@ -802,7 +828,6 @@ describe('registerWebWorker and webWorkerIntegration', () => { _sentryMessage: true, _sentryDebugIds: mockWorker3._sentryDebugIds, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); expect((helpers.WINDOW as any)._sentryDebugIds).toEqual({ @@ -822,7 +847,11 @@ describe('registerWebWorker and webWorkerIntegration', () => { describe('forwarded worker errors', () => { let client: BrowserClient; let captureEventSpy: MockInstance; - let mockWorker: { addEventListener: ReturnType; postMessage: ReturnType }; + let mockWorker: { + addEventListener: ReturnType; + dispatchEvent: ReturnType; + postMessage: ReturnType; + }; beforeEach(() => { vi.clearAllMocks(); @@ -835,7 +864,7 @@ describe('forwarded worker errors', () => { client.init(); captureEventSpy = vi.spyOn(client, 'captureEvent'); - mockWorker = { addEventListener: vi.fn(), postMessage: vi.fn() }; + mockWorker = { addEventListener: vi.fn(), dispatchEvent: vi.fn(), postMessage: vi.fn() }; const integration = webWorkerIntegration({ worker: mockWorker as any }); integration.setupOnce!(); }); @@ -967,14 +996,34 @@ describe('forwarded worker errors', () => { }); }); - it('skips the global onerror copy only once the worker announced that it forwards errors', () => { - const onWorkerError = getListener(mockWorker.addEventListener, 'error'); + it('replays a forwarded error on the worker object with the error object attached', () => { + const error = new Error('boom'); + + forward({ + reason: error, + kind: 'error', + message: 'Uncaught Error: boom', + filename: 'http://localhost/worker.js', + url: 'http://localhost/chunk.js', + lineno: 12, + colno: 9, + }); + + expect(mockWorker.dispatchEvent).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + type: 'error', + message: 'Uncaught Error: boom', + filename: 'http://localhost/chunk.js', + lineno: 12, + colno: 9, + error, + }), + ); + }); - onWorkerError({}); - expect(helpers.ignoreNextOnError).not.toHaveBeenCalled(); + it('does not replay a forwarded rejection, which never fires on the worker object', () => { + forward({ reason: new Error('rejected'), kind: 'unhandledrejection' }); - receive({ _sentryDebugIds: undefined, _sentryForwardsErrors: true }); - onWorkerError({}); - expect(helpers.ignoreNextOnError).toHaveBeenCalledTimes(1); + expect(mockWorker.dispatchEvent).not.toHaveBeenCalled(); }); }); From 615cd67cde94de48e914878af87ba9dba777ba07 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 21 Sep 2026 14:57:19 +0100 Subject: [PATCH 08/11] fix(browser): Cancel worker errors only after the page acknowledged A successful postMessage only proves the forward was queued. A worker that the page never added to the integration, or a page on an older bundle, would have lost the error entirely once the worker cancelled it. The page now replies to the worker's announce, and the worker cancels its error events only once it has that reply. Older workers never sent the capability, so they never receive the reply either. --- .../browser/src/integrations/webWorker.ts | 34 ++++++- .../test/integrations/webWorker.test.ts | 98 +++++++++++++++---- 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index f84b83f92ea6..32aecd9d9b85 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -28,6 +28,10 @@ interface WebWorkerMessage { _sentryModuleMetadata?: Record; // eslint-disable-line @typescript-eslint/no-explicit-any _sentryWorkerError?: SerializedWorkerError; _sentryWasmImages?: Array; + /** Sent with the announce by workers that forward uncaught errors and wait for the reply below. */ + _sentryForwardsErrors?: boolean; + /** The page's reply: it captures and replays forwarded errors, so the worker may cancel them. */ + _sentryHandlesForwardedErrors?: boolean; } type WorkerErrorKind = 'error' | 'unhandledrejection'; @@ -143,6 +147,14 @@ function listenForSentryMessages(worker: Worker): void { if (isSentryMessage(event.data)) { event.stopImmediatePropagation(); // other listeners should not receive this message + // A worker must not cancel its error events until it knows this page + // captures and replays them, or an unregistered worker and an older + // page bundle would lose them. Only workers that declared the + // capability get the reply, so older workers never see it. + if (event.data._sentryForwardsErrors) { + worker.postMessage({ _sentryMessage: true, _sentryHandlesForwardedErrors: true }); + } + // Handle debug IDs if (event.data._sentryDebugIds) { DEBUG_BUILD && debug.log('Sentry debugId web worker message received', event.data); @@ -287,6 +299,10 @@ interface RegisterWebWorkerOptions { * no `error` object, so globalHandlers can only build an event from the message string. * Forwarding them here preserves the real stack, which matters most for wasm frames. * + * Call this before the worker registers its own `message` handlers. The page replies + * with one message that this function consumes, so handlers registered earlier would + * receive it. + * * @example * ```ts filename={worker.js} * import * as Sentry from '@sentry/'; @@ -305,12 +321,24 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { // V8's default of 10 truncates stacks before this code forwards them. Error.stackTraceLimit = 50; + let pageHandlesForwardedErrors = false; + + // Registered before the announce so the reply cannot arrive first. + self.addEventListener('message', (event: unknown) => { + const { data, stopImmediatePropagation } = event as { data?: unknown; stopImmediatePropagation?: () => void }; + if (isPlainObject(data) && data._sentryMessage === true && data._sentryHandlesForwardedErrors === true) { + pageHandlesForwardedErrors = true; + stopImmediatePropagation?.call(event); + } + }); + // Send debug IDs and raw module metadata to parent thread // The metadata will be parsed lazily on the main thread when needed self.postMessage({ _sentryMessage: true, _sentryDebugIds: self._sentryDebugIds ?? undefined, _sentryModuleMetadata: self._sentryModuleMetadata ?? undefined, + _sentryForwardsErrors: true, }); const forward = (serializedError: Omit): boolean => { @@ -339,7 +367,11 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { const { error, message, filename, lineno, colno } = errorEvent; const reason = error ?? message; - if (!forward({ kind: 'error', reason, message, url: filename, lineno, colno })) { + const forwarded = forward({ kind: 'error', reason, message, url: filename, lineno, colno }); + + // Until the page confirmed it handles forwarded errors, the bubbled copy + // is the only report that is sure to reach it. + if (!forwarded || !pageHandlesForwardedErrors) { return; } diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index 1a2189a43fa0..f96586414bce 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -451,6 +451,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); }); @@ -470,6 +471,7 @@ describe('registerWebWorker', () => { 'worker-file2.js': 'debug-id-2', }, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); }); @@ -483,6 +485,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); }); @@ -500,6 +503,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: rawMetadata, + _sentryForwardsErrors: true, }); }); @@ -512,6 +516,7 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); }); @@ -533,6 +538,7 @@ describe('registerWebWorker', () => { 'worker-file.js': 'debug-id-1', }, _sentryModuleMetadata: rawMetadata, + _sentryForwardsErrors: true, }); }); @@ -553,6 +559,40 @@ describe('registerWebWorker', () => { return fullEvent; } + function receiveFromPage(data: unknown): { stopImmediatePropagation: ReturnType } { + const messageEvent = { data, stopImmediatePropagation: vi.fn() }; + getListener(mockWorkerSelf.addEventListener, 'message')(messageEvent); + return messageEvent; + } + + function acknowledge(): void { + receiveFromPage({ _sentryMessage: true, _sentryHandlesForwardedErrors: true }); + } + + it('consumes the acknowledgement from the page and leaves other messages alone', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + const ack = receiveFromPage({ _sentryMessage: true, _sentryHandlesForwardedErrors: true }); + const other = receiveFromPage({ msg: 'WORKER_READY' }); + + expect(ack.stopImmediatePropagation).toHaveBeenCalledOnce(); + expect(other.stopImmediatePropagation).not.toHaveBeenCalled(); + }); + + it('forwards an uncaught error but does not cancel it before the page acknowledged', () => { + registerWebWorker({ self: mockWorkerSelf as any }); + + const error = new Error('boom'); + const event = trigger('error', { error }); + + expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ + _sentryMessage: true, + _sentryWorkerError: expect.objectContaining({ reason: error, kind: 'error' }), + }); + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + it('raises the stack trace limit so forwarded stacks are not truncated', () => { Error.stackTraceLimit = 10; @@ -563,6 +603,7 @@ describe('registerWebWorker', () => { it('forwards an uncaught error with its location, name and kind "error"', () => { registerWebWorker({ self: mockWorkerSelf as any }); + acknowledge(); mockWorkerSelf.location = { href: 'http://localhost/worker.js' }; const error = new Error('boom'); @@ -593,6 +634,7 @@ describe('registerWebWorker', () => { it('lets the error bubble when the forward failed, so the page still reports it', () => { registerWebWorker({ self: mockWorkerSelf as any }); + acknowledge(); mockWorkerSelf.postMessage.mockImplementation(() => { throw new DOMException('could not be cloned', 'DataCloneError'); }); @@ -751,9 +793,15 @@ describe('registerWebWorker and webWorkerIntegration', () => { 'Error at \n /shared-file.js': 'main-debug-id', }; - let cb1: ((arg0: any) => any) | undefined = undefined; - let cb2: ((arg0: any) => any) | undefined = undefined; - let cb3: ((arg0: any) => any) | undefined = undefined; + // Each mock stands in for both the worker and its `self`, so messages + // posted either way reach every message listener registered on it. + type Listeners = Record any>>; + const listeners1: Listeners = {}; + const listeners2: Listeners = {}; + const listeners3: Listeners = {}; + const deliver = (listeners: Listeners, message: unknown): void => { + (listeners.message ?? []).forEach(listener => listener({ data: message, stopImmediatePropagation: vi.fn() })); + }; // Setup mock worker const mockWorker = { @@ -762,11 +810,10 @@ describe('registerWebWorker and webWorkerIntegration', () => { 'Error at \n /worker-file2.js': 'worker-debug-2', 'Error at \n /shared-file.js': 'worker-debug-id', }, - addEventListener: vi.fn((_, l) => (cb1 = l)), - postMessage: vi.fn(message => { - // @ts-expect-error - cb is defined - cb1({ data: message, stopImmediatePropagation: vi.fn() }); - }), + addEventListener: vi.fn( + (type: string, l: (arg0: any) => any) => (listeners1[type] = [...(listeners1[type] ?? []), l]), + ), + postMessage: vi.fn(message => deliver(listeners1, message)), }; const mockWorker2 = { @@ -775,11 +822,10 @@ describe('registerWebWorker and webWorkerIntegration', () => { 'Error at \n /worker-2-file2.js': 'worker-2-debug-2', }, - addEventListener: vi.fn((_, l) => (cb2 = l)), - postMessage: vi.fn(message => { - // @ts-expect-error - cb is defined - cb2({ data: message, stopImmediatePropagation: vi.fn() }); - }), + addEventListener: vi.fn( + (type: string, l: (arg0: any) => any) => (listeners2[type] = [...(listeners2[type] ?? []), l]), + ), + postMessage: vi.fn(message => deliver(listeners2, message)), }; const mockWorker3 = { @@ -787,11 +833,10 @@ describe('registerWebWorker and webWorkerIntegration', () => { 'Error at \n /worker-3-file1.js': 'worker-3-debug-1', 'Error at \n /worker-3-file2.js': 'worker-3-debug-2', }, - addEventListener: vi.fn((_, l) => (cb3 = l)), - postMessage: vi.fn(message => { - // @ts-expect-error - cb is defined - cb3({ data: message, stopImmediatePropagation: vi.fn() }); - }), + addEventListener: vi.fn( + (type: string, l: (arg0: any) => any) => (listeners3[type] = [...(listeners3[type] ?? []), l]), + ), + postMessage: vi.fn(message => deliver(listeners3, message)), }; const integration = webWorkerIntegration({ worker: [mockWorker as any, mockWorker2 as any] }); @@ -807,6 +852,7 @@ describe('registerWebWorker and webWorkerIntegration', () => { _sentryMessage: true, _sentryDebugIds: mockWorker._sentryDebugIds, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); expect((helpers.WINDOW as any)._sentryDebugIds).toEqual({ @@ -828,6 +874,7 @@ describe('registerWebWorker and webWorkerIntegration', () => { _sentryMessage: true, _sentryDebugIds: mockWorker3._sentryDebugIds, _sentryModuleMetadata: undefined, + _sentryForwardsErrors: true, }); expect((helpers.WINDOW as any)._sentryDebugIds).toEqual({ @@ -996,6 +1043,21 @@ describe('forwarded worker errors', () => { }); }); + it('acknowledges a worker that declared it forwards errors', () => { + receive({ _sentryDebugIds: undefined, _sentryForwardsErrors: true }); + + expect(mockWorker.postMessage).toHaveBeenCalledExactlyOnceWith({ + _sentryMessage: true, + _sentryHandlesForwardedErrors: true, + }); + }); + + it('does not message a worker registered by an older SDK, whose handlers would receive it', () => { + receive({ _sentryDebugIds: undefined }); + + expect(mockWorker.postMessage).not.toHaveBeenCalled(); + }); + it('replays a forwarded error on the worker object with the error object attached', () => { const error = new Error('boom'); From e0333b9244a0b60d2fd4a4e5d8822fbbf1a63984 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 21 Sep 2026 15:56:25 +0100 Subject: [PATCH 09/11] fix(browser): Replay only cancelled worker errors and acknowledge late workers The page replayed every forwarded error, so before the acknowledgement arrived listeners on the worker object ran twice, once for the native event and once for the replay. The worker now marks each forwarded error with whether it cancelled the native event, and the page replays only those. The capability travelled only on the one-shot announce, so a worker added after it was never acknowledged and kept bubbling for good. Every worker message now carries it, and the page acknowledges on the first one it sees. --- .../browser/src/integrations/webWorker.ts | 34 ++++++++++++------- .../test/integrations/webWorker.test.ts | 33 ++++++++++++++++-- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index 32aecd9d9b85..9dec09443eef 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -28,7 +28,7 @@ interface WebWorkerMessage { _sentryModuleMetadata?: Record; // eslint-disable-line @typescript-eslint/no-explicit-any _sentryWorkerError?: SerializedWorkerError; _sentryWasmImages?: Array; - /** Sent with the announce by workers that forward uncaught errors and wait for the reply below. */ + /** Sent with every message by workers that forward uncaught errors and wait for the reply below. */ _sentryForwardsErrors?: boolean; /** The page's reply: it captures and replays forwarded errors, so the worker may cancel them. */ _sentryHandlesForwardedErrors?: boolean; @@ -51,6 +51,8 @@ interface SerializedWorkerError { colno?: number; /** Set when `reason` is a plain `{ message, stack }` copy of an error that did not clone. */ plainError?: boolean; + /** The worker stopped the native event from bubbling, so the page replays it. */ + cancelled?: boolean; } interface WebWorkerIntegrationOptions { @@ -143,6 +145,8 @@ export const webWorkerIntegration = defineIntegration(({ worker }: WebWorkerInte })) as IntegrationFn; function listenForSentryMessages(worker: Worker): void { + let acknowledged = false; + worker.addEventListener('message', event => { if (isSentryMessage(event.data)) { event.stopImmediatePropagation(); // other listeners should not receive this message @@ -150,8 +154,11 @@ function listenForSentryMessages(worker: Worker): void { // A worker must not cancel its error events until it knows this page // captures and replays them, or an unregistered worker and an older // page bundle would lose them. Only workers that declared the - // capability get the reply, so older workers never see it. - if (event.data._sentryForwardsErrors) { + // capability get the reply, so older workers never see it. Every + // worker message carries it, so a worker added after its announce is + // acknowledged on its first forwarded error. + if (event.data._sentryForwardsErrors && !acknowledged) { + acknowledged = true; worker.postMessage({ _sentryMessage: true, _sentryHandlesForwardedErrors: true }); } @@ -204,7 +211,7 @@ function listenForSentryMessages(worker: Worker): void { } function handleForwardedWorkerError(worker: Worker, workerError: SerializedWorkerError): void { - const { reason, kind, name, filename, url, lineno, colno, plainError, message } = workerError; + const { reason, kind, name, filename, url, lineno, colno, plainError, message, cancelled } = workerError; // Older workers only ever forwarded rejections and send no `kind`. const isUnhandledRejection = kind !== 'error'; @@ -214,11 +221,11 @@ function handleForwardedWorkerError(worker: Worker, workerError: SerializedWorke addNonEnumerableProperty(error, 'name', name); } - // The worker cancelled its native error event, which also silences - // `error` listeners on the worker object in the page. Replay it for them - // with the error object the native event never carries. A dispatched - // event has no default action, so it does not reach `window.onerror`. - if (!isUnhandledRejection && typeof ErrorEvent === 'function') { + // A cancelled native error event also silences `error` listeners on the + // worker object in the page. Replay it for them with the error object the + // native event never carries. A dispatched event has no default action, + // so it does not reach `window.onerror`. + if (cancelled && typeof ErrorEvent === 'function') { worker.dispatchEvent(new ErrorEvent('error', { message, filename: url || filename, lineno, colno, error })); } @@ -367,11 +374,12 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { const { error, message, filename, lineno, colno } = errorEvent; const reason = error ?? message; - const forwarded = forward({ kind: 'error', reason, message, url: filename, lineno, colno }); - // Until the page confirmed it handles forwarded errors, the bubbled copy // is the only report that is sure to reach it. - if (!forwarded || !pageHandlesForwardedErrors) { + const cancelled = pageHandlesForwardedErrors; + const forwarded = forward({ kind: 'error', reason, message, url: filename, lineno, colno, cancelled }); + + if (!forwarded || !cancelled) { return; } @@ -406,6 +414,7 @@ function postSerializedWorkerError( try { self.postMessage({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: serializedError, }); return true; @@ -420,6 +429,7 @@ function postSerializedWorkerError( try { self.postMessage({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: { ...serializedError, reason: plainReason, plainError }, }); return true; diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index f96586414bce..5a90ac57c433 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -587,7 +587,8 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryWorkerError: expect.objectContaining({ reason: error, kind: 'error' }), + _sentryForwardsErrors: true, + _sentryWorkerError: expect.objectContaining({ reason: error, kind: 'error', cancelled: false }), }); expect(event.preventDefault).not.toHaveBeenCalled(); expect(consoleErrorSpy).not.toHaveBeenCalled(); @@ -617,6 +618,7 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: { reason: error, filename: 'http://localhost/worker.js', @@ -626,6 +628,7 @@ describe('registerWebWorker', () => { url: 'http://localhost/chunk.js', lineno: 12, colno: 9, + cancelled: true, }, }); expect(event.preventDefault).toHaveBeenCalledOnce(); @@ -654,6 +657,7 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: error, name: 'RuntimeError' }), }); }); @@ -665,6 +669,7 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: 'Uncaught Error: boom', kind: 'error', @@ -684,6 +689,7 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: { reason, filename: undefined, @@ -710,6 +716,7 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenCalledTimes(3); expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: { message: 'boom', stack: error.stack }, plainError: true, @@ -732,6 +739,7 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: { message: 'boom', stack: error.stack }, plainError: true, @@ -748,6 +756,7 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: { message: 'wasm exception', stack: exception.stack }, plainError: true, @@ -763,6 +772,7 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, + _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: { retry: '[Function: retry]' }, plainError: false, @@ -1043,8 +1053,18 @@ describe('forwarded worker errors', () => { }); }); - it('acknowledges a worker that declared it forwards errors', () => { + it('acknowledges a worker that declared it forwards errors, once', () => { receive({ _sentryDebugIds: undefined, _sentryForwardsErrors: true }); + receive({ _sentryWorkerError: { reason: new Error('boom'), kind: 'error' }, _sentryForwardsErrors: true }); + + expect(mockWorker.postMessage).toHaveBeenCalledExactlyOnceWith({ + _sentryMessage: true, + _sentryHandlesForwardedErrors: true, + }); + }); + + it('acknowledges a worker added after its announce on its first forwarded error', () => { + receive({ _sentryWorkerError: { reason: new Error('boom'), kind: 'error' }, _sentryForwardsErrors: true }); expect(mockWorker.postMessage).toHaveBeenCalledExactlyOnceWith({ _sentryMessage: true, @@ -1058,7 +1078,7 @@ describe('forwarded worker errors', () => { expect(mockWorker.postMessage).not.toHaveBeenCalled(); }); - it('replays a forwarded error on the worker object with the error object attached', () => { + it('replays a cancelled error on the worker object with the error object attached', () => { const error = new Error('boom'); forward({ @@ -1069,6 +1089,7 @@ describe('forwarded worker errors', () => { url: 'http://localhost/chunk.js', lineno: 12, colno: 9, + cancelled: true, }); expect(mockWorker.dispatchEvent).toHaveBeenCalledExactlyOnceWith( @@ -1083,6 +1104,12 @@ describe('forwarded worker errors', () => { ); }); + it('does not replay an error the worker let bubble, since the native event fires for it', () => { + forward({ reason: new Error('boom'), kind: 'error', message: 'Uncaught Error: boom', cancelled: false }); + + expect(mockWorker.dispatchEvent).not.toHaveBeenCalled(); + }); + it('does not replay a forwarded rejection, which never fires on the worker object', () => { forward({ reason: new Error('rejected'), kind: 'unhandledrejection' }); From 55d47a954e393a4ba75f02c354a9f3cf2924154c Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Mon, 21 Sep 2026 12:19:34 -0400 Subject: [PATCH 10/11] fix(browser): Skip the onerror copy only for worker errors that were forwarded The worker no longer cancels its error events or waits for an acknowledgement from the page. The page records each forwarded error and skips the one window.onerror report that matches it, so an error thrown while the worker script first runs is no longer reported twice, and a missing forward can only cause a duplicate, never a lost error. --- .../browser-webworker-vite/index.html | 1 + .../browser-webworker-vite/src/main.ts | 5 + .../browser-webworker-vite/src/worker4.ts | 6 + .../tests/errors.test.ts | 34 ++- packages/browser/src/helpers.ts | 40 ++- .../src/integrations/globalhandlers.ts | 2 +- .../browser/src/integrations/webWorker.ts | 115 +++------ packages/browser/test/helpers.test.ts | 43 +++- .../test/integrations/webWorker.test.ts | 229 +++++------------- 9 files changed, 224 insertions(+), 251 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker4.ts diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html index 5dd7ddcc323e..fea43a70c1de 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/index.html @@ -20,5 +20,6 @@ + diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts index 2f8f9b83c07e..e62becc39603 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/main.ts @@ -54,6 +54,11 @@ document.querySelector('#trigger-error-2')!.addEventListener( }); }); +document.querySelector('#trigger-startup-error')!.addEventListener('click', async () => { + const Worker4 = await import('./worker4.ts?worker'); + webWorkerIntegration.addWorker(new Worker4.default()); +}); + document.querySelector('#trigger-error-3')!.addEventListener('click', async () => { const Worker3 = await import('./worker3.ts?worker'); const worker3 = new Worker3.default(); diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker4.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker4.ts new file mode 100644 index 000000000000..c3fe81ff33ab --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/src/worker4.ts @@ -0,0 +1,6 @@ +import * as Sentry from '@sentry/browser'; + +Sentry.registerWebWorker({ self }); + +// Thrown while the worker script first runs, before the page's acknowledgement can arrive +throw new Error('Uncaught error during worker startup'); diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts index 5e0e2d4f4ae1..d7ab2107ed2d 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts @@ -87,10 +87,9 @@ test('emits exactly one event for an uncaught worker error', async ({ page }) => await page.locator('#trigger-error').click(); await firstErrorPromise; - // The worker cancels the native error event, so page listeners on the - // worker object only see the replayed one, which carries the error object. + // Page listeners on the worker object still get the native event, once. expect(await page.evaluate(() => (window as any).workerErrorEvents)).toEqual([ - { message: 'Uncaught Error: Uncaught error in worker', hasError: true }, + { message: 'Uncaught Error: Uncaught error in worker', hasError: false }, ]); // A bubbled copy of the first throw would have been reported before the @@ -127,6 +126,35 @@ test('locates a thrown primitive by its ErrorEvent position', async ({ page }) = expect(exception?.stacktrace?.frames?.[0]?.colno).toBeGreaterThan(0); }); +test('emits exactly one event for an error thrown during worker startup', async ({ page }) => { + const values: Array = []; + void waitForError('browser-webworker-vite', event => { + const value = event.exception?.values?.[0]?.value; + if (value?.includes('Uncaught error during worker startup')) { + values.push(value); + } + return false; + }); + + const startupErrorPromise = waitForError('browser-webworker-vite', event => { + return !!event.exception?.values?.[0]?.value?.includes('Uncaught error during worker startup'); + }); + const laterErrorPromise = waitForError('browser-webworker-vite', event => { + return event.exception?.values?.[0]?.value === 'Uncaught error in worker'; + }); + + await page.goto('/'); + + await page.locator('#trigger-startup-error').click(); + await startupErrorPromise; + + // Any duplicate of the startup error is sent long before this later error. + await page.locator('#trigger-error').click(); + await laterErrorPromise; + + expect(values).toHaveLength(1); +}); + test("user worker message handlers don't trigger for sentry messages", async ({ page }) => { const workerReadyPromise = new Promise(resolve => { let workerMessageCount = 0; diff --git a/packages/browser/src/helpers.ts b/packages/browser/src/helpers.ts index c961534df2fd..4b060abafc71 100644 --- a/packages/browser/src/helpers.ts +++ b/packages/browser/src/helpers.ts @@ -1,4 +1,4 @@ -import type { Mechanism, WrappedFunction } from '@sentry/core'; +import type { HandlerDataError, Mechanism, WrappedFunction } from '@sentry/core'; import { addExceptionMechanism, addExceptionTypeValue, @@ -15,11 +15,45 @@ export const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; let ignoreOnError: number = 0; +type ErrorReport = Pick; + +const ignoredErrorReports: ErrorReport[] = []; + /** * @hidden */ -export function shouldIgnoreOnError(): boolean { - return ignoreOnError > 0; +export function shouldIgnoreOnError(data?: HandlerDataError): boolean { + if (ignoreOnError > 0) { + return true; + } + + const index = data ? ignoredErrorReports.findIndex(report => isSameErrorReport(report, data)) : -1; + if (index === -1) { + return false; + } + + ignoredErrorReports.splice(index, 1); + return true; +} + +/** + * Skips the next `onerror` report that matches `report`, once. A report that + * never arrives is forgotten after the current task. + * + * @hidden + */ +export function ignoreNextOnErrorMatching(report: ErrorReport): void { + ignoredErrorReports.push(report); + setTimeout(() => { + const index = ignoredErrorReports.indexOf(report); + if (index !== -1) { + ignoredErrorReports.splice(index, 1); + } + }); +} + +function isSameErrorReport(a: ErrorReport, b: ErrorReport): boolean { + return a.msg === b.msg && a.url === b.url && a.line === b.line && a.column === b.column; } /** diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index f258913a8e16..c3515a77275a 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -54,7 +54,7 @@ function _installGlobalOnErrorHandler(client: Client): void { addGlobalErrorInstrumentationHandler(data => { const { stackParser, attachStacktrace } = getOptions(); - if (getClient() !== client || shouldIgnoreOnError()) { + if (getClient() !== client || shouldIgnoreOnError(data)) { return; } diff --git a/packages/browser/src/integrations/webWorker.ts b/packages/browser/src/integrations/webWorker.ts index 9dec09443eef..41420d49acc0 100644 --- a/packages/browser/src/integrations/webWorker.ts +++ b/packages/browser/src/integrations/webWorker.ts @@ -2,7 +2,6 @@ import type { DebugImage, Integration, IntegrationFn } from '@sentry/core'; import { addNonEnumerableProperty, captureEvent, - consoleSandbox, debug, defineIntegration, getClient, @@ -13,7 +12,7 @@ import { } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; import { eventFromUnknownInput, extractMessage, extractType } from '../eventbuilder'; -import { WINDOW } from '../helpers'; +import { ignoreNextOnErrorMatching, WINDOW } from '../helpers'; import { _enhanceEventWithInitialFrame, _eventFromRejectionWithPrimitive, @@ -22,16 +21,14 @@ import { export const INTEGRATION_NAME = 'WebWorker' as const; +const MAX_FORWARDED_ERRORS = 20; + interface WebWorkerMessage { _sentryMessage: boolean; _sentryDebugIds?: Record; _sentryModuleMetadata?: Record; // eslint-disable-line @typescript-eslint/no-explicit-any _sentryWorkerError?: SerializedWorkerError; _sentryWasmImages?: Array; - /** Sent with every message by workers that forward uncaught errors and wait for the reply below. */ - _sentryForwardsErrors?: boolean; - /** The page's reply: it captures and replays forwarded errors, so the worker may cancel them. */ - _sentryHandlesForwardedErrors?: boolean; } type WorkerErrorKind = 'error' | 'unhandledrejection'; @@ -43,7 +40,7 @@ interface SerializedWorkerError { kind?: WorkerErrorKind; /** Structured clone resets any name outside the built-in set to `Error`. */ name?: string; - /** The `ErrorEvent` message, replayed on the worker object for page listeners. */ + /** The `ErrorEvent` message, matched against the copy that bubbles to `window.onerror`. */ message?: string; /** Script the error was thrown in, which can differ from the worker script. */ url?: string; @@ -51,8 +48,6 @@ interface SerializedWorkerError { colno?: number; /** Set when `reason` is a plain `{ message, stack }` copy of an error that did not clone. */ plainError?: boolean; - /** The worker stopped the native event from bubbling, so the page replays it. */ - cancelled?: boolean; } interface WebWorkerIntegrationOptions { @@ -145,23 +140,27 @@ export const webWorkerIntegration = defineIntegration(({ worker }: WebWorkerInte })) as IntegrationFn; function listenForSentryMessages(worker: Worker): void { - let acknowledged = false; + // Forwarded errors whose message-only copy has yet to bubble to the page. + const forwardedErrors: Array> = []; + + // The bubbled copy fires `error` on the worker object and then `window.onerror` in the same task, after the + // forward arrived. Skipping only copies of errors we actually received means a missing forward can at worst + // cause a duplicate, never a lost error. + worker.addEventListener('error', ({ message, filename, lineno, colno }) => { + const index = forwardedErrors.findIndex( + e => e.message === message && e.url === filename && e.lineno === lineno && e.colno === colno, + ); + if (index !== -1) { + // Earlier entries were cancelled inside the worker and will never bubble. + forwardedErrors.splice(0, index + 1); + ignoreNextOnErrorMatching({ msg: message, url: filename, line: lineno, column: colno }); + } + }); worker.addEventListener('message', event => { if (isSentryMessage(event.data)) { event.stopImmediatePropagation(); // other listeners should not receive this message - // A worker must not cancel its error events until it knows this page - // captures and replays them, or an unregistered worker and an older - // page bundle would lose them. Only workers that declared the - // capability get the reply, so older workers never see it. Every - // worker message carries it, so a worker added after its announce is - // acknowledged on its first forwarded error. - if (event.data._sentryForwardsErrors && !acknowledged) { - acknowledged = true; - worker.postMessage({ _sentryMessage: true, _sentryHandlesForwardedErrors: true }); - } - // Handle debug IDs if (event.data._sentryDebugIds) { DEBUG_BUILD && debug.log('Sentry debugId web worker message received', event.data); @@ -204,14 +203,21 @@ function listenForSentryMessages(worker: Worker): void { // Handle errors and unhandled rejections forwarded from worker if (event.data._sentryWorkerError) { DEBUG_BUILD && debug.log('Sentry worker error message received', event.data._sentryWorkerError); - handleForwardedWorkerError(worker, event.data._sentryWorkerError); + const { kind, message, url, lineno, colno } = event.data._sentryWorkerError; + if (kind === 'error') { + // Bounded because a worker that cancels its own errors never bubbles them. + if (forwardedErrors.push({ message, url, lineno, colno }) > MAX_FORWARDED_ERRORS) { + forwardedErrors.shift(); + } + } + handleForwardedWorkerError(event.data._sentryWorkerError); } } }); } -function handleForwardedWorkerError(worker: Worker, workerError: SerializedWorkerError): void { - const { reason, kind, name, filename, url, lineno, colno, plainError, message, cancelled } = workerError; +function handleForwardedWorkerError(workerError: SerializedWorkerError): void { + const { reason, kind, name, filename, url, lineno, colno, plainError } = workerError; // Older workers only ever forwarded rejections and send no `kind`. const isUnhandledRejection = kind !== 'error'; @@ -221,14 +227,6 @@ function handleForwardedWorkerError(worker: Worker, workerError: SerializedWorke addNonEnumerableProperty(error, 'name', name); } - // A cancelled native error event also silences `error` listeners on the - // worker object in the page. Replay it for them with the error object the - // native event never carries. A dispatched event has no default action, - // so it does not reach `window.onerror`. - if (cancelled && typeof ErrorEvent === 'function') { - worker.dispatchEvent(new ErrorEvent('error', { message, filename: url || filename, lineno, colno, error })); - } - const client = getClient(); if (!client) { return; @@ -306,10 +304,6 @@ interface RegisterWebWorkerOptions { * no `error` object, so globalHandlers can only build an event from the message string. * Forwarding them here preserves the real stack, which matters most for wasm frames. * - * Call this before the worker registers its own `message` handlers. The page replies - * with one message that this function consumes, so handlers registered earlier would - * receive it. - * * @example * ```ts filename={worker.js} * import * as Sentry from '@sentry/'; @@ -328,32 +322,20 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { // V8's default of 10 truncates stacks before this code forwards them. Error.stackTraceLimit = 50; - let pageHandlesForwardedErrors = false; - - // Registered before the announce so the reply cannot arrive first. - self.addEventListener('message', (event: unknown) => { - const { data, stopImmediatePropagation } = event as { data?: unknown; stopImmediatePropagation?: () => void }; - if (isPlainObject(data) && data._sentryMessage === true && data._sentryHandlesForwardedErrors === true) { - pageHandlesForwardedErrors = true; - stopImmediatePropagation?.call(event); - } - }); - // Send debug IDs and raw module metadata to parent thread // The metadata will be parsed lazily on the main thread when needed self.postMessage({ _sentryMessage: true, _sentryDebugIds: self._sentryDebugIds ?? undefined, _sentryModuleMetadata: self._sentryModuleMetadata ?? undefined, - _sentryForwardsErrors: true, }); - const forward = (serializedError: Omit): boolean => { + const forward = (serializedError: Omit): void => { const { reason } = serializedError; DEBUG_BUILD && debug.log(`[Sentry Worker] Forwarding ${serializedError.kind} to parent`, serializedError); - return postSerializedWorkerError(self, { + postSerializedWorkerError(self, { ...serializedError, filename: self.location?.href, name: isError(reason) ? extractType(reason) : undefined, @@ -363,34 +345,15 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { // Uncaught errors bubble to the parent, but the propagated ErrorEvent // carries no error object. Forwarding the object keeps the real stack. self.addEventListener('error', (event: unknown) => { - const errorEvent = event as { + const { error, message, filename, lineno, colno } = event as { error?: unknown; message?: string; filename?: string; lineno?: number; colno?: number; - preventDefault?: () => void; }; - const { error, message, filename, lineno, colno } = errorEvent; - const reason = error ?? message; - // Until the page confirmed it handles forwarded errors, the bubbled copy - // is the only report that is sure to reach it. - const cancelled = pageHandlesForwardedErrors; - const forwarded = forward({ kind: 'error', reason, message, url: filename, lineno, colno, cancelled }); - - if (!forwarded || !cancelled) { - return; - } - - // The page now has the error with its stack, so the message-only copy - // must not bubble there as well. A cancelled error prints nothing, so - // log it to keep it visible in DevTools. - errorEvent.preventDefault?.(); - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.error(reason); - }); + forward({ kind: 'error', reason: error ?? message, message, url: filename, lineno, colno }); }); // Unhandled rejections do not bubble to the parent thread at all. @@ -405,19 +368,18 @@ export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { * `postMessage` structured-clones the reason. A `DataCloneError` must never * escape the worker's own error handler, so the forward is retried with * plain data that clones in every browser, including ones that cannot clone - * `Error` at all. Returns whether the page received the error. + * `Error` at all. */ function postSerializedWorkerError( self: MinimalDedicatedWorkerGlobalScope, serializedError: SerializedWorkerError, -): boolean { +): void { try { self.postMessage({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: serializedError, }); - return true; + return; } catch { // Not cloneable, fall through and send plain data instead. } @@ -429,13 +391,10 @@ function postSerializedWorkerError( try { self.postMessage({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: { ...serializedError, reason: plainReason, plainError }, }); - return true; } catch { // Dropping the forward is better than throwing out of the worker's error handler. - return false; } } diff --git a/packages/browser/test/helpers.test.ts b/packages/browser/test/helpers.test.ts index 740ff2ecd7e6..6ffda1158ae3 100644 --- a/packages/browser/test/helpers.test.ts +++ b/packages/browser/test/helpers.test.ts @@ -1,6 +1,6 @@ import type { WrappedFunction } from '@sentry/core'; -import { describe, expect, it, vi } from 'vitest'; -import { wrap } from '../src/helpers'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ignoreNextOnErrorMatching, shouldIgnoreOnError, wrap } from '../src/helpers'; describe('internal wrap()', () => { it('should wrap only functions', () => { @@ -201,3 +201,42 @@ describe('internal wrap()', () => { expect(wrapped).not.toBe('something that is not a function'); }); }); + +describe('ignoreNextOnErrorMatching()', () => { + const report = { msg: 'Uncaught Error: boom', url: 'http://localhost/worker.js', line: 12, column: 9 }; + + beforeEach(async () => { + // The wrap() tests above leave real `ignoreNextOnError` resets pending. + await new Promise(resolve => setTimeout(resolve)); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.runAllTimers(); + vi.useRealTimers(); + }); + + it('skips a matching report once', () => { + ignoreNextOnErrorMatching(report); + + expect(shouldIgnoreOnError({ ...report })).toBe(true); + expect(shouldIgnoreOnError({ ...report })).toBe(false); + }); + + it('does not skip a report that differs in any field', () => { + ignoreNextOnErrorMatching(report); + + expect(shouldIgnoreOnError({ ...report, msg: 'Uncaught Error: other' })).toBe(false); + expect(shouldIgnoreOnError({ ...report, url: 'http://localhost/other.js' })).toBe(false); + expect(shouldIgnoreOnError({ ...report, line: 13 })).toBe(false); + expect(shouldIgnoreOnError({ ...report, column: 10 })).toBe(false); + expect(shouldIgnoreOnError()).toBe(false); + }); + + it('forgets a report that never arrives after the current task', () => { + ignoreNextOnErrorMatching(report); + vi.runAllTimers(); + + expect(shouldIgnoreOnError({ ...report })).toBe(false); + }); +}); diff --git a/packages/browser/test/integrations/webWorker.test.ts b/packages/browser/test/integrations/webWorker.test.ts index 5a90ac57c433..a5c1b268c175 100644 --- a/packages/browser/test/integrations/webWorker.test.ts +++ b/packages/browser/test/integrations/webWorker.test.ts @@ -32,6 +32,7 @@ vi.mock('../../src/helpers', () => ({ WINDOW: { _sentryDebugIds: undefined, }, + ignoreNextOnErrorMatching: vi.fn(), })); function getListener(addEventListener: ReturnType, type: string): (event: any) => void { @@ -47,14 +48,12 @@ describe('webWorkerIntegration', () => { let mockWorker: { addEventListener: ReturnType; - dispatchEvent: ReturnType; postMessage: ReturnType; _sentryDebugIds?: Record; }; let mockWorker2: { addEventListener: ReturnType; - dispatchEvent: ReturnType; postMessage: ReturnType; _sentryDebugIds?: Record; }; @@ -73,13 +72,11 @@ describe('webWorkerIntegration', () => { // Setup mock worker mockWorker = { addEventListener: vi.fn(), - dispatchEvent: vi.fn(), postMessage: vi.fn(), }; mockWorker2 = { addEventListener: vi.fn(), - dispatchEvent: vi.fn(), postMessage: vi.fn(), }; @@ -420,7 +417,6 @@ describe('registerWebWorker', () => { let mockWorkerSelf: { postMessage: ReturnType; addEventListener: ReturnType; - dispatchEvent: ReturnType; _sentryDebugIds?: Record; _sentryModuleMetadata?: Record; location?: { href?: string }; @@ -435,7 +431,6 @@ describe('registerWebWorker', () => { mockWorkerSelf = { postMessage: vi.fn(), addEventListener: vi.fn(), - dispatchEvent: vi.fn(), }; }); @@ -451,7 +446,6 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); }); @@ -471,7 +465,6 @@ describe('registerWebWorker', () => { 'worker-file2.js': 'debug-id-2', }, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); }); @@ -485,7 +478,6 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); }); @@ -503,7 +495,6 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: rawMetadata, - _sentryForwardsErrors: true, }); }); @@ -516,7 +507,6 @@ describe('registerWebWorker', () => { _sentryMessage: true, _sentryDebugIds: undefined, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); }); @@ -538,62 +528,14 @@ describe('registerWebWorker', () => { 'worker-file.js': 'debug-id-1', }, _sentryModuleMetadata: rawMetadata, - _sentryForwardsErrors: true, }); }); describe('error forwarding', () => { - let consoleErrorSpy: MockInstance; - - beforeEach(() => { - consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - }); - - afterEach(() => { - consoleErrorSpy.mockRestore(); - }); - - function trigger(type: string, event: Record): { preventDefault: ReturnType } { - const fullEvent = { preventDefault: vi.fn(), ...event }; - getListener(mockWorkerSelf.addEventListener, type)(fullEvent); - return fullEvent; - } - - function receiveFromPage(data: unknown): { stopImmediatePropagation: ReturnType } { - const messageEvent = { data, stopImmediatePropagation: vi.fn() }; - getListener(mockWorkerSelf.addEventListener, 'message')(messageEvent); - return messageEvent; - } - - function acknowledge(): void { - receiveFromPage({ _sentryMessage: true, _sentryHandlesForwardedErrors: true }); + function trigger(type: string, event: unknown): void { + getListener(mockWorkerSelf.addEventListener, type)(event); } - it('consumes the acknowledgement from the page and leaves other messages alone', () => { - registerWebWorker({ self: mockWorkerSelf as any }); - - const ack = receiveFromPage({ _sentryMessage: true, _sentryHandlesForwardedErrors: true }); - const other = receiveFromPage({ msg: 'WORKER_READY' }); - - expect(ack.stopImmediatePropagation).toHaveBeenCalledOnce(); - expect(other.stopImmediatePropagation).not.toHaveBeenCalled(); - }); - - it('forwards an uncaught error but does not cancel it before the page acknowledged', () => { - registerWebWorker({ self: mockWorkerSelf as any }); - - const error = new Error('boom'); - const event = trigger('error', { error }); - - expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ - _sentryMessage: true, - _sentryForwardsErrors: true, - _sentryWorkerError: expect.objectContaining({ reason: error, kind: 'error', cancelled: false }), - }); - expect(event.preventDefault).not.toHaveBeenCalled(); - expect(consoleErrorSpy).not.toHaveBeenCalled(); - }); - it('raises the stack trace limit so forwarded stacks are not truncated', () => { Error.stackTraceLimit = 10; @@ -604,11 +546,10 @@ describe('registerWebWorker', () => { it('forwards an uncaught error with its location, name and kind "error"', () => { registerWebWorker({ self: mockWorkerSelf as any }); - acknowledge(); mockWorkerSelf.location = { href: 'http://localhost/worker.js' }; const error = new Error('boom'); - const event = trigger('error', { + trigger('error', { error, message: 'Uncaught Error: boom', filename: 'http://localhost/chunk.js', @@ -618,7 +559,6 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: { reason: error, filename: 'http://localhost/worker.js', @@ -628,24 +568,8 @@ describe('registerWebWorker', () => { url: 'http://localhost/chunk.js', lineno: 12, colno: 9, - cancelled: true, }, }); - expect(event.preventDefault).toHaveBeenCalledOnce(); - expect(consoleErrorSpy).toHaveBeenCalledExactlyOnceWith(error); - }); - - it('lets the error bubble when the forward failed, so the page still reports it', () => { - registerWebWorker({ self: mockWorkerSelf as any }); - acknowledge(); - mockWorkerSelf.postMessage.mockImplementation(() => { - throw new DOMException('could not be cloned', 'DataCloneError'); - }); - - const event = trigger('error', { error: new Error('boom') }); - - expect(event.preventDefault).not.toHaveBeenCalled(); - expect(consoleErrorSpy).not.toHaveBeenCalled(); }); it('sends the error name separately because structured clone resets it', () => { @@ -657,7 +581,6 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: error, name: 'RuntimeError' }), }); }); @@ -669,7 +592,6 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: 'Uncaught Error: boom', kind: 'error', @@ -684,12 +606,10 @@ describe('registerWebWorker', () => { registerWebWorker({ self: mockWorkerSelf as any }); const reason = new Error('rejected'); - const event = trigger('unhandledrejection', { reason }); - expect(event.preventDefault).not.toHaveBeenCalled(); + trigger('unhandledrejection', { reason }); expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: { reason, filename: undefined, @@ -716,7 +636,6 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenCalledTimes(3); expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: { message: 'boom', stack: error.stack }, plainError: true, @@ -739,7 +658,6 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: { message: 'boom', stack: error.stack }, plainError: true, @@ -756,7 +674,6 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: { message: 'wasm exception', stack: exception.stack }, plainError: true, @@ -772,7 +689,6 @@ describe('registerWebWorker', () => { expect(mockWorkerSelf.postMessage).toHaveBeenLastCalledWith({ _sentryMessage: true, - _sentryForwardsErrors: true, _sentryWorkerError: expect.objectContaining({ reason: { retry: '[Function: retry]' }, plainError: false, @@ -803,15 +719,9 @@ describe('registerWebWorker and webWorkerIntegration', () => { 'Error at \n /shared-file.js': 'main-debug-id', }; - // Each mock stands in for both the worker and its `self`, so messages - // posted either way reach every message listener registered on it. - type Listeners = Record any>>; - const listeners1: Listeners = {}; - const listeners2: Listeners = {}; - const listeners3: Listeners = {}; - const deliver = (listeners: Listeners, message: unknown): void => { - (listeners.message ?? []).forEach(listener => listener({ data: message, stopImmediatePropagation: vi.fn() })); - }; + let cb1: ((arg0: any) => any) | undefined = undefined; + let cb2: ((arg0: any) => any) | undefined = undefined; + let cb3: ((arg0: any) => any) | undefined = undefined; // Setup mock worker const mockWorker = { @@ -820,10 +730,11 @@ describe('registerWebWorker and webWorkerIntegration', () => { 'Error at \n /worker-file2.js': 'worker-debug-2', 'Error at \n /shared-file.js': 'worker-debug-id', }, - addEventListener: vi.fn( - (type: string, l: (arg0: any) => any) => (listeners1[type] = [...(listeners1[type] ?? []), l]), - ), - postMessage: vi.fn(message => deliver(listeners1, message)), + addEventListener: vi.fn((_, l) => (cb1 = l)), + postMessage: vi.fn(message => { + // @ts-expect-error - cb is defined + cb1({ data: message, stopImmediatePropagation: vi.fn() }); + }), }; const mockWorker2 = { @@ -832,10 +743,11 @@ describe('registerWebWorker and webWorkerIntegration', () => { 'Error at \n /worker-2-file2.js': 'worker-2-debug-2', }, - addEventListener: vi.fn( - (type: string, l: (arg0: any) => any) => (listeners2[type] = [...(listeners2[type] ?? []), l]), - ), - postMessage: vi.fn(message => deliver(listeners2, message)), + addEventListener: vi.fn((_, l) => (cb2 = l)), + postMessage: vi.fn(message => { + // @ts-expect-error - cb is defined + cb2({ data: message, stopImmediatePropagation: vi.fn() }); + }), }; const mockWorker3 = { @@ -843,10 +755,11 @@ describe('registerWebWorker and webWorkerIntegration', () => { 'Error at \n /worker-3-file1.js': 'worker-3-debug-1', 'Error at \n /worker-3-file2.js': 'worker-3-debug-2', }, - addEventListener: vi.fn( - (type: string, l: (arg0: any) => any) => (listeners3[type] = [...(listeners3[type] ?? []), l]), - ), - postMessage: vi.fn(message => deliver(listeners3, message)), + addEventListener: vi.fn((_, l) => (cb3 = l)), + postMessage: vi.fn(message => { + // @ts-expect-error - cb is defined + cb3({ data: message, stopImmediatePropagation: vi.fn() }); + }), }; const integration = webWorkerIntegration({ worker: [mockWorker as any, mockWorker2 as any] }); @@ -862,7 +775,6 @@ describe('registerWebWorker and webWorkerIntegration', () => { _sentryMessage: true, _sentryDebugIds: mockWorker._sentryDebugIds, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); expect((helpers.WINDOW as any)._sentryDebugIds).toEqual({ @@ -884,7 +796,6 @@ describe('registerWebWorker and webWorkerIntegration', () => { _sentryMessage: true, _sentryDebugIds: mockWorker3._sentryDebugIds, _sentryModuleMetadata: undefined, - _sentryForwardsErrors: true, }); expect((helpers.WINDOW as any)._sentryDebugIds).toEqual({ @@ -904,11 +815,7 @@ describe('registerWebWorker and webWorkerIntegration', () => { describe('forwarded worker errors', () => { let client: BrowserClient; let captureEventSpy: MockInstance; - let mockWorker: { - addEventListener: ReturnType; - dispatchEvent: ReturnType; - postMessage: ReturnType; - }; + let mockWorker: { addEventListener: ReturnType; postMessage: ReturnType }; beforeEach(() => { vi.clearAllMocks(); @@ -921,7 +828,7 @@ describe('forwarded worker errors', () => { client.init(); captureEventSpy = vi.spyOn(client, 'captureEvent'); - mockWorker = { addEventListener: vi.fn(), dispatchEvent: vi.fn(), postMessage: vi.fn() }; + mockWorker = { addEventListener: vi.fn(), postMessage: vi.fn() }; const integration = webWorkerIntegration({ worker: mockWorker as any }); integration.setupOnce!(); }); @@ -1053,66 +960,60 @@ describe('forwarded worker errors', () => { }); }); - it('acknowledges a worker that declared it forwards errors, once', () => { - receive({ _sentryDebugIds: undefined, _sentryForwardsErrors: true }); - receive({ _sentryWorkerError: { reason: new Error('boom'), kind: 'error' }, _sentryForwardsErrors: true }); - - expect(mockWorker.postMessage).toHaveBeenCalledExactlyOnceWith({ - _sentryMessage: true, - _sentryHandlesForwardedErrors: true, - }); - }); + const bubbled = { + message: 'Uncaught Error: boom', + filename: 'http://localhost/worker.js', + lineno: 12, + colno: 9, + }; + const forwardedError = { + reason: new Error('boom'), + kind: 'error', + message: bubbled.message, + url: bubbled.filename, + lineno: bubbled.lineno, + colno: bubbled.colno, + }; - it('acknowledges a worker added after its announce on its first forwarded error', () => { - receive({ _sentryWorkerError: { reason: new Error('boom'), kind: 'error' }, _sentryForwardsErrors: true }); + it('skips the global onerror copy of an error that was forwarded', () => { + forward(forwardedError); + getListener(mockWorker.addEventListener, 'error')(bubbled); - expect(mockWorker.postMessage).toHaveBeenCalledExactlyOnceWith({ - _sentryMessage: true, - _sentryHandlesForwardedErrors: true, + expect(helpers.ignoreNextOnErrorMatching).toHaveBeenCalledExactlyOnceWith({ + msg: 'Uncaught Error: boom', + url: 'http://localhost/worker.js', + line: 12, + column: 9, }); }); - it('does not message a worker registered by an older SDK, whose handlers would receive it', () => { - receive({ _sentryDebugIds: undefined }); + it('keeps the global onerror copy of an error that was never forwarded', () => { + getListener(mockWorker.addEventListener, 'error')(bubbled); - expect(mockWorker.postMessage).not.toHaveBeenCalled(); + expect(helpers.ignoreNextOnErrorMatching).not.toHaveBeenCalled(); }); - it('replays a cancelled error on the worker object with the error object attached', () => { - const error = new Error('boom'); - - forward({ - reason: error, - kind: 'error', - message: 'Uncaught Error: boom', - filename: 'http://localhost/worker.js', - url: 'http://localhost/chunk.js', - lineno: 12, - colno: 9, - cancelled: true, - }); + it('keeps the global onerror copy when a different error was forwarded', () => { + forward({ ...forwardedError, lineno: 13 }); + getListener(mockWorker.addEventListener, 'error')(bubbled); - expect(mockWorker.dispatchEvent).toHaveBeenCalledExactlyOnceWith( - expect.objectContaining({ - type: 'error', - message: 'Uncaught Error: boom', - filename: 'http://localhost/chunk.js', - lineno: 12, - colno: 9, - error, - }), - ); + expect(helpers.ignoreNextOnErrorMatching).not.toHaveBeenCalled(); }); - it('does not replay an error the worker let bubble, since the native event fires for it', () => { - forward({ reason: new Error('boom'), kind: 'error', message: 'Uncaught Error: boom', cancelled: false }); + it('skips each forwarded error once', () => { + const onWorkerError = getListener(mockWorker.addEventListener, 'error'); + + forward(forwardedError); + onWorkerError(bubbled); + onWorkerError(bubbled); - expect(mockWorker.dispatchEvent).not.toHaveBeenCalled(); + expect(helpers.ignoreNextOnErrorMatching).toHaveBeenCalledOnce(); }); - it('does not replay a forwarded rejection, which never fires on the worker object', () => { - forward({ reason: new Error('rejected'), kind: 'unhandledrejection' }); + it('does not expect a bubbled copy of a forwarded rejection', () => { + forward({ ...forwardedError, kind: 'unhandledrejection' }); + getListener(mockWorker.addEventListener, 'error')(bubbled); - expect(mockWorker.dispatchEvent).not.toHaveBeenCalled(); + expect(helpers.ignoreNextOnErrorMatching).not.toHaveBeenCalled(); }); }); From 68328c8090b751375338a672c2fbafdbe20d09de Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Mon, 21 Sep 2026 12:53:43 -0400 Subject: [PATCH 11/11] test(e2e): Record worker error events on the stream that resolves the test Each waitForError opens its own stream to the event proxy and events are not ordered across streams, so a separate recorder could lag behind the event that ends the test and miss it. --- .../tests/errors.test.ts | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts index d7ab2107ed2d..d000eb1e128c 100644 --- a/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/browser-webworker-vite/tests/errors.test.ts @@ -66,21 +66,16 @@ test('captures an error with debug ids and pageload trace context', async ({ pag test('emits exactly one event for an uncaught worker error', async ({ page }) => { const mechanisms: Array = []; - // Never resolves. It only records every error event that arrives, so - // the global handler's copy of a throw would show up here. - void waitForError('browser-webworker-vite', event => { + // Records on the same stream it resolves on, since events are not ordered across streams. + const secondErrorPromise = waitForError('browser-webworker-vite', event => { if (!event.type && event.exception?.values?.[0]) { mechanisms.push(event.exception.values[0].mechanism?.type); } - return false; + return event.exception?.values?.[0]?.value === 'Uncaught error in worker 2'; }); - const firstErrorPromise = waitForError('browser-webworker-vite', event => { return event.exception?.values?.[0]?.value === 'Uncaught error in worker'; }); - const secondErrorPromise = waitForError('browser-webworker-vite', event => { - return event.exception?.values?.[0]?.value === 'Uncaught error in worker 2'; - }); await page.goto('/'); @@ -128,20 +123,17 @@ test('locates a thrown primitive by its ErrorEvent position', async ({ page }) = test('emits exactly one event for an error thrown during worker startup', async ({ page }) => { const values: Array = []; - void waitForError('browser-webworker-vite', event => { + // Records on the same stream it resolves on, since events are not ordered across streams. + const laterErrorPromise = waitForError('browser-webworker-vite', event => { const value = event.exception?.values?.[0]?.value; if (value?.includes('Uncaught error during worker startup')) { values.push(value); } - return false; + return value === 'Uncaught error in worker'; }); - const startupErrorPromise = waitForError('browser-webworker-vite', event => { return !!event.exception?.values?.[0]?.value?.includes('Uncaught error during worker startup'); }); - const laterErrorPromise = waitForError('browser-webworker-vite', event => { - return event.exception?.values?.[0]?.value === 'Uncaught error in worker'; - }); await page.goto('/');