Transport parity: permanent-error classification, body-less responses, CONTROL_BYTE, producer-failure race, defaultTimeoutMs (#82) by Wahbeh-Mohammad · Pull Request #97 · dexpace/nodejs-sdk

Wahbeh-Mohammad · 2026-09-05T10:42:46Z

Closes #82. Milestone 5 of the audit remediation umbrella #67, wave 6b, on top of #81. Decision D20 in docs/audit-67-decisions.md.

The two shipped transports disagreed on four things, and a fifth was handed over from #76. Every fix moves the decision into @dexpace/transport-shared — the precedent is abort-mapping.ts — so the two adapters cannot answer differently again, and docs/sdk-documentation/write-a-transport.md tells a third transport to use the same code.

What changed

1. CONTROL_BYTE was excepting LF as well as HTAB (3c72ef0). /[\x00-\x08\x0B-\x1F\x7F]/u skips \x0A. Nothing observable changed — Headers.addInbound rejects LF and the try/catch recorded the same drop — which is exactly why it survived from Phase 8a. The class is now identical to core's hasForbiddenInboundValueByte, and the constant is exported from its module (not the barrel) so its test can read the character class directly; a test through degradeInboundHeaders passed against the broken class.

2. One classification table for a permanent native failure (73af7c5). @dexpace/transport-fetch wrapped every native rejection as TransportFailureError, which retry/classify.ts reports retryable for being an IoError — so ftp://example.com spent the caller's whole retry budget re-proving a URL that cannot change. undici refused the same condition through TERMINAL_ARGUMENT_CODES. New dispatch-classification.ts in @dexpace/transport-shared decides it once, as an allow-list of three positive recognitions:

a terminal argument code on the error or its immediate cause — undici's whole dispatcher leg, and Bun's fetch;
a TypeError with no cause, which is how undici's fetch (Node's global fetch) reports argument validation, network failures always carrying one;
a cause naming one of three WHATWG scheme refusals, the only way that same fetch can report ftp:// at all.

Everything else falls through to the retryable TransportFailureError TRANSPORT-20 makes a MUST. 'bad port' is excluded by name: port 1 is on WHATWG's blocked list, so TRANSPORT-20's own dead-port probe arrives with that reason on Node. A DexpaceError is passed through unchanged — it was classified at its source.

3. A body-less response reports body === null on both adapters (55f06ac). Three of the four native combinations disagreed: undici's dispatcher always hands back a BodyReadable; Node's fetch returns null; Bun 1.3.14's fetch returns a live ReadableStream for 204, 304 and HEAD alike (measured 2026-09-05). hasNoResponseBody(method, status) is the rule now, so the shape is the SDK's rather than the runtime's, and each adapter releases the handle it declines to expose — cancel() on fetch's, dump() on undici's, since Response.close() is a no-op on a null body and an undrained BodyReadable holds a pooled connection.

4. A producer failure aborts the native call it raced (deb32e0). send() rejected while the native call was still pending and nothing cancelled it (TRANSPORT-9). It could not: for a send with no caller signal and no composed timeout, forkSignal(undefined) returned no signal at all. ForkedSignal.signal is now always live, the interface gains abort(reason), and detach() latches it so the new direction cannot become the SEAM-16 violation the fork exists to prevent. Both transports read whether the caller aborted before pulling the fork; reading it after would surface every producer failure as a CancellationError. producerFailure now classifies its own rejection at its source — the same catch runs native rejections through a table that reads a bare TypeError as permanent, and a producer that threw one would have been mistaken for the wire refusing the request.

