-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(browser): Forward worker metadata for third-party error filtering #18756
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
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 |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ export const INTEGRATION_NAME = 'WebWorker'; | |
| interface WebWorkerMessage { | ||
| _sentryMessage: boolean; | ||
| _sentryDebugIds?: Record<string, string>; | ||
| _sentryModuleMetadata?: Record<string, any>; // eslint-disable-line @typescript-eslint/no-explicit-any | ||
| _sentryWorkerError?: SerializedWorkerError; | ||
| } | ||
|
|
||
|
|
@@ -122,6 +123,18 @@ function listenForSentryMessages(worker: Worker): void { | |
| }; | ||
| } | ||
|
|
||
| // Handle module metadata | ||
| if (event.data._sentryModuleMetadata) { | ||
| DEBUG_BUILD && debug.log('Sentry module metadata web worker message received', event.data); | ||
| // Merge worker's raw metadata into the global object | ||
| // It will be parsed lazily when needed by getMetadataForUrl | ||
| WINDOW._sentryModuleMetadata = { | ||
| ...event.data._sentryModuleMetadata, | ||
| // Module metadata of the main thread have precedence over the worker's in case of a collision. | ||
| ...WINDOW._sentryModuleMetadata, | ||
| }; | ||
andreiborza marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // Handle unhandled rejections forwarded from worker | ||
| if (event.data._sentryWorkerError) { | ||
| DEBUG_BUILD && debug.log('Sentry worker rejection message received', event.data._sentryWorkerError); | ||
|
|
@@ -187,14 +200,18 @@ interface MinimalDedicatedWorkerGlobalScope { | |
| } | ||
|
|
||
| interface RegisterWebWorkerOptions { | ||
| self: MinimalDedicatedWorkerGlobalScope & { _sentryDebugIds?: Record<string, string> }; | ||
| self: MinimalDedicatedWorkerGlobalScope & { | ||
| _sentryDebugIds?: Record<string, string>; | ||
| _sentryModuleMetadata?: Record<string, any>; // eslint-disable-line @typescript-eslint/no-explicit-any | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Use this function to register the worker with the Sentry SDK. | ||
| * | ||
| * 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 | ||
| * | ||
|
|
@@ -215,10 +232,12 @@ 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 | ||
| // 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, | ||
| }); | ||
|
|
||
| // Set up unhandledrejection handler inside the worker | ||
|
|
@@ -251,11 +270,12 @@ function isSentryMessage(eventData: unknown): eventData is WebWorkerMessage { | |
| return false; | ||
| } | ||
|
|
||
| // Must have at least one of: debug IDs or worker error | ||
| // Must have at least one of: debug IDs, module metadata, or worker error | ||
| const hasDebugIds = '_sentryDebugIds' in eventData; | ||
| const hasModuleMetadata = '_sentryModuleMetadata' in eventData; | ||
| const hasWorkerError = '_sentryWorkerError' in eventData; | ||
|
|
||
| if (!hasDebugIds && !hasWorkerError) { | ||
| if (!hasDebugIds && !hasModuleMetadata && !hasWorkerError) { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -264,6 +284,14 @@ function isSentryMessage(eventData: unknown): eventData is WebWorkerMessage { | |
| return false; | ||
| } | ||
|
|
||
| // Validate module metadata if present | ||
| if ( | ||
| hasModuleMetadata && | ||
| !(isPlainObject(eventData._sentryModuleMetadata) || eventData._sentryModuleMetadata === undefined) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
|
Comment on lines
273
to
294
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. l: I'm wondering if we should turn around this validation logic to default to false and only emit (logaf-l so feel free to skip)
Member
Author
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. I think that ends up with even more complicated code because we still need to ensure that present fields are validated, we can't exit early basically. The code is more readable this way, unless I'm missing something 😅. |
||
| // Validate worker error if present | ||
| if (hasWorkerError && !isPlainObject(eventData._sentryWorkerError)) { | ||
| return false; | ||
|
|
||
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.
l: Why not
unknown?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.
To be aligned with the way we define it in the main-thread, maybe we can change that at a later point?