Skip to content

test: keep the accepted socket's buffer empty in the paused peer-reset tests - #39653

Open
robobun wants to merge 1 commit into
mainfrom
farm/ad8dfe0c/reset-tests-no-unread-data
Open

test: keep the accepted socket's buffer empty in the paused peer-reset tests#39653
robobun wants to merge 1 commit into
mainfrom
farm/ad8dfe0c/reset-tests-no-unread-data

test: keep the accepted socket's buffer empty in the paused peer-rese…

98f0367
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 19, 2026 in 15m 57s

Code review found 1 important issue

Found 3 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important test/js/bun/net/socket.test.ts:4172-4176 socket.test.ts paused test rework described in PR is missing from the diff
🟡 Nit test/js/node/tls/node-tls-server.test.ts:2551-2552 peerGotData promise has no rejection path

Annotations

Check failure on line 4176 in test/js/bun/net/socket.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

socket.test.ts paused test rework described in PR is missing from the diff

The PR description says "In socket.test.ts the paused case becomes a second case of the child-peer test from #39615", but the only change to this file is this comment update — the paused test at lines 4097-4156 is untouched, and line 4140 still does `peer.write("queued behind the pause")` (23 bytes into the paused accepted socket's buffer) immediately before `peer.terminate()`. That is the "24 bytes then RST back to back: 37/40" shape from this PR's own Notes and directly violates the guidance

Check warning on line 2552 in test/js/node/tls/node-tls-server.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

peerGotData promise has no rejection path

The new `peerGotData` promise is only resolved from the peer's `data()` handler — nothing rejects it (peer `close()` is a no-op, peer `error`/`connectError` reject only `peerReady`, which is already settled by this point). If a regression breaks the write from the paused socket or the peer errors/closes before receiving "still here", `await t.peerGotData` hangs until the harness timeout instead of failing with a diagnosable message. Consider rejecting `peerGotData` from the peer's `error` and