fix(browser): Capture unhandled rejection errors for web worker integration #18054
Conversation
…-doesnt-handle-unhandledrejection-events
| // The parent will handle primitives vs errors the same way globalHandlers does | ||
| const serializedError: SerializedWorkerError = { | ||
| reason: reason, | ||
| filename: self.location?.href, | ||
| }; | ||
|
|
||
| // Forward to parent thread | ||
| self.postMessage({ | ||
| _sentryMessage: true, | ||
| _sentryWorkerError: serializedError, | ||
| }); |
There was a problem hiding this comment.
Bug: Structured cloning of Error objects from web workers via postMessage causes loss of prototype, preventing proper stack trace parsing.
Severity: HIGH | Confidence: 0.95
🔍 Detailed Analysis
When an Error object is sent from a web worker to the main thread via postMessage, JavaScript's structured cloning converts it into a plain object, losing its Error prototype. This occurs for unhandled promise rejections in web workers. The eventFromUnknownInput() function treats the cloned object as a plain object, as isError() fails. Consequently, its stack property is not parsed into structured StackFrame objects but is serialized into the __serialized__ extra field, preventing proper stack trace display.
💡 Suggested Fix
Either serialize the stack property separately and pass it as a syntheticException, or serialize the stack frames directly from the worker before sending the message.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/browser/src/integrations/webWorker.ts#L227-L241
Potential issue: When an `Error` object is sent from a web worker to the main thread via
`postMessage`, JavaScript's structured cloning converts it into a plain object, losing
its `Error` prototype. This occurs for unhandled promise rejections in web workers. The
`eventFromUnknownInput()` function treats the cloned object as a plain object, as
`isError()` fails. Consequently, its `stack` property is not parsed into structured
`StackFrame` objects but is serialized into the `__serialized__` extra field, preventing
proper stack trace display.
Did we get this right? 👍 / 👎 to inform future reviews.
| 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'; |
There was a problem hiding this comment.
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.
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.
Good point! I missed this and you're right, we should handle this in the same way.
| // The parent will handle primitives vs errors the same way globalHandlers does | ||
| const serializedError: SerializedWorkerError = { | ||
| reason: reason, | ||
| filename: self.location?.href, |
…-doesnt-handle-unhandledrejection-events
…-doesnt-handle-unhandledrejection-events
…-doesnt-handle-unhandledrejection-events
Previously, only synchronous errors thrown in web workers would bubble up and be captured. Unhandled promise rejections within workers would fail silently since they don't propagate to the parent thread automatically. This enhancement ensures complete error coverage for web worker code.