webcore: revoke a Worker's blob URLs when the worker exits - #35828
Closed
robobun wants to merge 3 commits into
Closed
webcore: revoke a Worker's blob URLs when the worker exits#35828robobun wants to merge 3 commits into
robobun wants to merge 3 commits into
Claude / Claude Code Review
completed
Jul 25, 2026 in 11m 9s
Code review found 2 potential issues
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | test/js/web/workers/worker_blob.test.ts:141 |
Bare .rejects.toThrow() without error class/message |
| 🟡 Nit | test/js/web/workers/worker_blob.test.ts:137-138 |
New tests don't wire worker 'error' to reject — hang on failure |
Annotations
Check warning on line 141 in test/js/web/workers/worker_blob.test.ts
claude / Claude Code Review
Bare .rejects.toThrow() without error class/message
Bare `.rejects.toThrow()` passes on any rejection, so this line doesn't actually pin the revoked-URL behavior — REVIEW.md asks for a specific error class/code/message (never bare `toThrow()`). Consider asserting the actual error, e.g. ``.rejects.toThrow(TypeError)`` or matching `/Failed to resolve blob:/` (which is what `fetch` on a missing blob URL produces). Nit only — line 140's `resolveObjectURL(url)` → `undefined` already proves revocation.
Check warning on line 138 in test/js/web/workers/worker_blob.test.ts
claude / Claude Code Review
New tests don't wire worker 'error' to reject — hang on failure
nit: both new tests await `new Promise(resolve => worker.once("message", resolve))` without wiring `worker.once("error", reject)`. If a regression breaks Worker eval startup or `URL.createObjectURL` inside the worker, `message` never fires and the test hangs to the file timeout instead of failing fast with the actual error. Same shape at line 152. REVIEW.md: "Wire EVERY failure event (`error`, `close`, `abort`, process exit) to reject the awaited promise."
Loading