5. defaultTimeoutMs validated at both factories (ac26cb0, carried from #76 / D14). It reached AbortSignal.timeout() unchecked: Node throws RangeError on 1.5, 2**32 and -1; Bun 1.3.14 accepts the first two. requireValidDefaultTimeoutMs applies the identical integer 1 .. 2**32 - 1 rule RequestOptionsBuilder.timeoutMs has enforced since #76, with the identical wording, as a TypeError before anything is allocated.

Test rows added

Every row was watched red against the unfixed transport first.

Where	Rows	Red before on
`transport-conformance` (both adapters run it)	`an unsupported URL scheme fails outside the IoError tree`	fetch (undici already passed)
`transport-conformance`	204 / 304 / HEAD each asserting `body === null`, the `content-length` the case justifies, `reasonPhrase` `undefined`-or-string; plus the GET twin over the same route	both
`transport-conformance`	six unhonourable `defaultTimeoutMs` values refused at the factory, plus the in-range twin that still sends	both
`tests/node-conformance/transport.test.mjs`	unsupported scheme; null body for 204 and HEAD with the GET twin; unhonourable `defaultTimeoutMs`	fetch / undici / both respectively
`transport-shared`	12 `dispatch-classification` rows, 5 `body-less` rows, 3 `default-timeout` rows, 4 `signal-fork` rows for the two-way fork and the latch, 4 `CONTROL_BYTE` rows	n/a (new modules) or LF
`transport-fetch`	5 classification rows; 2 instrumented producer-race rows	both race rows
`transport-undici`	2 instrumented producer-race rows over a bring-your-own `Dispatcher`	both

TRANSPORT-9's race is not a shared conformance row and run-suite.ts's header says why: proving a pending native call was cancelled needs the signal the adapter handed it, which only an instrumented native client can show. The same reasoning TRANSPORT-22 already carried there.

TransportCapabilities gains a required buildWithDefaultTimeoutMs(value) — required rather than a flag because §17 assumes every transport has a default (TRANSPORT-5 is written against one), and typed number because every value the rows supply legitimately is one.

Totals after: bun run test 2526 tests across 169 files; bun run test:node 184.

Deviation rows added

One, appended to "Deviations recorded outside a phase" in docs/deviations.md per D0:

TRANSPORT-20's "any transport failure that produced no HTTP response" is read as an exchange that failed, not as a request the native client refused to make.

The reading is not new — undici has applied it since Phase 8a — but it was recorded only in that phase's checklist while @dexpace/transport-fetch did the opposite for the identical condition. The row records that the MUST is still the default, and why 'bad port' is excluded.

reasonPhrase is deliberately not re-ledgered: it is already a row beside §10 item 13 (D7).

Documentation

write-a-transport.md grows from eleven rules to thirteen: rule 4 (classify with the shared table), rule 8 (a body-less response reports body === null), rule 6 rewritten for the fork's second direction, rule 12 generalised from "refuse a proxy you cannot honour" to "refuse at construction what you cannot honour". The plumbing table and the runTransportConformanceSuite example are updated. All three package READMEs gain the new behaviour.

Two citations this branch moved are re-anchored (undici-transport.ts:138,151-158,192 → :147,160-167,201; fetch-transport.ts:76-79 → :79-82), as are the two TRANSPORT-22 test citations in run-suite.ts's header.

API reports

transport-shared.api.md regenerated: hasNoResponseBody, isPermanentDispatchFailure, toDispatchFailure, requireValidDefaultTimeoutMs, and ForkedSignal's new shape. transport-fetch and transport-undici are byte-identical — the changes there are @throws and parameter prose, which the reports do not carry.

Deferred — release machinery

Suspended for this run under D1. Recoverable:

patch changeset for @dexpace/transport-shared: CONTROL_BYTE covers LF; new @internal exports hasNoResponseBody, isPermanentDispatchFailure, toDispatchFailure, requireValidDefaultTimeoutMs; ForkedSignal.signal is no longer optional and the interface gains abort(); producerFailure rejects with a wrapped TransportFailureError.
patch changeset for @dexpace/transport-fetch: a permanent misconfiguration (unsupported scheme, forbidden method, invalid argument) is now a non-retryable TypeError rather than a retryable TransportFailureError — behaviour visible to any caller relying on the retry budget; 204/304/HEAD now carry body === null; defaultTimeoutMs outside 1 .. 2**32 - 1 now throws at the factory; a producer failure cancels the in-flight fetch. Arguably minor rather than patch, since two of the four change an observable outcome.
patch changeset for @dexpace/transport-undici: 204/304/HEAD now carry body === null instead of an empty stream — a consumer that read the body to completion still works, one that branched on body !== null does not; defaultTimeoutMs validated at the factory; a producer failure cancels the in-flight dispatch; the permanent/retryable table moved but its verdicts are unchanged.
docs/first-release.md untouched, though the transport-fetch entry above is its "free before the first bump" class.

Gate

node .claude/skills/ci-preflight/run-ci.mjs --clean, one run, from a swept tree, pinned to Bun 1.3.14: all 20 steps passed.

`degradeInboundHeaders`' inbound-value gate read `/[\x00-\x08\x0B-\x1F\x7F]/u`, which skips `\x0A` along with the intended `\x09`. Nothing observable changed — `Headers.addInbound` applies core's own `hasForbiddenInboundValueByte`, which does reject LF, and the `try`/`catch` two lines down records the same drop — so the two gates were redundant and only one of them was right. The class is now `/[\x00-\x08\x0A-\x1F\x7F]/u`, identical to core's. The constant is exported from the module (not from the barrel) so its test can read the character class directly: a test that went through `degradeInboundHeaders` would have passed against the broken class, which is how this survived from Phase 8a to audit #67 / #82. Found by: audit #67 / #82.

`@dexpace/transport-fetch` wrapped every native rejection as `TransportFailureError`, which `retry/classify.ts` reports retryable for being an `IoError`. `Request` accepts any absolute URL, so `ftp://example.com` reached `fetch`, was refused permanently, and spent the caller's whole retry budget re-proving it. `@dexpace/transport-undici` refused the same condition through `TERMINAL_ARGUMENT_CODES`: the two adapters classified one condition oppositely. The decision moves to `@dexpace/transport-shared`'s new `dispatch-classification.ts` — the precedent is `abort-mapping.ts` — as `isPermanentDispatchFailure` plus `toDispatchFailure`, and both adapters call it. It is an allow-list of three positive recognitions, so an unrecognised rejection stays the retryable `TransportFailureError` TRANSPORT-20 makes a MUST: - a terminal argument code on the error or its immediate cause, which is undici's whole dispatcher leg (`UND_ERR_INVALID_ARG`, `UND_ERR_NOT_SUPPORTED`) and Bun's `fetch` (`ERR_INVALID_ARG_VALUE` and friends); - a `TypeError` with no `cause`, which is how undici's `fetch` — Node's global `fetch` — reports argument validation, network failures always carrying one; - a cause naming one of three WHATWG scheme refusals, which is the only way that same `fetch` can report `ftp://` at all. `bad port` is deliberately excluded: port 1 is on WHATWG's blocked list, so TRANSPORT-20's own dead-port probe arrives with that reason and must stay retryable. A `DexpaceError` is passed through unchanged — it was classified at its source. Rows added, red against the unfixed fetch transport and already green against undici: `an unsupported URL scheme fails outside the IoError tree` in the shared suite (asserting `isIoError(e) === false`, which is exactly what the retry engine asks), the Node-runtime twin in `tests/node-conformance/transport.test.mjs` because the two runtimes use entirely different error shapes for it, five unit rows in `transport-fetch`, and twelve in `transport-shared`. Found by: audit #67 / #82.

…apters 204, 304, 205, 101, 103, every HEAD and a 2xx CONNECT can carry no body, and three of the four native combinations the two adapters meet disagreed about how to say so. undici's dispatcher always hands back a `BodyReadable`, so `@dexpace/transport-undici` wrapped an empty stream; Node's `fetch` returns `null` per the spec; Bun 1.3.14's `fetch` returns a live `ReadableStream` for all three (measured 2026-09-05), so `@dexpace/transport-fetch` was reporting the runtime's answer rather than the contract's. `hasNoResponseBody(method, status)` in `@dexpace/transport-shared` is now the rule and both adapters apply it, so `body === null` is a property of the SDK on every runtime. It is the WHATWG shape and the one `http/response.ts:18` already types; the rejected alternative, an empty stream on both, makes a consumer read to learn there is nothing to read. Each adapter releases the native handle it declines to expose — `cancel()` on fetch's, `dump()` on undici's. `Response.close()` is a no-op on a null body, so nobody else would, and an undrained `BodyReadable` holds the pooled connection open until the dispatcher times it out (TRANSPORT-25, SEAM-30). Rows: 204, 304 and HEAD in the shared suite, each asserting `body === null`, the `content-length` the case does or does not justify, and `reasonPhrase` as `undefined`-or-string — the fetch/undici divergence there is D7's ledger row beside §10 item 13 and is not re-ledgered. A GET over the same route is the twin, so nulling a body-less response cannot quietly null an ordinary one. All six were red on both adapters. `tests/node-conformance/transport.test.mjs` gets the runtime-divergent case, red on undici and green on fetch there, which is the asymmetry the Bun rows cannot show. `fixtures.ts`'s `route` passed the 70-line cap, so the three body-less fixtures are their own function. Found by: audit #67 / #82.

When a streaming request-body producer lost the race in `#dispatch`, `send()` rejected while the native call was still pending, and nothing cancelled it. A response arriving afterwards was dropped with its body neither read nor released — TRANSPORT-9's leak, from the request side. `abandon` unwound the producer; it could not reach the fork. It could not reach the fork because for a send with no caller signal and no composed timeout there was none: `forkSignal(undefined)` returned `{signal: undefined}` and both transports dispatched with no signal at all, which is exactly the case with nothing left to cancel with. `ForkedSignal.signal` is now always a live `AbortSignal` — one controller nobody may ever abort, and indistinguishable to the native client from no signal — and the interface gains `abort(reason)`. `detach()` latches it, so the new direction cannot become the SEAM-16 violation the fork's original direction exists to prevent. Both transports read whether the *caller* aborted before pulling the fork themselves; reading it after would surface every producer failure as a `CancellationError`. `producerFailure` now classifies its own rejection as the retryable `TransportFailureError`, which is what both catches already produced for it. That is not cosmetic: the same catch now runs native rejections through a table that reads a bare `TypeError` as a permanent misconfiguration, and a producer that threw one would have been mistaken for the wire refusing the request. `prepareBody`'s buffered branch has classified the same failure at its source since Phase 8a. Instrumented rows on both transports — a `FetchLike` and a bring-your-own `Dispatcher` whose native call resolves 30ms after the producer fails — assert the dispatched signal is aborted with the producer's error and that the late response never settles into the send. Both were red. Their twins assert a delivered response leaves the fork unaborted. `signal-fork.test.ts` gains four rows for the two-way fork and the latch. The fetch `defaultTimeoutMs` row is rewritten: "a signal was handed over" no longer discriminates anything, so it asserts the deadline is honoured instead. Found by: audit #67 / #82.

`defaultTimeoutMs` was unchecked on both transports and reached `AbortSignal.timeout()` untouched. Node throws `RangeError` on `1.5`, `2**32` and `-1`; Bun 1.3.14 accepts the first two. The same misconfigured transport therefore failed every send on one runtime and used a deadline nobody asked for on the other. It is also the last such path. `RequestOptionsBuilder.timeoutMs` has enforced the integer `1 .. 2**32 - 1` range at its setter since audit #67 / #76, on HTTP-35's reading that a timeout a setter accepted and a transport then refused belongs at the call site. `requireValidDefaultTimeoutMs` in `@dexpace/transport-shared` applies the identical rule with the identical wording, and both factories call it first thing — before `selectDispatchers` allocates, so a refusal cannot leak an `Agent` with no transport to close it through. A `TypeError`, matching the two construction-time refusals `undiciTransport` already raises and asserted the same way. `@throws` on both factories, and the `defaultTimeoutMs` TSDoc now states the range. `TransportCapabilities` gains a required `buildWithDefaultTimeoutMs(value)`: required rather than a flag because §17 assumes every transport has a default (TRANSPORT-5 is written against one), and typed `number` because every value the rows supply legitimately is one. Twelve rows, red on both adapters, plus the in-range twin that proves narrowing did not reject a legitimate default, plus the Node-runtime case — that one matters because Node is the runtime that used to fail late and loudly where Bun failed silently. Carried from audit #67 / #76 (D14's hand-off). Found by: audit #67 / #82.

`docs/deviations.md` gains one row for the reading the classification table rests on: `TRANSPORT-20`'s "any transport failure that produced no HTTP response" is read as an exchange that failed, not as a request the native client refused to make, and such a refusal is reported outside the `IoError` tree so `retry/classify.ts`'s allow-list makes it non-retryable. The reading is not new — undici has applied it since Phase 8a — but it lived only in that phase's checklist, and until this run `@dexpace/transport-fetch` did the opposite for the identical condition. The row records that the MUST is still the default: the table is an allow-list, and `bad port` is excluded by name because TRANSPORT-20's own dead-port probe arrives with that reason on Node. `reasonPhrase` is deliberately not re-ledgered — it is already a row beside §10 item 13. Two citations this branch moved are re-anchored: the SOCKS row's `undici-transport.ts:138,151-158,192` -> `:147,160-167,201` and `fetch-transport.ts:76-79` -> `:79-82`. `run-suite.ts`'s header re-anchors the two TRANSPORT-22 test citations for the same reason, and says why TRANSPORT-9's producer race is not a shared row: only an instrumented native client can show that a pending call was cancelled. READMEs: `transport-shared`'s module table gains the three new modules and the fork's second direction; both adapters' behaviour lists gain the classification rule, the body-less contract and the `defaultTimeoutMs` range.

Wahbeh-Mohammad added 6 commits September 5, 2026 13:10

Wahbeh-Mohammad merged commit 901b50e into audit/remediation-67 Sep 5, 2026
1 check passed

This was referenced Sep 5, 2026

Audit #67 remediation: milestones 1–5 (#68–#82) #98

Merged

fetch transport: permanent-error classification; both transports: 204/304/HEAD rows, CONTROL_BYTE regex, producer-failure race #82

Closed

Wahbeh-Mohammad deleted the audit/67/82-transport-parity branch September 7, 2026 18:41

Navigation Menu

Uh oh!

Transport parity: permanent-error classification, body-less responses, CONTROL_BYTE, producer-failure race, defaultTimeoutMs (#82) - #97

Transport parity: permanent-error classification, body-less responses, CONTROL_BYTE, producer-failure race, defaultTimeoutMs (#82)#97
Wahbeh-Mohammad merged 6 commits into
audit/remediation-67from
audit/67/82-transport-parity

Wahbeh-Mohammad commented Sep 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Wahbeh-Mohammad commented Sep 5, 2026

What changed

Test rows added

Deviation rows added

Documentation

API reports

Deferred — release machinery

Gate

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant