inspector: interrupt a busy JS loop so Debugger.pause is serviced#32549
Open
robobun wants to merge 10 commits into
Open
inspector: interrupt a busy JS loop so Debugger.pause is serviced#32549robobun wants to merge 10 commits into
robobun wants to merge 10 commits into
Claude / Claude Code Review
completed
Jun 21, 2026 in 16m 14s
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/regression/issue/32548.test.ts:69-77 |
busyPromise has no rejection path — test hangs with no diagnostic if child never prints ready marker |
| 🟡 Nit | test/regression/issue/32548.test.ts:1-7 |
Regression test header should be issue URL only |
Annotations
Check warning on line 77 in test/regression/issue/32548.test.ts
claude / Claude Code Review
busyPromise has no rejection path — test hangs with no diagnostic if child never prints ready marker
`busyPromise` has no rejection path: only `busyResolve` is destructured, the `for await` loop ends silently if stdout closes without `"busy-ready"`, and `.catch(() => {})` swallows stream errors — so if the child exits early, `await busyPromise` (L153) hangs until the harness timeout instead of reaching the diagnostic `catch` block. Mirror the stderr reader at L62-67: destructure `reject: busyReject`, call it after the loop if the marker was never seen, and forward the IIFE's `.catch` to it. (Pe
Check warning on line 7 in test/regression/issue/32548.test.ts
claude / Claude Code Review
Regression test header should be issue URL only
Per CLAUDE.md ("Regression tests get exactly one comment: the issue URL"), drop lines 2-7 — the bug-history narration belongs in the PR description (where it already is), not in the test file. Keep just `// https://github.com/oven-sh/bun/issues/32548`.
Loading