-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(replay): Improve error messages when compression worker fails to load #19008
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
base: develop
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -33,7 +33,8 @@ export class WorkerHandler { | |
| if ((data as WorkerResponse).success) { | ||
| resolve(); | ||
| } else { | ||
| reject(); | ||
| DEBUG_BUILD && debug.warn('Received worker message with unsuccessful status', data); | ||
| reject(new Error('Received worker message with unsuccessful status')); | ||
| } | ||
| }, | ||
| { once: true }, | ||
|
|
@@ -42,7 +43,12 @@ export class WorkerHandler { | |
| this._worker.addEventListener( | ||
| 'error', | ||
| error => { | ||
| reject(error); | ||
| DEBUG_BUILD && debug.warn('Failed to load Replay compression worker', error); | ||
| reject( | ||
| new Error( | ||
| `Failed to load Replay compression worker: ${error instanceof ErrorEvent && error.message ? error.message : 'Unknown error. This can happen due to CSP policy restrictions, network issues, or the worker script failing to load.'}`, | ||
| ), | ||
| ); | ||
|
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. Missing test for improved error message regression fixLow Severity · Bugbot Rules This PR improves error messages when the compression worker fails to load, but no tests were added to verify the improved error messages are actually produced. The existing test at Additional Locations (1) |
||
| }, | ||
| { once: 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.
Redundant debug logging results in duplicate error messages
Low Severity
The new
debug.warncalls at lines 36 and 46 are redundant because the callerEventBufferProxy._ensureWorkerIsLoaded()already logs these errors usingdebug.exception(). Looking at the logger implementation,debug.exception()logs both the message AND the error object to the console. This means when the worker fails to load, the same error appears multiple times in debug output - first as a warning fromWorkerHandler, then as error logs fromEventBufferProxy. The existing logging inEventBufferProxyprovides more useful context (mentioning the fallback behavior), making the new logs unnecessary noise.Additional Locations (1)
packages/replay-internal/src/eventBuffer/WorkerHandler.ts#L35-L36