Tell a body that never arrived apart from one that will not decode
go/pkg/basecamp/vaults.go and schedules.go are the only two non-test call sites of a
generated.Parse* function, and both handed any error from it to a SPEC §6 renderer that stamps
CodeAPI, statusless, Retryable: false. But io.ReadAll(rsp.Body) is the first statement of
both generated parsers and its error is returned verbatim, so a truncated body, a reset connection
or corrupt chunk framing arrived at those renderers by exactly the same path a JSON fault does — and
came out permanently non-retryable. A caller reading Retryable treated a transient failure as
permanent; a caller reaching for a JSON error through errors.As found a transport error.
The fix: mark the origin at the read
Both call sites hold the *http.Response between the generated call and the parse, so the body is
wrapped there. A private io.ReadCloser forwards Read/Close and tags every failure with a
private bodyReadError; the two renderers gate on that marker and unwrap it, so the caller still
sees the transport's own error and no new type becomes public.
io.EOF passes through untouched, compared against that exact value rather than through
errors.Is: io.ReadAll ends its loop on err == io.EOF and treats every other value as a
failure, so matching its test exactly is what makes the marker cover precisely the set io.ReadAll
can hand back — including a wrapped EOF, which io.ReadAll would also surface as an error.
The origin is now a fact of construction rather than a judgement about an error, which is what the
three surrounding doc comments already claimed happened ("the origin is known by construction rather
than guessed"). They are now literally true instead of approximately true.
Rejected: inferring the origin from the error's type
An earlier revision of this PR gated on a deny-list of transport error types (io.ErrUnexpectedEOF,
net.Error, the context sentinels). Copilot caught that it does not close the bug, and it was
right: there is no error set to match on. net/http reports corrupt chunk framing as a bare
errors.New("invalid byte in chunk length"), the transport's automatic gunzip returns
gzip.ErrHeader, and WithTransport lets a caller supply a body whose Read returns anything at
all — none of them a net.Error, none of them a named type. All were still stamped statusless
non-retryable, which is #773 surviving under a different error shape. Inverting it into an
allow-list of encoding/json sentinels fails the other way (see case 3 below). Marking is also the
smaller change: it needs no argument about which error set is closed, so 42 lines of comment
defending a classification became 28 explaining a wrapper. One sentence in bodyReadError's doc
keeps the record of why inference was rejected, so nobody re-derives the deny-list.
Red proof, per case
Four cases per site, in one test function each so a later edit has to face all of them at once: two
body-read failures (1, 4) and two decoder failures (2, 3). Implementation stashed and restored by
cp + diff -q throughout — never git checkout --, never git stash, since the stash stack is
shared across worktrees here.
1. The original bug, against the pre-fix source (origin/main at a6d3c0ba4)
--- FAIL: TestDocumentsService_GetSeparatesTransportFailuresFromDecodeFailures
documents_test.go: expected the transport error verbatim, got a "api_error" *Error (retryable=false): GetDocument returned a body that does not decode as a document: unexpected EOF: ...
--- FAIL: TestSchedulesService_GetEntrySeparatesTransportFailuresFromDecodeFailures
schedules_test.go: expected the transport error verbatim, got a "api_error" *Error (retryable=false): GetScheduleEntry returned a body that does not decode as a schedule entry: unexpected EOF: ...
REAL_EXIT=1
Cases 2 and 3 are green pre-fix, and that is correct — pre-fix behaviour for a body that arrived
whole was already right, so both are no-regression controls, not red proofs.
2. Copilot's case, against the rejected deny-list revision
=== RUN .../a_read_failure_with_no_matchable_type_stays_the_transport's_error
documents_test.go:920: expected the read failure verbatim, got a "api_error" *Error (retryable=false): GetDocument returned a body that does not decode as a document: invalid byte in chunk length: The merge-safe Update/Edit resend this record's fields verbatim, so a malformed response cannot be written back safely. Use Replace to write the record deliberately.
schedules_test.go:1681: expected the read failure verbatim, got a "api_error" *Error (retryable=false): GetScheduleEntry returned a body that does not decode as a schedule entry: invalid byte in chunk length: The merge-safe UpdateEntry/EditEntry resend this record's fields verbatim, so a malformed response cannot be written back safely. Use ReplaceEntry to write the record deliberately.
--- FAIL: TestDocumentsService_GetSeparatesTransportFailuresFromDecodeFailures
--- PASS: .../a_body_that_stops_mid-read_stays_the_transport's_error
--- PASS: .../a_malformed_body_is_the_statusless_api_error
--- PASS: .../a_decoder_error_that_is_neither_is_still_the_statusless_api_error
--- FAIL: .../a_read_failure_with_no_matchable_type_stays_the_transport's_error
--- FAIL: TestSchedulesService_GetEntrySeparatesTransportFailuresFromDecodeFailures
(same four)
REAL_EXIT=1
The deny-list passes cases 1–3 and fails case 4 at both sites. All four pass against the marker.
3. Cases 3 and 4 fail in opposite directions
Either case alone rules out one bad gate; together they rule out inferring the origin from the
error's type at all. Each half is shown load-bearing by mutating the shipped fix, not by
assertion:
| Mutation applied to the shipped fix | Case 3 (*time.ParseError) | Case 4 (chunk framing) |
|---|---|---|
Allow-list the decoder (*json.SyntaxError / *json.UnmarshalTypeError) | FAIL | pass |
Deny-list the transport (io.ErrUnexpectedEOF / net.Error / context) | pass | FAIL |
| Marker (shipped) | pass | pass |
Allow-list mutant, applied to documents.go:
--- FAIL: .../a_decoder_error_that_is_neither_is_still_the_statusless_api_error (0.01s)
documents_test.go:907: expected *Error, got *time.ParseError: parsing time "not-a-date" as "2006-01-02T15:04:05Z07:00": cannot parse "not-a-date" as "2006"
REAL_EXIT=1
Case 4 pins its own premise — the framing error must be not io.ErrUnexpectedEOF, not a
net.Error and not a context sentinel — so if net/http ever gives it a type, the case fails
loudly instead of quietly ceasing to test what it was added for.
4. The io.EOF carve-out is load-bearing
Removing it (marking every read error, io.EOF included) reddens 69 assertions across the documents
and schedules suites, REAL_EXIT=1, starting documents_test.go:154: unexpected error: EOF. The
happy path covers that line; it is not an untested branch.
Staging the failures
A normal handler's ResponseWriter derives Content-Length from what was written and re-frames
chunked output itself, so both failures are staged by hijacking the connection and writing raw
bytes: Content-Length: 4096 followed by thirty bytes for the truncation, and
Transfer-Encoding: chunked followed by a bogus chunk length for the framing error. The helper
counts PUTs, so every case proves no write-back followed the failed read.
Gates
Run with mise activated and under LC_ALL=C, exit codes written into the log rather than echoed
after a pipe:
LC_ALL=C make go-check—REAL_EXIT=0(fmt-check, vet, golangci-lint, full test suite)LC_ALL=C make go-check-drift—REAL_EXIT=0(250 / 250 operations, no critical drift)LC_ALL=C go test -race -count=3over the affected tests —REAL_EXIT=0
No generated files, no spec, no other SDK: the other five buffer the body before decoding, as
triaged on the issue.
Closes #773