-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(browser): Capture unhandled rejection errors for web worker integration #18054
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cf6a8f8
796be14
a5b5595
0b45f92
d01a76e
6c2ddc0
3ccdcb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,34 @@ | ||
| // This worker manually replicates what Sentry.registerWebWorker() does | ||
| // (In real code with a bundler, you'd import and call Sentry.registerWebWorker({ self })) | ||
|
|
||
| self._sentryDebugIds = { | ||
| 'Error at http://sentry-test.io/worker.js': 'worker-debug-id-789', | ||
| }; | ||
|
|
||
| // Send debug IDs | ||
| self.postMessage({ | ||
| _sentryMessage: true, | ||
| _sentryDebugIds: self._sentryDebugIds, | ||
| }); | ||
|
|
||
| // Set up unhandledrejection handler (same as registerWebWorker) | ||
| self.addEventListener('unhandledrejection', event => { | ||
| self.postMessage({ | ||
| _sentryMessage: true, | ||
| _sentryWorkerError: { | ||
| reason: event.reason, | ||
| filename: self.location.href, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| self.addEventListener('message', event => { | ||
| if (event.data.type === 'throw-error') { | ||
| throw new Error('Worker error for testing'); | ||
| } | ||
|
|
||
| if (event.data.type === 'throw-rejection') { | ||
| // Create an unhandled rejection | ||
| Promise.reject(new Error('Worker unhandled rejection')); | ||
| } | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,21 @@ | ||
| import type { Integration, IntegrationFn } from '@sentry/core'; | ||
| import { debug, defineIntegration, isPlainObject } from '@sentry/core'; | ||
| import { captureEvent, debug, defineIntegration, getClient, isPlainObject, isPrimitive } from '@sentry/core'; | ||
| import { DEBUG_BUILD } from '../debug-build'; | ||
| import { eventFromUnknownInput } from '../eventbuilder'; | ||
| import { WINDOW } from '../helpers'; | ||
| import { _eventFromRejectionWithPrimitive, _getUnhandledRejectionError } from './globalhandlers'; | ||
|
|
||
| export const INTEGRATION_NAME = 'WebWorker'; | ||
|
|
||
| interface WebWorkerMessage { | ||
| _sentryMessage: boolean; | ||
| _sentryDebugIds?: Record<string, string>; | ||
| _sentryWorkerError?: SerializedWorkerError; | ||
| } | ||
|
|
||
| interface SerializedWorkerError { | ||
| reason: unknown; | ||
| filename?: string; | ||
| } | ||
|
|
||
| interface WebWorkerIntegrationOptions { | ||
|
|
@@ -94,25 +102,75 @@ interface WebWorkerIntegration extends Integration { | |
| export const webWorkerIntegration = defineIntegration(({ worker }: WebWorkerIntegrationOptions) => ({ | ||
| name: INTEGRATION_NAME, | ||
| setupOnce: () => { | ||
| (Array.isArray(worker) ? worker : [worker]).forEach(w => listenForSentryDebugIdMessages(w)); | ||
| (Array.isArray(worker) ? worker : [worker]).forEach(w => listenForSentryMessages(w)); | ||
| }, | ||
| addWorker: (worker: Worker) => listenForSentryDebugIdMessages(worker), | ||
| addWorker: (worker: Worker) => listenForSentryMessages(worker), | ||
| })) as IntegrationFn<WebWorkerIntegration>; | ||
|
|
||
| function listenForSentryDebugIdMessages(worker: Worker): void { | ||
| function listenForSentryMessages(worker: Worker): void { | ||
| worker.addEventListener('message', event => { | ||
| if (isSentryDebugIdMessage(event.data)) { | ||
| if (isSentryMessage(event.data)) { | ||
| event.stopImmediatePropagation(); // other listeners should not receive this message | ||
| DEBUG_BUILD && debug.log('Sentry debugId web worker message received', event.data); | ||
| WINDOW._sentryDebugIds = { | ||
| ...event.data._sentryDebugIds, | ||
| // debugIds of the main thread have precedence over the worker's in case of a collision. | ||
| ...WINDOW._sentryDebugIds, | ||
| }; | ||
|
|
||
| // Handle debug IDs | ||
| if (event.data._sentryDebugIds) { | ||
| DEBUG_BUILD && debug.log('Sentry debugId web worker message received', event.data); | ||
| WINDOW._sentryDebugIds = { | ||
| ...event.data._sentryDebugIds, | ||
| // debugIds of the main thread have precedence over the worker's in case of a collision. | ||
| ...WINDOW._sentryDebugIds, | ||
| }; | ||
| } | ||
|
|
||
| // Handle 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); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| function handleForwardedWorkerRejection(workerError: SerializedWorkerError): void { | ||
| const client = getClient(); | ||
| if (!client) { | ||
| return; | ||
| } | ||
|
|
||
| const stackParser = client.getOptions().stackParser; | ||
| const attachStacktrace = client.getOptions().attachStacktrace; | ||
|
|
||
| const error = workerError.reason; | ||
|
|
||
| // 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); | ||
|
|
||
| event.level = 'error'; | ||
|
|
||
| // Add worker-specific context | ||
| if (workerError.filename) { | ||
| event.contexts = { | ||
| ...event.contexts, | ||
| worker: { | ||
| filename: workerError.filename, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| captureEvent(event, { | ||
| originalException: error, | ||
| mechanism: { | ||
| handled: false, | ||
| type: 'auto.browser.web_worker.onunhandledrejection', | ||
| }, | ||
| }); | ||
Lms24 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| DEBUG_BUILD && debug.log('Captured worker unhandled rejection', error); | ||
| } | ||
|
|
||
| /** | ||
| * Minimal interface for DedicatedWorkerGlobalScope, only requiring the postMessage method. | ||
| * (which is the only thing we need from the worker's global object) | ||
|
|
@@ -124,6 +182,8 @@ function listenForSentryDebugIdMessages(worker: Worker): void { | |
| */ | ||
| interface MinimalDedicatedWorkerGlobalScope { | ||
| postMessage: (message: unknown) => void; | ||
| addEventListener: (type: string, listener: (event: unknown) => void) => void; | ||
| location?: { href?: string }; | ||
| } | ||
|
|
||
| interface RegisterWebWorkerOptions { | ||
|
|
@@ -133,6 +193,14 @@ interface RegisterWebWorkerOptions { | |
| /** | ||
| * Use this function to register the worker with the Sentry SDK. | ||
| * | ||
| * This function will: | ||
| * - Send debug IDs to the parent thread | ||
| * - Set up a handler for unhandled rejections in the worker | ||
| * - Forward unhandled rejections 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. | ||
| * | ||
| * @example | ||
| * ```ts filename={worker.js} | ||
| * import * as Sentry from '@sentry/<your-sdk>'; | ||
|
|
@@ -147,17 +215,59 @@ interface RegisterWebWorkerOptions { | |
| * - `self`: The worker instance you're calling this function from (self). | ||
| */ | ||
| export function registerWebWorker({ self }: RegisterWebWorkerOptions): void { | ||
| // Send debug IDs to parent thread | ||
| self.postMessage({ | ||
| _sentryMessage: true, | ||
| _sentryDebugIds: self._sentryDebugIds ?? 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); | ||
|
|
||
| // 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, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good idea! |
||
| }; | ||
|
|
||
| // Forward to parent thread | ||
| self.postMessage({ | ||
| _sentryMessage: true, | ||
| _sentryWorkerError: serializedError, | ||
| }); | ||
|
Comment on lines
+231
to
+241
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Structured cloning of 🔍 Detailed AnalysisWhen an 💡 Suggested FixEither serialize the 🤖 Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
|
|
||
| DEBUG_BUILD && debug.log('[Sentry Worker] Forwarding unhandled rejection to parent', serializedError); | ||
| }); | ||
|
|
||
| DEBUG_BUILD && debug.log('[Sentry Worker] Registered worker with unhandled rejection handling'); | ||
| } | ||
|
|
||
| function isSentryDebugIdMessage(eventData: unknown): eventData is WebWorkerMessage { | ||
| return ( | ||
| isPlainObject(eventData) && | ||
| eventData._sentryMessage === true && | ||
| '_sentryDebugIds' in eventData && | ||
| (isPlainObject(eventData._sentryDebugIds) || eventData._sentryDebugIds === undefined) | ||
| ); | ||
| function isSentryMessage(eventData: unknown): eventData is WebWorkerMessage { | ||
| if (!isPlainObject(eventData) || eventData._sentryMessage !== true) { | ||
| return false; | ||
| } | ||
|
|
||
| // Must have at least one of: debug IDs or worker error | ||
| const hasDebugIds = '_sentryDebugIds' in eventData; | ||
| const hasWorkerError = '_sentryWorkerError' in eventData; | ||
|
|
||
| if (!hasDebugIds && !hasWorkerError) { | ||
| return false; | ||
| } | ||
|
|
||
| // Validate debug IDs if present | ||
| if (hasDebugIds && !(isPlainObject(eventData._sentryDebugIds) || eventData._sentryDebugIds === undefined)) { | ||
| return false; | ||
| } | ||
|
|
||
| // Validate worker error if present | ||
| if (hasWorkerError && !isPlainObject(eventData._sentryWorkerError)) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: Is there a reason why we need to manually construct the event? can we call
captureException(workerError.reason)instead? I might be missing something, so just asking.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how we handle errors in the global handler, i just want to preserve the same behavior as other promise rejections events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I missed this and you're right, we should handle this in the same way.