-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(replay): Avoid main-thread blocking in WorkerHandler under event bursts #20548
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
Merged
logaretm
merged 4 commits into
develop
from
awad/js-2308-workerhandler-blocks-the-main-thread-under-event-bursts
Apr 28, 2026
+226
−30
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
08a0812
fix(replay): Avoid main-thread blocking in WorkerHandler under event …
logaretm 2619963
test(replay): Add WorkerHandler unit tests
logaretm 327897d
fix(replay): Handle WorkerHandler edge cases from review
logaretm 9d9136a
chore: Bump size limit for CDN Bundle (Tracing, Replay, Feedback) unc…
logaretm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
174 changes: 174 additions & 0 deletions
174
packages/replay-internal/test/unit/eventBuffer/WorkerHandler.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| /** | ||
| * @vitest-environment jsdom | ||
| */ | ||
|
|
||
| import { describe, expect, it } from 'vitest'; | ||
| import { WorkerHandler } from '../../../src/eventBuffer/WorkerHandler'; | ||
| import type { WorkerResponse } from '../../../src/types'; | ||
|
|
||
| /** | ||
| * Minimal Worker stub that lets tests control when responses dispatch and | ||
| * track how many 'message' listeners are attached at any time. Real workers | ||
| * are async; we model that with a queue we drain manually so the test can | ||
| * assert on the listener count while requests are in flight. | ||
| */ | ||
| class MockWorker implements Pick<Worker, 'addEventListener' | 'removeEventListener' | 'postMessage' | 'terminate'> { | ||
| public listenerCount = 0; | ||
| public terminated = false; | ||
|
|
||
| private _listeners = new Map<string, Set<EventListenerOrEventListenerObject>>(); | ||
| private _pendingRequests: Array<{ id: number; method: string }> = []; | ||
|
|
||
| public addEventListener(type: string, listener: EventListenerOrEventListenerObject): void { | ||
| if (!this._listeners.has(type)) this._listeners.set(type, new Set()); | ||
| this._listeners.get(type)!.add(listener); | ||
| if (type === 'message') this.listenerCount++; | ||
| } | ||
|
|
||
| public removeEventListener(type: string, listener: EventListenerOrEventListenerObject): void { | ||
| const set = this._listeners.get(type); | ||
| if (set?.delete(listener) && type === 'message') this.listenerCount--; | ||
| } | ||
|
|
||
| public postMessage(data: unknown): void { | ||
| const { id, method } = data as { id: number; method: string }; | ||
| this._pendingRequests.push({ id, method }); | ||
| } | ||
|
|
||
| public terminate(): void { | ||
| this.terminated = true; | ||
| } | ||
|
|
||
| /** Dispatch the queued response for a given id (FIFO order otherwise). */ | ||
| public flushOne(overrides?: Partial<WorkerResponse>): void { | ||
| const next = this._pendingRequests.shift(); | ||
| if (!next) return; | ||
| const response: WorkerResponse = { | ||
| id: next.id, | ||
| method: next.method, | ||
| success: true, | ||
| response: `result-${next.id}`, | ||
| ...overrides, | ||
| }; | ||
| this._dispatch('message', { data: response } as MessageEvent); | ||
| } | ||
|
|
||
| public flushAll(): void { | ||
| while (this._pendingRequests.length > 0) this.flushOne(); | ||
| } | ||
|
|
||
| /** Dispatch a message that doesn't correspond to a queued request. */ | ||
| public dispatchRaw(response: Partial<WorkerResponse>): void { | ||
| this._dispatch('message', { data: response } as MessageEvent); | ||
| } | ||
|
|
||
| public get pendingCount(): number { | ||
| return this._pendingRequests.length; | ||
| } | ||
|
|
||
| private _dispatch(type: string, event: MessageEvent): void { | ||
| const set = this._listeners.get(type); | ||
| if (!set) return; | ||
| for (const listener of set) { | ||
| if (typeof listener === 'function') listener(event); | ||
| else listener.handleEvent(event); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const makeHandler = () => { | ||
| const worker = new MockWorker(); | ||
| const handler = new WorkerHandler(worker as unknown as Worker); | ||
| return { worker, handler }; | ||
| }; | ||
|
|
||
| describe('Unit | eventBuffer | WorkerHandler', () => { | ||
| it('does not attach a new message listener per postMessage call (regression: #20547)', async () => { | ||
| const { worker, handler } = makeHandler(); | ||
|
|
||
| // One listener is attached at construction time. | ||
| expect(worker.listenerCount).toBe(1); | ||
|
|
||
| // Fire a burst of in-flight requests. The pre-fix implementation attached | ||
| // one listener per call, growing linearly; this would dispatch every | ||
| // response to all attached listeners (O(n^2) main-thread work). | ||
| const promises = Array.from({ length: 100 }, (_, i) => handler.postMessage('addEvent', `arg-${i}`)); | ||
|
|
||
| expect(worker.listenerCount).toBe(1); | ||
| expect(worker.pendingCount).toBe(100); | ||
|
|
||
| worker.flushAll(); | ||
| await Promise.all(promises); | ||
|
|
||
| // Listener count is still 1 after the burst drains. | ||
| expect(worker.listenerCount).toBe(1); | ||
| }); | ||
|
|
||
| it('resolves concurrent postMessage calls with the correct response per id', async () => { | ||
| const { worker, handler } = makeHandler(); | ||
|
|
||
| const p0 = handler.postMessage<string>('addEvent', 'a'); | ||
| const p1 = handler.postMessage<string>('addEvent', 'b'); | ||
| const p2 = handler.postMessage<string>('addEvent', 'c'); | ||
|
|
||
| worker.flushAll(); | ||
|
|
||
| await expect(p0).resolves.toBe('result-0'); | ||
| await expect(p1).resolves.toBe('result-1'); | ||
| await expect(p2).resolves.toBe('result-2'); | ||
| }); | ||
|
|
||
| it('rejects when the worker reports success: false', async () => { | ||
| const { worker, handler } = makeHandler(); | ||
|
|
||
| const promise = handler.postMessage('addEvent', 'a'); | ||
| worker.flushOne({ success: false, response: 'boom' }); | ||
|
|
||
| await expect(promise).rejects.toThrow('Error in compression worker'); | ||
| }); | ||
|
|
||
| it('rejects and cleans up the pending entry when worker.postMessage throws synchronously', async () => { | ||
| const { worker, handler } = makeHandler(); | ||
| const error = new Error('DataCloneError'); | ||
| worker.postMessage = () => { | ||
| throw error; | ||
| }; | ||
|
|
||
| await expect(handler.postMessage('addEvent', 'a')).rejects.toBe(error); | ||
|
|
||
| // A subsequent successful call should still work — the previous failure | ||
| // didn't leave a stale entry behind. | ||
| worker.postMessage = MockWorker.prototype.postMessage.bind(worker); | ||
| const promise = handler.postMessage<string>('addEvent', 'b'); | ||
| worker.flushOne(); | ||
| await expect(promise).resolves.toBe('result-1'); | ||
| }); | ||
|
|
||
| it('ignores messages without a numeric id (e.g. the worker init message)', async () => { | ||
| const { worker, handler } = makeHandler(); | ||
|
|
||
| const promise = handler.postMessage<string>('addEvent', 'a'); | ||
|
|
||
| // Simulate the init message the worker emits on load. Should be ignored | ||
| // and not crash. | ||
| worker.dispatchRaw({ id: undefined, method: 'init', success: true }); | ||
|
|
||
| // The legitimate response still resolves. | ||
| worker.flushOne(); | ||
| await expect(promise).resolves.toBe('result-0'); | ||
| }); | ||
|
|
||
| it('destroy() rejects pending requests and detaches the listener', async () => { | ||
| const { worker, handler } = makeHandler(); | ||
|
|
||
| const p1 = handler.postMessage('addEvent', 'a'); | ||
| const p2 = handler.postMessage('addEvent', 'b'); | ||
|
|
||
| handler.destroy(); | ||
|
|
||
| await expect(p1).rejects.toThrow('Worker destroyed'); | ||
| await expect(p2).rejects.toThrow('Worker destroyed'); | ||
| expect(worker.terminated).toBe(true); | ||
| expect(worker.listenerCount).toBe(0); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.