Repository navigation
fix(http): retry a connection reset mid-body as a transport failure - #608
Conversation
A 200 whose connection dropped while the body was being read (undici's `TypeError: terminated`) hit the same catch as JSON.parse, so it was reported as a malformed response (INTERNAL, exit 1) telling the user to check the endpoint or a captive portal, and the request was never retried. Read and parse are now separate steps. A body-read failure goes through the same transport handling as a failed fetch (shared transportRetryDelayOrThrow): retried for GET and Idempotency-Key writes within the transport budget, otherwise UNAVAILABLE (exit 10) naming the dropped connection. A body read in full that is not JSON still raises the malformed-response error with no retry; PAYLOAD_TOO_LARGE, timeouts, aborts and interrupts are unchanged. Closes TestSprite#431
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
|
| Layer / File(s) | Summary |
|---|---|
Transport failure handling and regression tests src/lib/http.ts, src/lib/http.test.ts |
A shared helper handles transport diagnostics, retry decisions, retry events, and TransportError creation. The client uses it for fetch failures and response-body read failures. Completed invalid JSON remains a typed malformed-response error. Tests cover GET and idempotent-write retries, non-retry cases, exhaustion after four attempts, malformed JSON, and debug events. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Medium
Merge Risk: 
Minimal · previous review d6017
-
The response-body retry change is ready to merge after normal checks; no outstanding issue is established.
Pre-merge checks: 
5
-
Full details: Linked Issues check
-
Explanation
The PR satisfies the coding requirements in
#431.src/lib/http.tsseparatesreadBoundedText()fromJSON.parse(). Body-read failures usetransportRetryDelayOrThrow, which applies the existing transport budget, debug events, retry transitions, andTransportError. The code preserves non-retry behavior for keyless writes andretry: false. Full-body malformed JSON still usesmalformedResponseErrorwithout retry. The new tests insrc/lib/http.test.tscover GET recovery, idempotent-write recovery, non-retriable writes, disabled retries, budget exhaustion, malformed JSON, and debug classification. The diff adds no dependency or flag.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within
#431. The production diff only refactors transport-failure handling and separates body reading from parsing. The test diff adds focused regression coverage for response-body connection resets and preserved malformed-response behavior. No unrelated feature, dependency, flag, or endpoint change is present.
-
Finishing Touches
-
Fix failing CI checks
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
ℹ️ Recent review info
-
⚙️ Run configuration
-
- Configuration used: Repository: TestSprite/testsprite-cli/.coderabbit.yaml
- Review profile: CHILL
- Plan: Advanced
- Run ID:
54b47999-bc67-4b81-8b8a-0c7b54cef33b
📒 Files selected for processing: 2
-
src/lib/http.test.ts
src/lib/http.ts
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
ℹ️ Recent review info
-
⚙️ Run configuration
-
- Configuration used: Repository: TestSprite/testsprite-cli/.coderabbit.yaml
- Review profile: CHILL
- Plan: Advanced
- Run ID:
54b47999-bc67-4b81-8b8a-0c7b54cef33b
📒 Files selected for processing: 2
-
src/lib/http.test.tssrc/lib/http.ts
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
-
|
🏆 Season 4 CLI Improvement Bonus: entry recorded. Thanks, @Davidson3556! To be counted, this PR needs the following by Oct 29, 11:59 PM PT:
Rewards go to merged PRs judged a genuine improvement, and up to three bonus-eligible PRs count per person. Untick the box to withdraw the entry. This comment refreshes when you edit the PR. Full rules. |
What does this PR do?
Sometimes a
200arrives but the connection drops while the body is still being read. Node's fetch reports that from the body reader asTypeError: terminated.http.tsread and parsed the body inside onecatch, so this landed inmalformedResponseError():INTERNAL(exit 1) with advice to checkTESTSPRITE_API_URLor a captive portal.Following the scope in the triage comment on #431:
Idempotency-Keywrites within the existing transport budget, otherwiseTransportError, which isUNAVAILABLE(exit 10) with the messageconnection closed while reading the response body: terminated.malformedResponseError(INTERNAL, exit 1), with no retry.TransportError) moved into one private helper,transportRetryDelayOrThrow. The fetch failure and the body-read failure share it, so the two can't drift apart. The fetch path behaves exactly as before.PAYLOAD_TOO_LARGEand other typed errors from the bounded readrethrowIfAbort)204+allowNoContentbypassretry: false, which are still never replayedNo new dependency.
Related issue
Closes #431
Season 4 CLI Improvement Bonus
Discord username: Davidson
Type of change
Checklist
mainbranch.(
feat(...),fix(...),docs(...), …).npm run lintandnpm run format:checkpass.npm run typecheckpasses.npm testpasses and coverage stays at or above the 80% gate.credentials required).
README.md/DOCUMENTATION.mdwhererelevant. (No doc change: this restores the documented transport-retry behaviour for one more failure point.)
Notes for reviewers
Before / after with the built CLI and a real socket. A local server answers the first request with
200headers plus{"items":[, then destroys the socket. The second request gets a healthy body.Tests (
src/lib/http.test.ts, newdescribe: "connection reset while reading a 200 body"). The reset is simulated with aReadableStreamthat delivers part of the body and then errors withTypeError('terminated')carrying anUND_ERR_SOCKETcause, which is the shape Node's fetch produces. 6 of the 7 fail onmainand pass with the fix:Idempotency-Keyis retried.TransportError/UNAVAILABLE/ exit 10, with the transport message and the requestId.retry: falsegets one attempt.UNAVAILABLE.error/retrywitherrorCode: TRANSPORT.The guard that passes before and after: a body read in full that's truncated JSON (
{"ok":) is stillINTERNAL, exit 1, single attempt. The existing non-JSON-200 tests from #166 also still pass.Results: 4805 passed / 2 skipped (unit), 110 passed / 1 skipped (e2e). Typecheck, lint and format all pass.
Summary by CodeRabbit
Review status: CodeRabbit reported no actionable comments. The branch merges cleanly with current
main.