Skip to content

Commit 327897d

Browse files
committed
fix(replay): Handle WorkerHandler edge cases from review
- Wrap worker.postMessage in try/catch so a synchronous throw (e.g. DataCloneError, terminated worker) cleans up the pending entry and rejects the returned promise instead of leaking. - Skip messages without a numeric id in _onMessage. The worker emits { id: undefined, method: 'init', ... } on load; this was already handled implicitly via Map.get(undefined), but a typeof guard makes the contract explicit and is robust against future malformed messages.
1 parent 2619963 commit 327897d

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

packages/replay-internal/src/eventBuffer/WorkerHandler.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,25 @@ export class WorkerHandler {
9292
resolve: resolve as (value: unknown) => void,
9393
reject,
9494
});
95-
this._worker.postMessage({ id, method, arg });
95+
try {
96+
this._worker.postMessage({ id, method, arg });
97+
} catch (error) {
98+
// If postMessage throws synchronously (e.g. DataCloneError, worker
99+
// already terminated), drop the pending entry so it doesn't leak.
100+
this._pending.delete(id);
101+
reject(error);
102+
}
96103
});
97104
}
98105

99106
private _onMessage = ({ data }: MessageEvent): void => {
100107
const response = data as WorkerResponse;
108+
// The worker emits an init message with `id: undefined` on load, which is
109+
// handled by `ensureReady()` via its own listener. Ignore anything that
110+
// doesn't carry a numeric id we issued.
111+
if (typeof response.id !== 'number') {
112+
return;
113+
}
101114
const pending = this._pending.get(response.id);
102115
if (!pending || pending.method !== response.method) {
103116
return;

packages/replay-internal/test/unit/eventBuffer/WorkerHandler.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ class MockWorker implements Pick<Worker, 'addEventListener' | 'removeEventListen
5757
while (this._pendingRequests.length > 0) this.flushOne();
5858
}
5959

60+
/** Dispatch a message that doesn't correspond to a queued request. */
61+
public dispatchRaw(response: Partial<WorkerResponse>): void {
62+
this._dispatch('message', { data: response } as MessageEvent);
63+
}
64+
6065
public get pendingCount(): number {
6166
return this._pendingRequests.length;
6267
}
@@ -122,6 +127,37 @@ describe('Unit | eventBuffer | WorkerHandler', () => {
122127
await expect(promise).rejects.toThrow('Error in compression worker');
123128
});
124129

130+
it('rejects and cleans up the pending entry when worker.postMessage throws synchronously', async () => {
131+
const { worker, handler } = makeHandler();
132+
const error = new Error('DataCloneError');
133+
worker.postMessage = () => {
134+
throw error;
135+
};
136+
137+
await expect(handler.postMessage('addEvent', 'a')).rejects.toBe(error);
138+
139+
// A subsequent successful call should still work — the previous failure
140+
// didn't leave a stale entry behind.
141+
worker.postMessage = MockWorker.prototype.postMessage.bind(worker);
142+
const promise = handler.postMessage<string>('addEvent', 'b');
143+
worker.flushOne();
144+
await expect(promise).resolves.toBe('result-1');
145+
});
146+
147+
it('ignores messages without a numeric id (e.g. the worker init message)', async () => {
148+
const { worker, handler } = makeHandler();
149+
150+
const promise = handler.postMessage<string>('addEvent', 'a');
151+
152+
// Simulate the init message the worker emits on load. Should be ignored
153+
// and not crash.
154+
worker.dispatchRaw({ id: undefined, method: 'init', success: true });
155+
156+
// The legitimate response still resolves.
157+
worker.flushOne();
158+
await expect(promise).resolves.toBe('result-0');
159+
});
160+
125161
it('destroy() rejects pending requests and detaches the listener', async () => {
126162
const { worker, handler } = makeHandler();
127163

0 commit comments

Comments
 (0)