usockets: wait for the TLS spill at the HTTP close gates; drain a large https response after peer FIN - #35109
Merged
Merged
Claude / Claude Code Review
completed
Jul 22, 2026 in 14m 27s
Code review found 2 potential issues
Found 2 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/node/http/node-http-backpressure.test.ts:204-209 |
New describe("https") block detaches an existing why-comment from its test |
| 🟡 Nit | test/js/node/http/node-http-backpressure.test.ts:241-250 |
Dead halfClose parameter in https test helper |
Annotations
Check warning on line 209 in test/js/node/http/node-http-backpressure.test.ts
claude / Claude Code Review
New describe("https") block detaches an existing why-comment from its test
The new `describe("https")` block was inserted between a pre-existing why-comment (lines 198–203, "A 'drain' listener that writes again … never a torn second write") and the test it documents (`it("res.write() from 'drain' after client FIN is not torn mid-write")`, now at line 279). Appending the "Same scenario over TLS" comment onto that block and putting ~70 lines of https tests after it makes the original comment read as documentation for the https block — which has no torn-second-write case
Check warning on line 250 in test/js/node/http/node-http-backpressure.test.ts
claude / Claude Code Review
Dead halfClose parameter in https test helper
The `halfClose` parameter on `halfCloseTlsRequestBodyBytes` is dead: all three `it.each` rows pass `true`, so the `else socket.write(request)` branch is never taken. Either drop the parameter/branch (the helper's name already implies half-close) or add the intended non-half-close row to the matrix.
Loading