From e7788160a326aa10e85c7df4b8d0bfefba33d893 Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Wed, 26 Aug 2026 19:38:26 +0300 Subject: [PATCH] =?UTF-8?q?feat(core):=20recovery-chain=20primitives=20?= =?UTF-8?q?=E2=80=94=20product-spec=20=C2=A78.2=20(RECOV-1..RECOV-16)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4b. Ships `packages/core/src/recovery/` — six files, no folder barrel, nothing on the published API surface (`packages/core/etc/core.api.md` is byte-identical): - `outcome.ts` Outcome, success/failure/fold (RECOV-1) - `request-chain.ts` sequential fold, empty = identity, throw propagates for the orchestrator to convert (RECOV-3, 14) - `response-chain.ts` response phase on Success only, recovery phase always, close-on-throw exactly once with the original throwable primary, no auto-close on a deliberately returned substitute (RECOV-4..9, 12..14) - `cancellation.ts` wrapCancellation — never throws, which is what keeps RECOV-2 absolute (RECOV-11) - `status-mapping.ts` a thin response step over 3b's unchanged toHttpError() (RECOV-15, 16) - `orchestrator.ts` dispatchWithRecovery — one try/catch over the request chain and the transport hop; the final unwrap rethrows by identity (RECOV-2, 10) Plus two package-root helpers: `assertNever` in `invariant.ts` (the codebase's first discriminated-union `default`) and `suppress()` in `suppress.ts`. F1, the cross-phase blocker, resolved to branch (b) --------------------------------------------------- RECOV-12 pairs a step's throwable with a close failure, which is what `SuppressedError` is for — and `SuppressedError` reached Node only in 24.0.0, against this package's `engines.node >=20.3` floor (set by `AbortSignal.any()`), and is absent from the `lib` it compiles against, so the direct form neither type-checks nor runs there. Raising the floor would drop Node 18, 20 and 22 for one error class. `suppress()` uses the native class where the runtime has one and a shape-compatible stand-in where it does not, reading the global per call. Callers assert the shape, never `instanceof SuppressedError` — that form would silently assert nothing on the floor. Phases 5a, 6a, 6b and 6c reached for the native class on the same premise; their docs now point at the helper. F2 resolved as a Deviation Ledger row: the zero-vs-fifteen `invariant()` split with 4c is project-wide, so Phase 10 settles the density rule once. Three review passes ------------------- Pass 1, against the knowledge corpus: a dead `statusMappingStep;` statement reaching the published dist/ (`satisfies` erases to its operand, not to nothing); two test files that could not survive parallel execution because they deleted a global; no type-level test for the exported generic `Outcome`; the RECOV-15 conformance clause tested on the step in isolation rather than through the chain; two step-down-rule violations. Pass 2, against the normative text: **a RECOV-8 violation** — `apply()` could raise `TypeError: undefined is not an object` when a step returned a non-outcome, against "MUST NOT throw under any input". `toFailureClosingSuccess` is now total: the discriminant read and the `close()` call share one `try`, so a misbehaving step becomes a Failure with its own throwable still primary. Also an unguarded `String()` in `assertNever`'s default message, which throws on a null-prototype object. Pass 3: re-ran every step of both CI jobs, swept the structure, and wrote what survives into `docs/open-items.md` section F. Also fixes a merge residue: the phase-3 merge left `bunfig.toml` with a duplicated `[test] root` key, which TOML rejects, so `bun test` failed to load bunfig at all on this branch. Verification (all exit 0) ------------------------- `bun install --frozen-lockfile`, typecheck, lint, build, `bun test --coverage` (588 tests / 50 files, 98.68% funcs / 99.73% lines against the 80% floor), api, lint:publish, verify:dual-consumption, verify:consumer-types, verify:seam-1, verify:runtime-floor, audit, test:node (36 cases, +1 file covering the SuppressedError guard and RECOV-12 over Node's own Web Streams), test:knowledge. No `node:` import, no `enum`, no internal barrel, SPDX on line 1 of all 15 new files, no import cycle under `packages/core/src`. Refs: dexpace/nodejs-sdk#8 --- .../2026-08-26-recovery-chain-primitives.md | 24 + docs/open-items.md | 123 ++++- .../2026-07-25-phase4b-recovery-chain.md | 214 +++++---- ...ecution-context-and-pipelines-checklist.md | 36 +- .../plans/2026-07-26-phase5a-retry.md | 12 +- .../plans/2026-07-28-phase6b-sse.md | 26 +- ...2026-07-23-nodejs-sdk-v1-roadmap-design.md | 22 +- ...026-07-25-phase4b-recovery-chain-design.md | 49 +- .../specs/2026-07-28-phase6a-serde-design.md | 10 +- .../specs/2026-07-28-phase6b-sse-design.md | 8 +- .../2026-07-28-phase6c-pagination-design.md | 6 +- packages/core/src/invariant.test.ts | 43 +- packages/core/src/invariant.ts | 29 ++ .../core/src/recovery/cancellation.test.ts | 43 ++ packages/core/src/recovery/cancellation.ts | 33 ++ .../core/src/recovery/orchestrator.test.ts | 370 +++++++++++++++ packages/core/src/recovery/orchestrator.ts | 80 ++++ packages/core/src/recovery/outcome.test.ts | 157 +++++++ packages/core/src/recovery/outcome.ts | 72 +++ .../core/src/recovery/request-chain.test.ts | 152 +++++++ packages/core/src/recovery/request-chain.ts | 53 +++ .../core/src/recovery/response-chain.test.ts | 426 ++++++++++++++++++ packages/core/src/recovery/response-chain.ts | 147 ++++++ .../core/src/recovery/status-mapping.test.ts | 154 +++++++ packages/core/src/recovery/status-mapping.ts | 41 ++ packages/core/src/suppress.test.ts | 85 ++++ packages/core/src/suppress.ts | 92 ++++ test/node-conformance/recovery-chain.test.mjs | 159 +++++++ 28 files changed, 2521 insertions(+), 145 deletions(-) create mode 100644 .changeset/2026-08-26-recovery-chain-primitives.md create mode 100644 packages/core/src/recovery/cancellation.test.ts create mode 100644 packages/core/src/recovery/cancellation.ts create mode 100644 packages/core/src/recovery/orchestrator.test.ts create mode 100644 packages/core/src/recovery/orchestrator.ts create mode 100644 packages/core/src/recovery/outcome.test.ts create mode 100644 packages/core/src/recovery/outcome.ts create mode 100644 packages/core/src/recovery/request-chain.test.ts create mode 100644 packages/core/src/recovery/request-chain.ts create mode 100644 packages/core/src/recovery/response-chain.test.ts create mode 100644 packages/core/src/recovery/response-chain.ts create mode 100644 packages/core/src/recovery/status-mapping.test.ts create mode 100644 packages/core/src/recovery/status-mapping.ts create mode 100644 packages/core/src/suppress.test.ts create mode 100644 packages/core/src/suppress.ts create mode 100644 test/node-conformance/recovery-chain.test.mjs diff --git a/.changeset/2026-08-26-recovery-chain-primitives.md b/.changeset/2026-08-26-recovery-chain-primitives.md new file mode 100644 index 0000000..a444a15 --- /dev/null +++ b/.changeset/2026-08-26-recovery-chain-primitives.md @@ -0,0 +1,24 @@ +--- +'@dexpace/core': patch +--- + +Add the recovery-chain primitives for product-spec §8.2 (`RECOV-1`–`RECOV-16`). No public API change. + +Everything this adds lives under `packages/core/src/recovery/` plus two package-root helpers, and none of it is +re-exported from `src/index.ts` — `packages/core/etc/core.api.md` is byte-identical before and after. `patch` +rather than an empty changeset because files under `packages/` did change: the published tarball carries the +new `dist/recovery/*.js` and `dist/suppress.js`, and a consumer stepping through the package in a debugger will +see them. + +What landed: `Outcome` with `success`/`failure`/`fold`; `RequestRecoveryChain` and `ResponseRecoveryChain` +(defensive copies on both, concurrency-safe by construction); `dispatchWithRecovery`, whose single `try`/`catch` +wraps both the request chain and the transport hop so no throwable from either can bypass the recovery hooks; +`wrapCancellation`; and `statusMappingStep`, a thin response step over Phase 3b's unchanged `toHttpError()`. +`assertNever` joins `invariant.ts` as the codebase's first discriminated-union `default` case. + +One consumer-visible-in-principle detail worth recording: `RECOV-12` pairs a step's throwable with a close +failure, which is what `SuppressedError` is for — and `SuppressedError` reached Node only in 24.0.0, against +this package's `>=20.3` floor. Rather than raise the floor and drop Node 18, 20 and 22 for one error class, +`suppress()` uses the native class where the runtime has one and returns a shape-compatible stand-in (`name`, +`error`, `suppressed`) where it does not. Code that catches one of these should read its fields, not test +`instanceof SuppressedError`. diff --git a/docs/open-items.md b/docs/open-items.md index 07ec5f4..1f8d44d 100644 --- a/docs/open-items.md +++ b/docs/open-items.md @@ -3,9 +3,13 @@ Running register of everything known to be unmet, unverified, misreported, or deliberately deferred across the implemented portion of this project. Reviewed state: **scaffold milestone** (committed, `0ebdc79`), **Phase 1 — Core HTTP Domain Model** (branch `2-phase-1-core-http-domain-model`, uncommitted at time of -review), and **Phase 4a — Execution Context** (branch `7-phase-4a-execution-context`, three review passes). +review), **Phase 3a/3b**, **Phase 4a — Execution Context** (branch `7-phase-4a-execution-context`, three +review passes), and **Phase 4b — Recovery-Chain Primitives** (branch +`8-phase-4b-recovery-chain-primitives`). 4a and 4b are both merged into `9-phase-4c-stage-based-pipeline`. Last reviewed **2026-08-26**. +Sections A–E below were written against Phase 1 and are re-verified at each review; section F is Phase 4b's. + A requirement absent from this file is either satisfied or belongs to a phase that has not started. The point of the file is that nothing is unmet *silently* — every gap below is either scheduled against a named phase or awaiting a decision. @@ -144,7 +148,13 @@ observable and owes a real test. ## B. Gates and tooling -### B1 — NFR-10 / NFR-17: CI never runs on the declared minimum runtime — **ACT** (trigger has now fired) +### B1 — NFR-10 / NFR-17: CI never runs on the declared minimum runtime — **RESOLVED** (2026-08-26) + +Closed by the `node-conformance` job in `.github/workflows/ci.yml`, which runs `test:node` against the built +artifact as a matrix over `['20.3.0', 'lts/*']` — the declared floor and current LTS. `test/node-conformance/` +holds 36 cases. Re-verified 2026-08-26. Original finding kept below for provenance. + +#### Original finding — **ACT** (trigger has now fired) The scaffold checklist deferred this explicitly: *"recommend adding an `actions/setup-node@v4` step pinned to `18.17` running `scripts/verify-dual-consumption.mjs` once real Node-API usage lands (Phase 1 onward), rather @@ -161,13 +171,14 @@ run" is still missing. Phase 1 established the convention ("every new source file opens with `// SPDX-License-Identifier: MIT` on line 1") and every file under `packages/core/src/http/` complies. Three files predating it do not: -- `scripts/verify-runtime-floor.mjs` -- `scripts/verify-seam-1.mjs` -- `eslint.config.js` +- ~~`scripts/verify-runtime-floor.mjs`~~ — fixed +- ~~`scripts/verify-seam-1.mjs`~~ — fixed +- `eslint.config.js` — **still missing** (re-verified 2026-08-26; line 1 is + `import {createRequire} from 'node:module';`) `scripts/verify-dual-consumption.mjs` gained one during Phase 1, which is what makes the omission of its two siblings look accidental rather than scoped. NFR-13 is a review convention, not a mechanical gate, so this is a -one-line-per-file cleanup. +one-line cleanup on the one file left. Phase 9's `NFR-13` sweep owns it if it is not done sooner. ### B3 — NFR-12: reproducible builds asserted, never proven — **WATCH** @@ -177,7 +188,8 @@ it. Becomes real at first publish (~Phase 10): build twice, diff artifact digest ### B4 — NFR-14: `expect-type` breaks the single-source-of-versions convention — **WATCH** Every other devDependency is centralized at the workspace root; Phase 1 added `expect-type` to -`packages/core/package.json`'s own `devDependencies`. Harmless with one package — it is exactly the restatement +`packages/core/package.json`'s own `devDependencies` (re-verified 2026-08-26 — still there, and Phase 4b added +three more call sites, so the convention is now load-bearing in four files rather than two). Harmless with one package — it is exactly the restatement NFR-14 warns about once a second package exists (Phase 8). Either hoist it to the root now or fold it into the NFR-14 decision at Phase 8. @@ -225,16 +237,16 @@ No action now. Each is already owned by a named phase; this table exists so none | Item | Requirement | Owner phase | Note | |---|---|---|---| -| Body lifecycle: write/replayability, single-use, close, charset | HTTP-36 – HTTP-43 | 3b | `Request`/`Response` `body` is typed `unknown` as an explicit placeholder | -| Lazy `TypedResponse` with parse-once memoization | HTTP-44, HTTP-45 | 3b | | -| `MultipartBody` — the one builder-based model HTTP-3 lists that Phase 1 did not build | HTTP-51 | 3b | Depends on body-lifecycle contracts | -| 1 MiB error-body buffering cap | HTTP-52 | 3b | | +| ~~Body lifecycle: write/replayability, single-use, close, charset~~ | HTTP-36 – HTTP-43 | 3b | **Done** — `packages/core/src/body/`, merged 2026-08-26 | +| ~~Lazy `TypedResponse` with parse-once memoization~~ | HTTP-44, HTTP-45 | 3b | **Done** — `body/typed-response.ts` | +| ~~`MultipartBody`~~ | HTTP-51 | 3b | **Done** — `body/multipart-body.ts`. Its non-appearance clause stays partial; see the roadmap's Phase-3-owned residuals | +| ~~1 MiB error-body buffering cap~~ | HTTP-52 | 3b | **Done** — `body/http-status-error.ts`; `RECOV-16` reuses it unchanged | | `Request.equals` compares body by reference, not by value | HTTP-46 (body clause) | 3b | Blocked on a real `Body` model supplying value equality | | `RequestConditions.applyTo` cannot emit an obs-text ETag | HTTP-18 vs HTTP-48/50 | 10 | Spec text in scope does not resolve the tension; strict outbound path kept rather than guessed. Documented in `applyTo`'s TSDoc | | Seam contracts (byte-stream, transport, codec, projection) | SEAM-2 – SEAM-30 | 2–8 | | | Adapter packages, peer-dependency dedup | NFR-2 | 8 | | | Shrink-survival regression guard | NFR-9 | 9 | | -| Concurrency-model agnosticism check | NFR-11 | 4c | Retargeted from "Phase 4" by the 4a design: everything in 4a is synchronous, and 4c's stage pipeline is where async-facing surface appears | +| Concurrency-model agnosticism check | NFR-11 | 4c | Retargeted from "Phase 4" by the 4a design: everything in 4a is synchronous, 4b's surface is `Promise`-only, and 4c's stage pipeline is where async-facing surface appears. 4c's plan claims closure; re-verify when 4c executes | | `CTX-17`'s positive half — the first store entry installed by the first promotion | CTX-17 | 4c | 4a satisfies only the negative half (constructing a head context must not auto-register it), which holds structurally because `context.ts` never imports `store.ts`. Wiring the store into the promotions would invert the layering and make every promotion a global side effect | | Real W3C Trace Context generation behind `InstrumentationBundle` | CTX-14, CTX-15 | 7 | 4a ships the bundle's frozen shape and the no-op default only. `activeSpan`/`tracerFactory` stay typed `unknown`, and `activeSpan` is `undefined` rather than a no-op span object, until a tracing adapter defines `Span` | | `contextsEqual()`, value equality over `ExecutionContext` | CTX-5 (equality framing) | none | Built only if 4b or 4c needs one. `CTX-5`'s operative half — pinning an explicit shared key — ships via `ContextInit.key` | @@ -260,6 +272,93 @@ departure. --- +## F. Phase 4b — Recovery-Chain Primitives + +Three review passes ran over this phase; everything they found is either fixed in the branch or listed here. +Nothing below blocks the phase — the `RECOV-1`–`RECOV-16` mapping is satisfied and every CI step is green. + +### F1 — `ResponseRecoveryChain.apply()` still trusts its *seed* outcome — **WATCH** + +`RECOV-8` is absolute: "the response recovery chain's apply operation MUST NOT throw under any input." Pass 2 +found and closed the reachable half — a *step* returning a non-outcome used to raise +`TypeError: undefined is not an object` out of `apply()`, because `toFailureClosingSuccess` read `.kind` outside +its `try`. That function is now total, and three regression tests pin it. + +What is not guarded is the seed: `apply(garbage)` with at least one response step installed throws on the +`current.kind !== 'success'` read at the loop head. Left alone deliberately — `current` is only ever the +caller's argument at that point, and the sole caller is `dispatchWithRecovery`, which constructs it with +`success()` or `wrapCancellation()`. Guarding it needs either a cast plus an optional chain (which +`no-unnecessary-condition` rejects on a typed value) or the postcondition assertions F3 defers. + +**Trigger:** `recovery/` gaining a public export, or any JavaScript caller reaching `apply()` directly. Either +makes the seed a third-party value and this a real defect. + +### F2 — A step returning a non-outcome poisons the fold silently when nothing downstream reads it — **WATCH** + +The mirror of F1 on the value side. A response step returning `undefined` yields `success(undefined)`; if no +later step touches it, `apply()` resolves with a malformed Success and `dispatchWithRecovery` hands `undefined` +back as the response. Nothing throws, so `RECOV-8` holds — the failure surfaces layers away, in the caller. + +This is the concrete cost named in the roadmap's finding F2 (assertion density), and it is why that finding is +recorded as a Deviation Ledger row rather than as "no assertions needed." **Trigger:** the same as F1, or +Phase 5's retry step being the first real third-party-shaped consumer. + +### F3 — Zero `invariant()` assertions across `recovery/` — **SCHEDULED** (Phase 10) + +`docs/knowledge/assertions.md:6-7` sets a 2-per-function module average; this phase ships none across roughly a +dozen functions. Project-wide inconsistency rather than 4b's — Phases 1/2/3b/4a ship zero, 4c's plan ships +fifteen — so adding them to 4b alone would deepen the split. Recorded in the phase design's Deviation Ledger. + +One constraint Phase 10 must carry into the decision: **at the fold sites, `invariant()` is the wrong tool.** +An `invariant()` inside `apply()` throws, and `RECOV-8` forbids `apply()` from throwing. The correct shape +there is to convert a broken postcondition into a Failure, which is what the F1 fix already does. + +### F4 — The chains are classes where `data-modeling.md:10` asks for free functions — **SCHEDULED** (Phase 10) + +`RequestRecoveryChain` / `ResponseRecoveryChain` own no lifecycle and hold no mutable state, so the corpus +would have them be plain data plus free functions. Kept as classes because `RECOV-14`'s text is written about +the chain and step *instances*, and because the defensive copy wants a construction boundary. Ledgered in the +phase design. + +### F5 — `#private` fields carry no per-use justification — **SCHEDULED** (Phase 10) + +`data-modeling.md:20-23` makes `private` the default and requires a comment justifying each `#private` as a +genuine runtime-privacy requirement. No such claim is made for either chain class — unlike 3b's `Response`, +whose `#closed` must survive `Object.freeze(this)`. `#private` is the package-wide style (Phases 1, 3b, 4a), so +this is one project-wide reconciliation, not a 4b edit. + +### F6 — `RECOV-11` is a no-op in this port — **SCHEDULED** (Phase 10, ledgered) + +`wrapCancellation(error)` is `failure(error)`. The reference re-asserts a clearable `Thread.interrupt()` flag; +`AbortSignal.aborted` is durable once fired and the SDK never holds the caller's `AbortController`, so there is +nothing to re-assert. The helper exists as the one named site where the disposition lives. If Phase 5's retry +step lands without giving it behavior, inline it there and carry the disposition wholly in the ledger. + +### F7 — `suppress()`'s branch selection is only ever half-covered on any single runtime — **WATCH** + +`suppress()` returns the native `SuppressedError` where the runtime has one and `FallbackSuppressedError` where +it does not. No test forces the other branch by deleting the global — that cannot survive parallel execution +(`docs/knowledge/testing.md:50`). Coverage comes from the `test:node` matrix instead: `lts/*` exercises the +native branch, the pinned `20.3.0` exercises the fallback. **Trigger:** if the matrix ever collapses to one +runtime, or the floor rises past Node 24 (where the fallback becomes dead code to be deleted, not guarded). + +### F8 — 4b did not depend on 4a — **RESOLVED** (2026-08-26) + +The issue lists Phases 0–4a as 4b's dependency and 4b's design says "4a (execution context, done)". Neither +was true on the `8-phase-4b-recovery-chain-primitives` branch, where `packages/core/src/context/` did not +exist: 4b turned out not to depend on it — its only imports outside `recovery/` are `http/`, +`body/http-status-error.js`, `seams/transport.js`, `invariant.js` and `suppress.js`. The design's +parenthetical is corrected. The sequencing half is now closed too: 4a and 4b are both merged into +`9-phase-4c-stage-based-pipeline`, so `context/` is present ahead of 4c, which does depend on it. + +### F9 — The Phase 4 checklist's ✅ marks for 4a and 4c are still plan-level — **WATCH** + +`plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md` now says so explicitly in its Status +line, but the §7.x and §8.1 tables read identically to the §8.2 ones that are now real. Re-scan both when their +phases execute, per this file's own maintenance rule. + +--- + ## Maintaining this file Add an entry the moment a gap is found, not when it is fixed — the failure mode this file prevents is a diff --git a/docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md b/docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md index 0316f97..cc64d7c 100644 --- a/docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md +++ b/docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md @@ -16,46 +16,38 @@ reuse Phase 3b's already-async `toHttpError()` directly. **Nothing in this phase barrel** — `recovery/` is resilience-layer plumbing; `api-extractor`'s committed report must come back byte-identical. -**Tech Stack:** TypeScript 5.8+, `fast-check` for the invariant-bearing-function property tests. No new runtime +**Tech Stack:** TypeScript 5.8+, `fast-check` for the invariant-bearing-function property tests, and the +runtime-guarded `suppress()` helper this phase adds for `RECOV-12` (see the notice below). No new runtime dependencies — `SEAM-1` untouched. -> ### ⛔ BLOCKED — do not execute this plan yet +> ### ✅ UNBLOCKED — executed 2026-08-26 > -> **`RECOV-12`'s `SuppressedError` is not available on the declared runtime floor.** An earlier draft of this -> plan claimed it was "already available since Phase 3b's checkpoint lib bump" — that is false and has been -> removed. The checkpoint raised `engines.node` only to the first release exposing `Symbol.dispose`/ -> `Symbol.asyncDispose` (`plans/2026-07-25-checkpoint-scaffold-through-phase3a.md:57`, believed `18.18.0`, -> which also forbids any further floor movement as "unreviewed drift"). Node backported those two symbols on -> its own; `SuppressedError` is a V8 global from the full Explicit Resource Management proposal and is absent -> on every 18.x runtime. +> **F1 (`SuppressedError`) resolved to branch (b): a runtime-guarded `suppress()` helper.** The roadmap's +> "F1 resolution — the verified version facts" settled the choice on evidence: `SuppressedError` belongs to the +> full Explicit Resource Management proposal, which reached Node only in **24.0.0**. Branch (a) is therefore not +> a patch bump — it means `engines.node >= 24`, dropping Node 18, 20 and 22 outright for one error class. The +> floor stayed at `>=20.3` (set by `AbortSignal.any()`), and this package's `lib` is `["ES2023", "DOM", +> "DOM.AsyncIterable"]`, which does not supply `SuppressedError`'s type either. > -> Adding `esnext.disposable` to `lib` supplies the *type* only. So `new SuppressedError(...)` at Task 3 type- -> checks, passes `bun test` locally, and then throws `ReferenceError: SuppressedError is not defined` at call -> time — precisely the `NFR-10` trap `docs/knowledge/tooling-and-quality-gates.md:60-61` describes. Task 7's -> `bun run verify:node-floor`, `bun run test:node`, and the `node-floor-conformance` job pinned to `18.17.0` -> would all fail. +> `packages/core/src/suppress.ts` ships `suppress(error, suppressed, message)`: it constructs the native class +> when `globalThis.SuppressedError` exists and a shape-compatible stand-in (`name`, `error`, `suppressed`) when +> it does not, reading the global per call rather than capturing it at module load. Task 3 calls it instead of +> `new SuppressedError(...)`, and every assertion is written against the shape rather than +> `toBeInstanceOf(SuppressedError)` — the `instanceof` form would silently assert nothing on the floor runtime. +> Both branches are forced in `suppress.test.ts`, and `test/node-conformance/recovery-chain.test.mjs` re-forces +> the guarded branch from real Node, since `bun test` alone only ever exercises whichever branch Bun takes. > -> **Needs a decision, and it is cross-phase** — Phases 5a, 6a, 6b and 6c reach for `SuppressedError` on the same -> premise (`plans/2026-07-26-phase5a-retry.md:36`, `plans/2026-07-28-phase6a-serde.md`'s `closingAfter` helper, -> `specs/2026-07-28-phase6b-sse-design.md:163`, `specs/2026-07-28-phase6c-pagination-design.md:192`), so -> whichever option lands must land in all five: +> **The same helper is the fix for Phases 5a, 6a, 6b and 6c**, which reach for `SuppressedError` on the same +> premise. Their plans now point here; each replaces `new SuppressedError(...)` with `suppress(...)` when it +> executes. > -> - **(a) Raise `engines.node`** past the first release shipping Explicit Resource Management. Consumer-visible -> breaking change, and the checkpoint forbids unsanctioned floor moves. Confirm the exact release first. -> - **(b) A runtime-guarded `suppress(primary, secondary)` helper** in `packages/core/src/`, using native -> `SuppressedError` when `globalThis.SuppressedError` exists and attaching a `suppressed` property otherwise — -> the same guarded shape the roadmap already sanctioned for `Symbol.asyncDispose`. Changes Task 3's -> `expect(wrapped).toBeInstanceOf(SuppressedError)` assertion. -> -> **Second open decision, non-blocking: assertion density.** This phase ships zero `invariant()` calls across -> roughly a dozen functions, against `docs/knowledge/assertions.md:6-7`'s 2-per-function module average. The -> concrete cost: no `apply()` checks that a step returned a value at all, so a step returning `undefined` -> poisons the fold silently and surfaces layers away. Project-wide inconsistency rather than 4b's alone — -> Phases 1/2/3b/4a ship zero, 4c ships fifteen. Resolve as either postcondition assertions at the fold sites -> or a Deviation Ledger row, ideally project-wide at Phase 10. -> -> Both items are tracked in the roadmap's "Open Findings — Phase 4b Validation Review (2026-07-28)" section. -> Everything else that review raised (F3–F10) is already applied to this plan and its design. +> **F2 (assertion density) resolved to a Deviation Ledger row**, the alternative F2 itself named. This phase +> ships zero `invariant()` calls across roughly a dozen functions, against `docs/knowledge/assertions.md:6-7`'s +> 2-per-function module average, with a named cost: no `apply()` postcondition checks that a step returned a +> value at all, so a step returning `undefined` poisons the fold silently. It is a project-wide inconsistency +> rather than 4b's — Phases 1/2/3b/4a ship zero, 4c ships fifteen — so assertions added to 4b alone would deepen +> the split rather than close it. The design's ledger carries the row; Phase 10 settles the density rule once, +> project-wide. **Prerequisite:** This plan assumes Phases 0, 1, 2, 3a, 3b, and 4a are already implemented exactly as their own plans specify. Concretely: `packages/core/src/http/*` exports `DexpaceError`, `Request`, `Response`, @@ -92,8 +84,11 @@ addition alongside the existing `invariant()`/`InvariantViolation` it already ex - **`RECOV-12`'s close-on-throw is a hand-written `try`/`catch`, never `using`/`await using`.** Native disposal's auto-generated `SuppressedError` puts the *later* error (the disposal failure) first, making it primary and the original body error `.suppressed` — the opposite of what `RECOV-12` wants (the step's original - throwable stays primary; a close failure rides along as `.suppressed`). Construct - `new SuppressedError(originalError, closeError, message)` by hand — original first. + throwable stays primary; a close failure rides along as `.suppressed`). Build it with + `suppress(originalError, closeError, message)` from `../suppress.js` — original first. **Never + `new SuppressedError(...)`:** it is absent on the declared floor (Node 24.0.0 and up only) and absent from + this package's `lib`, so the direct form neither type-checks nor runs there. Assert its shape (`name`, + `error`, `suppressed`), never `toBeInstanceOf(SuppressedError)`. - **`RECOV-13`: a step that deliberately *returns* a different outcome (no throw) is never auto-closed.** Only a caught throw triggers the close-and-wrap path. Do not add a "close whenever the outcome changes" check — that would violate `RECOV-13` by closing a response a step meant to keep alive or already closed itself. @@ -151,6 +146,8 @@ addition alongside the existing `invariant()`/`InvariantViolation` it already ex ``` packages/core/src/invariant.ts # MODIFY: add assertNever() (Task 1) packages/core/src/invariant.test.ts # MODIFY: add assertNever coverage +packages/core/src/suppress.ts # NEW: suppress(), the guarded SuppressedError (F1 (b)) (Task 1b) +packages/core/src/suppress.test.ts # NEW: both branches of the guard, forced packages/core/src/recovery/ outcome.ts # Outcome, success(), failure(), fold() (Task 1) @@ -186,7 +183,7 @@ No `recovery/index.ts` (see Global Constraints). Task 7 runs the full gate seque onFailure): R` (from `recovery/outcome.ts`). Every later task in this plan imports `Outcome`/`success`/`failure` from `outcome.js`. -- [ ] **Step 1: Write the failing test for `assertNever`** +- [x] **Step 1: Write the failing test for `assertNever`** ```typescript // packages/core/src/invariant.test.ts @@ -212,12 +209,12 @@ describe('assertNever', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/invariant.test.ts` Expected: FAIL — `assertNever is not a function` (or similar export error). -- [ ] **Step 3: Add `assertNever` to `invariant.ts`** +- [x] **Step 3: Add `assertNever` to `invariant.ts`** Append to the existing file (do not touch the existing `invariant()`/`InvariantViolation` exports): @@ -234,19 +231,19 @@ export function assertNever(value: never, message = `unreachable case: ${String( } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/invariant.test.ts` Expected: PASS, including the 2 new tests. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/invariant.ts packages/core/src/invariant.test.ts git commit -m "feat(core): add assertNever exhaustiveness helper" ``` -- [ ] **Step 6: Write the failing test for `Outcome`** +- [x] **Step 6: Write the failing test for `Outcome`** ```typescript // packages/core/src/recovery/outcome.test.ts @@ -329,12 +326,12 @@ describe('fold identity law (RECOV-1)', () => { }); ``` -- [ ] **Step 7: Run and confirm it fails** +- [x] **Step 7: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/outcome.test.ts` Expected: FAIL — `Cannot find module './outcome.js'`. -- [ ] **Step 8: Write `outcome.ts`** +- [x] **Step 8: Write `outcome.ts`** ```typescript // packages/core/src/recovery/outcome.ts @@ -379,12 +376,12 @@ export function fold(outcome: Outcome, onSuccess: (value: T) => R, onFa } ``` -- [ ] **Step 9: Run and confirm it passes** +- [x] **Step 9: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/outcome.test.ts` Expected: PASS, 7 tests. -- [ ] **Step 10: Commit** +- [x] **Step 10: Commit** ```bash git add packages/core/src/recovery/outcome.ts packages/core/src/recovery/outcome.test.ts @@ -393,6 +390,43 @@ git commit -m "feat(core): add Outcome, success/failure/fold (RECOV-1)" --- +### Task 1b: `suppress()` — the runtime-guarded `SuppressedError` (F1 branch (b)) + +**Files:** +- Create: `packages/core/src/suppress.ts` +- Create: `packages/core/src/suppress.test.ts` + +**Interfaces:** +- Consumes: nothing. +- Produces: `interface SuppressedErrorLike`, `suppress(error: unknown, suppressed: unknown, message: string): + SuppressedErrorLike`. Task 3's `toFailureClosingSuccess` is its first call site; Phases 5a, 6a, 6b and 6c are + the next ones. + +- [x] **Step 1: Write the failing test** — both branches of the guard, forced. The native branch is skipped when + the runtime has no `SuppressedError`; the fallback branch is reached by deleting the global inside a + `try`/`finally` that restores the original property descriptor, with a following test asserting the restore + actually happened. Without that, whichever branch the test runtime happens to take is the only one ever + covered, and the floor runtime takes the *other* one. + +- [x] **Step 2: Run and confirm it fails** — `Cannot find module './suppress.js'`. + +- [x] **Step 3: Write `suppress.ts`** — read `globalThis.SuppressedError` **per call**, not at module load, via + an intersection cast (`globalThis as typeof globalThis & {SuppressedError?: SuppressedErrorConstructor}`); a + cast to a bare optional-property type trips TS's weak-type check. Fall back to a module-private class + extending `Error` that sets `name = 'SuppressedError'` and assigns `error`/`suppressed` in the constructor + body — no parameter properties (`erasableSyntaxOnly`). + +- [x] **Step 4: Run and confirm it passes** — 5 tests. + +- [x] **Step 5: Commit** + +```bash +git add packages/core/src/suppress.ts packages/core/src/suppress.test.ts +git commit -m "feat(core): add a runtime-guarded suppress() helper for RECOV-12" +``` + +--- + ### Task 2: `recovery/request-chain.ts` **Files:** @@ -405,7 +439,7 @@ git commit -m "feat(core): add Outcome, success/failure/fold (RECOV-1)" - Produces: `type RequestStep = (request: Request) => Promise`, `class RequestRecoveryChain`. Task 6 (`orchestrator.ts`) imports `RequestRecoveryChain`. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/request-chain.test.ts @@ -489,12 +523,12 @@ describe('RequestRecoveryChain.apply fold law', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/request-chain.test.ts` Expected: FAIL — `Cannot find module './request-chain.js'`. -- [ ] **Step 3: Write `request-chain.ts`** +- [x] **Step 3: Write `request-chain.ts`** ```typescript // packages/core/src/recovery/request-chain.ts @@ -529,12 +563,12 @@ export class RequestRecoveryChain { } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/request-chain.test.ts` Expected: PASS, 5 tests (including the fast-check property, which itself runs 100 cases by default). -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/request-chain.ts packages/core/src/recovery/request-chain.test.ts @@ -556,7 +590,7 @@ git commit -m "feat(core): add RequestRecoveryChain (RECOV-3, RECOV-14)" Outcome) => Promise>`, `class ResponseRecoveryChain`. Task 6 imports `ResponseRecoveryChain`; Task 5's `statusMappingStep` is typed as a `ResponseStep`. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/response-chain.test.ts @@ -756,9 +790,12 @@ describe('RECOV-12: close-on-throw while holding a Success', () => { expect(result.kind).toBe('failure'); const wrapped = result.kind === 'failure' ? result.error : undefined; - expect(wrapped).toBeInstanceOf(SuppressedError); - expect((wrapped as SuppressedError).error).toBe(originalError); - expect((wrapped as SuppressedError).suppressed).toBe(closeError); + // Shape, not `toBeInstanceOf(SuppressedError)`: the native class does not exist on the floor + // runtime, so the instanceof form would assert nothing there. + expect(wrapped).toBeInstanceOf(Error); + expect((wrapped as SuppressedErrorShape).name).toBe('SuppressedError'); + expect((wrapped as SuppressedErrorShape).error).toBe(originalError); + expect((wrapped as SuppressedErrorShape).suppressed).toBe(closeError); }); }); @@ -867,16 +904,17 @@ describe('RECOV-14: steps are safe for concurrent invocation', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/response-chain.test.ts` Expected: FAIL — `Cannot find module './response-chain.js'`. -- [ ] **Step 3: Write `response-chain.ts`** +- [x] **Step 3: Write `response-chain.ts`** ```typescript // packages/core/src/recovery/response-chain.ts import type {Response} from '../http/response.js'; +import {suppress} from '../suppress.js'; import {failure, success, type Outcome} from './outcome.js'; /** @internal */ @@ -887,15 +925,17 @@ export type RecoveryStep = (outcome: Outcome) => Promise): Promise> { if (current.kind === 'success') { try { await current.value.close(); } catch (closeError) { - return failure(new SuppressedError(thrownError, closeError, 'response close failed while handling step error')); + return failure( + suppress(thrownError, closeError, 'response close failed while handling a step error'), + ); } } return failure(thrownError); @@ -953,12 +993,12 @@ export class ResponseRecoveryChain { } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/response-chain.test.ts` Expected: PASS, 13 tests (including the fast-check property and the RECOV-14 concurrency test). -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/response-chain.ts packages/core/src/recovery/response-chain.test.ts @@ -980,7 +1020,7 @@ git commit -m "feat(core): add ResponseRecoveryChain (RECOV-4..RECOV-9, RECOV-12 disposition lives — if it is still a pure pass-through once Phase 5 lands, inline it there and move the disposition wholly into Phase 10's deviation ledger rather than keeping an abstraction with no behavior. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/cancellation.test.ts @@ -1026,12 +1066,12 @@ describe('wrapCancellation (RECOV-11)', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/cancellation.test.ts` Expected: FAIL — `Cannot find module './cancellation.js'`. -- [ ] **Step 3: Write `cancellation.ts`** +- [x] **Step 3: Write `cancellation.ts`** ```typescript // packages/core/src/recovery/cancellation.ts @@ -1060,7 +1100,7 @@ export function wrapCancellation(error: unknown): Outcome { } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/cancellation.test.ts` Expected: PASS, 4 tests. @@ -1068,7 +1108,7 @@ Expected: PASS, 4 tests. `CancellationError` is imported by the *test* only (to prove a classified cancellation gets no special treatment); `cancellation.ts` itself no longer needs it, so do not add the import back to the module. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/cancellation.ts packages/core/src/recovery/cancellation.test.ts @@ -1090,7 +1130,7 @@ git commit -m "feat(core): add wrapCancellation (RECOV-11)" task in this plan -- a future consumer (Phase 5 or 4c) installs it into a `ResponseRecoveryChain`'s response-step list. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/status-mapping.test.ts @@ -1149,12 +1189,12 @@ describe('statusMappingStep (RECOV-15)', () => { }); ``` -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/status-mapping.test.ts` Expected: FAIL — `Cannot find module './status-mapping.js'`. -- [ ] **Step 3: Write `status-mapping.ts`** +- [x] **Step 3: Write `status-mapping.ts`** ```typescript // packages/core/src/recovery/status-mapping.ts @@ -1191,12 +1231,12 @@ statusMappingStep satisfies ResponseStep; `Response` is now imported (type-only) because the explicit parameter and return annotations a `function` declaration needs replace the inference the `: ResponseStep` annotation was supplying. -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/status-mapping.test.ts` Expected: PASS, 4 tests. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/status-mapping.ts packages/core/src/recovery/status-mapping.test.ts @@ -1219,7 +1259,7 @@ git commit -m "feat(core): add statusMappingStep, wiring 3b's toHttpError into t - Produces: `interface DispatchConfig`, `dispatchWithRecovery(request: Request, config: DispatchConfig): Promise`. Terminal task of this plan -- no later task consumes this file. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```typescript // packages/core/src/recovery/orchestrator.test.ts @@ -1409,12 +1449,12 @@ describe('RECOV-11: the catch routes every throwable through wrapCancellation', `CancellationError` is imported from `../seams/transport.js` (Phase 2) alongside the `Transport` type. -- [ ] **Step 2: Run and confirm it fails** +- [x] **Step 2: Run and confirm it fails** Run: `cd packages/core && bun test src/recovery/orchestrator.test.ts` Expected: FAIL — `Cannot find module './orchestrator.js'`. -- [ ] **Step 3: Write `orchestrator.ts`** +- [x] **Step 3: Write `orchestrator.ts`** ```typescript // packages/core/src/recovery/orchestrator.ts @@ -1476,12 +1516,12 @@ export async function dispatchWithRecovery(request: Request, config: DispatchCon } ``` -- [ ] **Step 4: Run and confirm it passes** +- [x] **Step 4: Run and confirm it passes** Run: `cd packages/core && bun test src/recovery/orchestrator.test.ts` Expected: PASS, 7 tests. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add packages/core/src/recovery/orchestrator.ts packages/core/src/recovery/orchestrator.test.ts @@ -1498,7 +1538,7 @@ git commit -m "feat(core): add dispatchWithRecovery orchestrator (RECOV-2, RECOV - Consumes: every preceding task. - Produces: nothing new; verifies the whole phase is green and the public surface did not move. -- [ ] **Step 1: Run the full gate sequence** +- [x] **Step 1: Run the full gate sequence** ```bash cd /home/mohammad/Projects/dexpace/nodejs-sdk @@ -1517,7 +1557,17 @@ bun run audit Expected: all exit 0. Coverage at or above the 80% aggregate floor (`NFR-5`). -- [ ] **Step 2: Verify no `node:` import crept in** +- [x] **Step 1b: Add the Node-runtime conformance case** + +`test/node-conformance/README.md`'s membership rule: a phase touching a runtime-divergent surface adds a case +there, not only to `bun test`. `SuppressedError`'s presence is exactly that divergence — Bun and current Node +ship it, the declared 20.3 floor does not — so `test/node-conformance/recovery-chain.test.mjs` forces the +guarded branch from real Node (including with the global deleted) and re-runs `RECOV-12`'s +release-exactly-once over Node's own Web Streams implementation, whose `cancel()` and reader-lock timing are +independent of Bun's. `suppress` and `recovery/` are `@internal` with no public subpath, so it imports them by +direct `dist/` path, the way `io-byte-stream.test.mjs` does. + +- [x] **Step 2: Verify no `node:` import crept in** ```bash ! grep -rn "from 'node:" packages/core/src/recovery/ @@ -1525,7 +1575,7 @@ Expected: all exit 0. Coverage at or above the 80% aggregate floor (`NFR-5`). Expected: exit 0, no matches. -- [ ] **Step 3: Verify the public API surface did not move** +- [x] **Step 3: Verify the public API surface did not move** Step 1 already regenerated the report via `bun run api`; this only inspects the result. Run from the repo root: @@ -1538,7 +1588,7 @@ Expected: **no output, exit 0.** Nothing from `src/recovery/` reached the publis 3a's/4a's gate. If this fails, remove whatever export leaked into `packages/core/src/index.ts` rather than accepting the report change. -- [ ] **Step 4: Add a changeset** +- [x] **Step 4: Add a changeset** Because nothing enters the public API, this is a patch-level, no-consumer-impact change: @@ -1549,7 +1599,7 @@ bun run changeset Select `@dexpace/core`, choose **patch**, summary: `Internal: recovery-chain primitives for product-spec §8.2 (RECOV-1..16). No public API change.` -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add .changeset/ diff --git a/docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md b/docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md index c4d49d1..856b7d2 100644 --- a/docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md +++ b/docs/superpowers/plans/2026-07-26-phase4-execution-context-and-pipelines-checklist.md @@ -10,8 +10,9 @@ Verification of the three Phase 4 implementation plans — **Legend:** ✅ Planned, implemented and tested — 🚫 Not built (permanent simplification, named reason) — ⏳ Deferred (named target phase) — N/A Not applicable in this port. -**Status:** the plans are reviewed and corrected as of 2026-07-26 (see *Review findings applied*, below) but -**not yet executed**. Every ✅ means "the plan builds and tests it," not "it is on `main`." +**Status:** the plans are reviewed and corrected as of 2026-07-26 (see *Review findings applied*, below). **4b +is executed as of 2026-08-26**; 4a and 4c are not. For the §8.2 table a ✅ now means "built, tested and on the +branch"; everywhere else it still means "the plan builds and tests it." --- @@ -77,7 +78,7 @@ Verification of the three Phase 4 implementation plans — | RECOV-9 | SHOULD | Recovery steps should return a Failure rather than throw | ✅ | Satisfied structurally — both shapes are handled identically, documented rather than enforced | | RECOV-10 | MUST | Unwrap: Success returns the response; Failure rethrows the throwable **unchanged** | ✅ | 4b Task 6 — asserted with `rejects.toBe(typedError)`, identity not message | | RECOV-11 | MUST | Wrapping a cancellation throwable re-asserts the cancellation signal | ✅ (reframed) | 4b Task 4. An `AbortSignal` is durable once fired and the SDK never holds the caller's `AbortController`, so there is nothing to re-assert; the helper is `failure(error)` and **never throws**, which is what keeps RECOV-2 absolute. Ledgered | -| RECOV-12 | MUST | A step throwing while holding a Success closes that response exactly once, close error `suppressed`, original primary | ✅ | 4b Task 3 (`toFailureClosingSuccess`, hand-built `SuppressedError` — never `using`, whose auto-generated one inverts the priority). Close observed via the body stream's `cancel()` hook, since `Response` is frozen | +| RECOV-12 | MUST | A step throwing while holding a Success closes that response exactly once, close error `suppressed`, original primary | ✅ | 4b Task 3 (`toFailureClosingSuccess`) over Task 1b's guarded `suppress()` — never `using`, whose auto-generated `SuppressedError` inverts the priority, and never `new SuppressedError(...)`, which is absent on the declared floor. Close observed via the body stream's `cancel()` hook, since `Response` is frozen. Re-forced from real Node in `test/node-conformance/recovery-chain.test.mjs` | | RECOV-13 | MUST | A deliberately *returned* different outcome is never auto-closed | ✅ | 4b Task 3 — only a caught throw reaches the close path; asserted for both a substitute Failure and a substitute Success | | RECOV-14 | MUST | Step lists immutable; response chain copies both | ✅ | 4b Tasks 2 and 3 — the request chain is copied too, which the reference does not do and the requirement's own text recommends. Ledgered | | RECOV-15 | MUST | Only 400..599 map to the typed exception; every other status passes through | ✅ | 4b Task 5, delegating to Phase 3b's unchanged `toHttpError()` | @@ -155,6 +156,7 @@ Verification of the three Phase 4 implementation plans — | Negative-space assertions | styleguide 11.9 | ✅ | Duplicate-key install, no-op closes, cross-stage edits, missing anchors, reserved SEND, continuation reuse, transport `close()` never called | | Options object over positional params | `max-params: 3` | ✅ | `ContextInit` (4a), `DispatchConfig` (4b), `CursorInit` (4c). No `eslint-disable` anywhere in Phase 4 | | Fakes over mocks; no owned interface mocked | styleguide 11.3 | ✅ | File-local `Transport` stubs throughout; no `FakeTransport`, no `mock.module`, and (as of the 2026-07-26 review) no patched `Response` method and no patched `contextStore` singleton | +| A runtime-divergent surface gets a `test/node-conformance/` case | `test/node-conformance/README.md` membership rule | ✅ | 4b: `recovery-chain.test.mjs` — `SuppressedError`'s presence is the divergence (Bun and current Node have it, the 20.3 floor does not), plus `RECOV-12`'s release-exactly-once over Node's own Web Streams | | Every test file cites its requirement IDs | Phase 1 convention, for Phase 9 | ✅ | Top-of-file comment in every test file across all three plans | | 80% aggregate coverage floor | `NFR-5` | ✅ | Each phase's gate task | @@ -197,6 +199,34 @@ stay accurate except where noted here. Full text in the roadmap's *Open Findings --- +## Phase 4b execution (2026-08-26) + +Both of 4b's open decisions closed before execution; neither changed a `RECOV-*` disposition above. + +| Item | Resolution | +|---|---| +| **F1 (blocker, cross-phase):** `SuppressedError` absent on the declared floor | Branch (b) — `packages/core/src/suppress.ts` ships `suppress(error, suppressed, message)`, native class where the runtime has one and a shape-compatible stand-in where it does not, global read per call. Branch (a) was disqualified on evidence: `SuppressedError` reached Node in **24.0.0**, so raising the floor means dropping Node 18, 20 and 22 for one error class, against a floor of `>=20.3` set by `AbortSignal.any()`. The helper discharges the obligation for 5a, 6a, 6b and 6c too — they substitute the call when they execute | +| **F2:** zero `invariant()` assertions across `recovery/` | Deviation Ledger row in 4b's design, naming the concrete cost (a step returning `undefined` poisons the fold silently). Project-wide inconsistency — 1/2/3b/4a ship zero, 4c ships fifteen — so Phase 10 settles the density rule once rather than 4b becoming the one module that differs | +| Merge residue | The phase-3 merge left `bunfig.toml` with a duplicated `[test] root` key, which TOML rejects — `bun test` failed to load bunfig at all on this branch. Fixed in its own commit before any 4b work | + +**Gate evidence (all exit 0), every step both CI jobs run, in order:** `bun install --frozen-lockfile`, +`typecheck`, `lint`, `build`, `bun test --coverage` (588 tests across 50 files; 98.68% funcs / 99.73% lines +against the 80% floor), `api` with `packages/core/etc/core.api.md` byte-identical, `lint:publish`, +`verify:dual-consumption`, `verify:consumer-types`, `verify:seam-1`, `verify:runtime-floor`, `audit`, and the +`node-conformance` job's `test:node` (36 cases, 35 before this phase). Also `test:knowledge`, which CI does not +run. Structural: no `node:` import, no `enum`, no `recovery/index.ts`, SPDX on line 1 of all 15 new files, no +import cycle anywhere under `packages/core/src`. + +**Three review passes ran before this was called done.** Pass 1 (corpus-driven) found a dead `satisfies` +statement reaching the published `dist/`, two test files that could not survive parallel execution, a missing +type-level test for the exported generic `Outcome`, an untranscribed `RECOV-15` conformance clause, and two +step-down-rule violations. Pass 2 (normative-text-driven) found a **`RECOV-8` violation**: `apply()` could +throw a `TypeError` when a step returned a non-outcome, against "MUST NOT throw under any input" — closed by +making `toFailureClosingSuccess` total; plus an unguarded `String()` in `assertNever`'s default message. Pass 3 +re-ran every CI step and swept the structure. What survives is in `docs/open-items.md` under Phase 4b. + +--- + ## Deferred out of Phase 4 | Item | Target | Note | diff --git a/docs/superpowers/plans/2026-07-26-phase5a-retry.md b/docs/superpowers/plans/2026-07-26-phase5a-retry.md index 0be4314..5ea52be 100644 --- a/docs/superpowers/plans/2026-07-26-phase5a-retry.md +++ b/docs/superpowers/plans/2026-07-26-phase5a-retry.md @@ -38,11 +38,21 @@ classification, backoff math, and pacing parsing take no I/O and no clock — dr engine, not two stacks** — `RETRY-28` explicitly instructs a unifying port to make the total-timeout opt-in, which `RetrySettings.totalTimeoutMs` does. -**Tech Stack:** TypeScript 5.8+, native `SuppressedError`, `fast-check` for the four invariant-bearing pure +**Tech Stack:** TypeScript 5.8+, Phase 4b's guarded `suppress()` helper (never native `SuppressedError` — see below), `fast-check` for the four invariant-bearing pure functions, `bun test`. No new runtime dependencies — `SEAM-1` untouched. No `node:` imports — core's zero-`node:` invariant, mechanically enforced since the scaffold, still holds (the RFC 1123 parser and the timer are both platform-neutral). +> ### ✅ F1 CLOSED — use `suppress()`, not `new SuppressedError(...)` +> +> Resolved 2026-08-26 in Phase 4b as branch (b): `packages/core/src/suppress.ts` ships +> `suppress(error, suppressed, message)` — native `SuppressedError` when `globalThis.SuppressedError` exists, a +> shape-compatible stand-in (`name`, `error`, `suppressed`) when it does not. The native class reached Node only +> in **24.0.0** and `engines.node` is `>=20.3`, so the direct form neither type-checks (not in this package's +> `lib`) nor runs on the floor. Every `new SuppressedError(...)` below becomes `suppress(...)`, and every +> `toBeInstanceOf(SuppressedError)` becomes an assertion on that shape — the `instanceof` form would silently +> assert nothing on the floor runtime. + **Prerequisite:** This plan assumes Phases 0, 1, 2, 3a, 3b, 4a, 4b, and 4c are implemented exactly as their plans specify, **plus Phase 7a's `Clock` seam** (added by the 2026-07-28 Phase 7a brainstorm's retrofit — see the "`Clock` retrofit" note in `docs/superpowers/specs/2026-07-28-phase7a-configuration-design.md`, Scope section). diff --git a/docs/superpowers/plans/2026-07-28-phase6b-sse.md b/docs/superpowers/plans/2026-07-28-phase6b-sse.md index 306c580..f2076eb 100644 --- a/docs/superpowers/plans/2026-07-28-phase6b-sse.md +++ b/docs/superpowers/plans/2026-07-28-phase6b-sse.md @@ -14,26 +14,18 @@ The parser is a stateful class (`SSE-15`/`SSE-16` need observable state, `SSE-17 generator lives one layer up in the facade, where ownership belongs. **Tech Stack:** TypeScript 5.8+, `bun test`, `fast-check` for the two chunk-independence/round-trip properties, -native `SuppressedError`. No new runtime dependencies. No `node:` imports. **No serde imports at all** — enforced +Phase 4b's guarded `suppress()` helper. No new runtime dependencies. No `node:` imports. **No serde imports at all** — enforced by a new build script, not by review. -> ### ⛔ BLOCKED on the same cross-phase item as Phase 4b — do not execute Tasks 5–7 yet +> ### ✅ F1 CLOSED — use `suppress()`, not `new SuppressedError(...)` > -> **`SuppressedError` does not exist on the declared runtime floor.** `SSE-29` and `SSE-36` are implemented here -> with `new SuppressedError(...)`, and `engines.node` is `">=18.17"`. `SuppressedError` is a V8 global from the -> full Explicit Resource Management proposal and is absent on every 18.x runtime — Node backported -> `Symbol.dispose`/`Symbol.asyncDispose` on their own, not the error type. Adding `esnext.disposable` to `lib` -> supplies the *type* only, so `new SuppressedError(...)` type-checks, passes `bun test` locally, and then throws -> `ReferenceError: SuppressedError is not defined` under Task 9's `bun run verify:node-floor` / `bun run -> test:node` on the pinned 18.17.0 runner. That is exactly the `NFR-10` trap -> `docs/knowledge/tooling-and-quality-gates.md:60-61` describes. -> -> This is **not 6b's decision to make**: `plans/2026-07-25-phase4b-recovery-chain.md:24-48` already raised it as -> a blocker naming Phases 5a, 6a, 6b and 6c, with two options on the table — raise `engines.node`, or add a -> runtime-guarded `suppress(primary, secondary)` helper in `packages/core/src/`. Whichever lands, lands in all -> five. If the guarded-helper option is chosen, every `new SuppressedError(...)` below becomes -> `suppress(primary, secondary, message)` and the `toBeInstanceOf(SuppressedError)` assertions become assertions -> on that helper's shape. Tasks 1–4 and 8 are unaffected and can proceed. +> Resolved 2026-08-26 in Phase 4b as branch (b): `packages/core/src/suppress.ts` ships +> `suppress(error, suppressed, message)`, which constructs the native `SuppressedError` when +> `globalThis.SuppressedError` exists and a shape-compatible stand-in (`name`, `error`, `suppressed`) when it +> does not. `SuppressedError` reached Node only in **24.0.0** and `engines.node` is `>=20.3`, so the direct form +> neither type-checks (it is not in this package's `lib`) nor runs on the floor. Every `new SuppressedError(...)` +> below becomes `suppress(...)`, and every `toBeInstanceOf(SuppressedError)` becomes an assertion on that shape — +> the `instanceof` form would silently assert nothing on the floor runtime. No decision left to make here. **Prerequisite:** Phases 0 through **5c** implemented as their plans specify. **6a is deliberately *not* a prerequisite** — `SSE-37` (MUST) forbids any serde dependency in core SSE, so this phase imports nothing 6a diff --git a/docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md b/docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md index 855ed04..c6fabf7 100644 --- a/docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md +++ b/docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md @@ -376,8 +376,8 @@ had. **Verify a prerequisite against the artifact it was supposed to produce, no 2. **E1** (§5.4's three parts, which do not work separately). Cheaper now than when the checkpoint was written: the new `verify:consumer-types` gate mechanically proves a `lib` entry that is declared but whose floor was not raised, and proves the reverse too. -3. **Read 4b's F1 before designing against it** — amended 2026-08-26 with the verified `SuppressedError` - version facts, which resolve it to branch (b). See "F1 resolution — the verified version facts" under +3. **F1 is closed** — resolved to branch (b) and implemented 2026-08-26 as `packages/core/src/suppress.ts`. + Read it before designing against `SuppressedError` anywhere. See "F1 resolution — the verified version facts" under "Open Findings — Phase 4b Validation Review" further down this document. That amendment changes 4b's design input, not just its wording, and F1 already notes the resolution has to land in 5a, 6b and 6c at the same time. @@ -417,18 +417,28 @@ checks out against the earlier phase plans — `toHttpError(): Promise=18.17"`, raised at most to `18.18.0` at the 2026-07-25 checkpoint (which exposes `Symbol.dispose`/`Symbol.asyncDispose` only — Node backported those two symbols; `SuppressedError` is a V8 global from the full Explicit Resource Management proposal). `esnext.disposable` in `lib` supplies its *type*, so `new SuppressedError(...)` type-checks and then throws `ReferenceError` at call time — the exact `NFR-10` trap `tooling-and-quality-gates.md:60-61` describes. `bun test` passes locally; the `node-floor-conformance` job pinned to `18.17.0`, `verify:node-floor` and `test:node` all fail | PLAN:19-20 (Tech Stack, claims it is "already available since Phase 3b's checkpoint lib bump" — false), PLAN:804, SPEC:124; also 5a plan:36, 6b design:163, 6c design:192 | **Resolved 2026-08-26: take branch (b)** — the runtime-guarded `suppress()` helper. The "confirm the first supporting Node release" condition this row left open is now discharged, and it settles the choice rather than merely informing it; two of this row's own premises also turn out to be false. See "F1 resolution — the verified version facts" below the table. **Partially applied 2026-07-28:** the false Tech Stack claim is deleted and replaced with a blocking notice at the top of the plan stating the real constraint; the mechanism itself is untouched pending implementation of (b) | -| F2 | major — OPEN | Zero assertions across the whole `recovery/` module — a dozen functions, no `invariant()` call, against `assertions.md:6-7`'s 2-per-function module average (and `styleguide-overview.md:22-23` Rule 8). Neither document acknowledges the rule or argues an exemption. Concretely: no `apply()` checks that a step returned a value at all, so a step returning `undefined` poisons the fold silently. Project-wide inconsistency, not 4b's alone — Phases 1/2/3b/4a ship zero, 4c ships fifteen | PLAN:463-479, 818-859, 964-966, 1352-1370 | **Undecided.** Either postcondition assertions at the fold sites, or a Deviation Ledger row. Worth settling at the project level (Phase 10) rather than per-phase | +| F1 | **blocker** — ✅ closed | `SuppressedError` does not exist on the declared runtime floor. `engines.node` is `">=18.17"`, raised at most to `18.18.0` at the 2026-07-25 checkpoint (which exposes `Symbol.dispose`/`Symbol.asyncDispose` only — Node backported those two symbols; `SuppressedError` is a V8 global from the full Explicit Resource Management proposal). `esnext.disposable` in `lib` supplies its *type*, so `new SuppressedError(...)` type-checks and then throws `ReferenceError` at call time — the exact `NFR-10` trap `tooling-and-quality-gates.md:60-61` describes. `bun test` passes locally; the `node-floor-conformance` job pinned to `18.17.0`, `verify:node-floor` and `test:node` all fail | PLAN:19-20 (Tech Stack, claims it is "already available since Phase 3b's checkpoint lib bump" — false), PLAN:804, SPEC:124; also 5a plan:36, 6b design:163, 6c design:192 | **Resolved 2026-08-26: take branch (b)** — the runtime-guarded `suppress()` helper. The "confirm the first supporting Node release" condition this row left open is now discharged, and it settles the choice rather than merely informing it; two of this row's own premises also turn out to be false. See "F1 resolution — the verified version facts" below the table. **Partially applied 2026-07-28:** the false Tech Stack claim is deleted and replaced with a blocking notice at the top of the plan stating the real constraint; **Applied 2026-08-26:** `packages/core/src/suppress.ts` ships the guarded helper, `response-chain.ts` calls it, and assertions are written against its shape rather than `instanceof SuppressedError` — the `instanceof` form would silently assert nothing on the floor runtime | +| F2 | major — ✅ closed | Zero assertions across the whole `recovery/` module — a dozen functions, no `invariant()` call, against `assertions.md:6-7`'s 2-per-function module average (and `styleguide-overview.md:22-23` Rule 8). Neither document acknowledges the rule or argues an exemption. Concretely: no `apply()` checks that a step returned a value at all, so a step returning `undefined` poisons the fold silently. Project-wide inconsistency, not 4b's alone — Phases 1/2/3b/4a ship zero, 4c ships fifteen | PLAN:463-479, 818-859, 964-966, 1352-1370 | **Resolved 2026-08-26: Deviation Ledger row.** Recorded in 4b's design with the concrete cost named (a step returning `undefined` poisons the fold silently). Assertions added to 4b alone would deepen the 0-vs-15 split with 4c rather than close it, so the density rule is settled once at Phase 10 and applied project-wide | | F3 | major — ✅ applied | SPEC:270 still says "the only new failure surface is `wrapCancellation()`'s `invariant()` crash" — stale text from a superseded draft. SPEC:194-204, SPEC:279 and PLAN:63-74 all state the opposite. An agent executing from the File Layout section would restore the `invariant()`, and because the helper runs inside `dispatchWithRecovery`'s own `catch`, that throw bypasses the response and recovery chains — the one failure mode `RECOV-2` exists to prevent | SPEC:270-271 | Replace with `assertNever`'s `InvariantViolation` crash, matching the already-correct PLAN:89-90 | | F4 | minor — ✅ applied | Spec never designs the `assertNever` addition Task 1 builds. PLAN modifies `packages/core/src/invariant.ts` (new exported symbol, two tests, its own commit); SPEC's File Layout lists only `recovery/` | SPEC:258-268 vs PLAN:102-103, 124-197 | Add the `invariant.ts` line to the spec's File Layout with a one-line note that `fold()` is the codebase's first discriminated-union `switch` | | F5 | minor — ✅ applied | `RECOV-14`'s second normative sentence (steps safe for concurrent invocation; per-request state never on the step instance) is claimed but neither designed nor tested — both documents cite `RECOV-14` for the defensive copy only. The design does satisfy it (all per-call state is local), but nothing records or guards that | SPEC:141-144, PLAN:49-51 | One sentence in the design + one plan test interleaving two `apply()` calls on one chain | diff --git a/docs/superpowers/specs/2026-07-25-phase4b-recovery-chain-design.md b/docs/superpowers/specs/2026-07-25-phase4b-recovery-chain-design.md index 00202d7..76b5b8b 100644 --- a/docs/superpowers/specs/2026-07-25-phase4b-recovery-chain-design.md +++ b/docs/superpowers/specs/2026-07-25-phase4b-recovery-chain-design.md @@ -1,17 +1,19 @@ # Phase 4b — Recovery-Chain Primitives — Design -**Status:** Draft, approved for planning. **⛔ Two open decisions block execution** — see the blocking notice at -the top of `docs/superpowers/plans/2026-07-25-phase4b-recovery-chain.md`. In short: `RECOV-12`'s -`SuppressedError` does not exist on the declared `engines.node` floor (a cross-phase problem shared with 5a, 6b -and 6c), and this phase's zero-assertion module contradicts the corpus's 2-per-function average. Both are -tracked in the roadmap's "Open Findings — Phase 4b Validation Review (2026-07-28)" section. The rest of that -review's findings are applied to this document. +**Status:** Implemented 2026-08-26. **Both open decisions are closed.** `RECOV-12`'s `SuppressedError` is +reached through a runtime-guarded `suppress()` helper — branch (b) of the cross-phase F1 decision, which the +roadmap resolved on the verified version facts (`SuppressedError` reached Node only in 24.0.0; branch (a) would +mean `>=24`, dropping Node 18/20/22 outright). F2 (this phase's zero `invariant()` assertions) is recorded as a +Deviation Ledger row for Phase 10's project-wide pass, the disposition F2 itself named as the alternative to +fold-site postconditions. Both are tracked in the roadmap's "Open Findings — Phase 4b Validation Review +(2026-07-28)" section. **Purpose:** Implement the recovery-chain primitives — `Outcome`, the request and response recovery chains, the unified dispatch orchestrator, the cancellation-wrapping helper, and the status→typed-exception mapping step — satisfying `docs/product-spec/08-execution-pipelines.md` §8.2 (`RECOV-1`–`RECOV-16`). This is the second of three sub-phases the roadmap's Phase 4 ("Execution Context & Pipelines") splits into: 4a (execution context, -done), **4b** (this document, `§8.2`), 4c (stage-based pipeline, `§8.1`, built on 4a+4b). +**not yet implemented** — 4b turned out not to depend on it; see `docs/open-items.md` F8), **4b** (this +document, `§8.2`), 4c (stage-based pipeline, `§8.1`, which does depend on both 4a and 4b). **Governing documents:** `docs/product-spec/08-execution-pipelines.md` §8.2/§8.3 (normative, cited by ID throughout), `docs/sdk-design-nodejs/05-pipeline-architecture.md` (Node-port mapping for both pipeline layers), @@ -113,8 +115,16 @@ declared order within each group. than throw; both are handled identically by `apply()`. - **`RECOV-12`:** when a step throws while the current outcome is a `Success` holding a response, `apply()` closes that response (`response.close()`, from Phase 3b) before wrapping the throwable into a `Failure`, - attaching any close error as `suppressed` via a manually-constructed `SuppressedError` so a close failure never - masks the primary throwable. The response is released exactly once. + attaching any close error as `suppressed` through the `suppress()` helper so a close failure never masks the + primary throwable. The response is released exactly once. + + **`SuppressedError` is not a global on the declared floor.** It belongs to the full Explicit Resource + Management proposal, which reached Node only in **24.0.0**; `engines.node` is `>=20.3` and this package's + `lib` (`ES2023`, `DOM`, `DOM.AsyncIterable`) does not even supply the *type*. `packages/core/src/suppress.ts` + wraps that gap: `suppress(error, suppressed, message)` constructs the native class when + `globalThis.SuppressedError` exists and a shape-compatible stand-in (`name`, `error`, `suppressed`) when it + does not, reading the global per call rather than at module load. Callers never branch on which one they got, + and assertions are written against the shape, never `instanceof SuppressedError`. **This must be a hand-written `try`/`catch` around the `close()` call, not `using`/`await using`.** Native disposal's own `SuppressedError` construction puts the *later* error first — when a body already threw and @@ -133,7 +143,7 @@ declared order within each group. await current.value.close(); // Response.close() is Promise (3b) -- must be awaited to be catchable } catch (closeError) { return failure( - new SuppressedError(originalError, closeError, 'response close failed while handling step error'), + suppress(originalError, closeError, 'response close failed while handling a step error'), ); } } @@ -287,6 +297,7 @@ the violation — retrofit it before Phase 3b is executed, or record it in Phase ``` packages/core/src/invariant.ts # MODIFY: add assertNever() +packages/core/src/suppress.ts # NEW: suppress(), the runtime-guarded SuppressedError (F1 branch (b)) packages/core/src/recovery/ outcome.ts # Outcome, success(), failure(), fold() @@ -297,7 +308,7 @@ packages/core/src/recovery/ status-mapping.ts # statusMappingStep() ``` -`invariant.ts` is the one file outside `recovery/` this sub-phase touches. `docs/knowledge/data-modeling.md` +`invariant.ts` and `suppress.ts` are the two files outside `recovery/` this sub-phase touches. `docs/knowledge/data-modeling.md` requires every discriminated-union `switch` to close with `default: return assertNever(x)`, "defined once and imported everywhere," and no prior phase plan actually adds it — `fold()` is the codebase's first such `switch`, so `assertNever` lands here as a small addition alongside the `invariant()`/`InvariantViolation` that module @@ -317,6 +328,9 @@ see its section above for why an `invariant()` there would violate `RECOV-2`. | No new per-status typed-exception hierarchy for `RECOV-15` | `RECOV-15`'s "matching typed exception" (which some ports read as a per-status class family) | Phase 3b's flat `HttpStatusError` (carrying `status` + buffered body) already satisfies this, and the corpus caps custom error hierarchies at two levels; a per-status class family would violate that cap | | No default/preset recovery chain shipped in 4b | none — scope decision | Matches 4a's "primitives only" discipline; Phase 5 (retry) is the first real consumer and decides its own composition | | `#private` fields and methods on both chain classes (`#steps`, `#responseSteps`, `#recoverySteps`, `#runResponsePhase`, `#runRecoveryPhase`) | `docs/knowledge/data-modeling.md:20-23` — `private` is the default; `#private` requires a comment justifying a genuine runtime-privacy requirement | **No runtime-privacy claim is made.** These classes are unfrozen holders of a readonly array, unlike 3b's `Response`, whose `#closed` genuinely must survive `Object.freeze(this)`. `#private` is the established package-wide field style (Phase 1, 3b, and 4a's `ContextStore` all use it), so switching 4b alone would fragment the package and trip the corpus's own "never mix two styles within a module/package" rule. Recorded as a project-wide deviation for Phase 10 to reconcile in one pass, not fixed here | +| `RECOV-12`'s suppressed-error pairing goes through a runtime-guarded `suppress()` helper rather than `new SuppressedError(...)` | none — a runtime-floor constraint, not a spec deviation. Listed so Phase 10 sees the shape | `SuppressedError` reached Node in 24.0.0; `engines.node` is `>=20.3` and `lib` does not supply the type. Raising the floor to reach one error class would drop Node 18, 20 and 22. The helper returns the native class where it exists and a shape-compatible stand-in where it does not, so nothing downstream branches. Phases 5a, 6a, 6b and 6c share the helper | +| `RequestRecoveryChain` / `ResponseRecoveryChain` are classes holding an immutable step array | `docs/knowledge/data-modeling.md:10` — classes are reserved for things that own a lifecycle or hold mutable runtime state behind an invariant; everything else is plain data transformed by free functions | Neither chain owns a lifecycle or mutable state — a free `applyRequestChain(steps, request)` would satisfy the corpus directly. Kept as classes because `RECOV-14`'s second clause is written about the *step instance* and the chain instance ("per-request state never on the step instance"), and because the defensive copy has to happen once at a construction boundary rather than on every call. Recorded rather than corrected: the shape is what `§8.2` describes and what Phase 5's retry step will compose against | +| Zero `invariant()` assertions across `recovery/` | `docs/knowledge/assertions.md:6-7`'s 2-per-function module average (Rule 8) | F2, deliberately not closed here. The concrete cost is named: no `apply()` postcondition checks that a step returned a value at all, so a step returning `undefined` poisons the fold silently and surfaces layers away. It is a project-wide inconsistency rather than 4b's — Phases 1/2/3b/4a ship zero, 4c ships fifteen — so adding assertions to 4b alone would deepen the split rather than close it. Phase 10 settles the density rule once and applies it everywhere | | `fold(outcome, onSuccess, onFailure)` takes three positional parameters | `docs/knowledge/function-design.md:22-23` — "an options object when it has 3 or more parameters" | The prose rule is one parameter stricter than its own stated enforcement (`max-params: ['error', 3]` errors at four), so this passes lint while violating the corpus text — flagged as a corpus conflict in the roadmap, not silently ignored. Three positional parameters match Phase 2's already-shipped `Transport.send(request, options?, signal?)`; `fold(outcome, {onSuccess, onFailure})` would make 4b the only module in the package reading differently for a canonical two-branch fold | ## Testing @@ -350,6 +364,19 @@ trigger an auto-close of the original (`RECOV-13`); `dispatchWithRecovery` rethr byte-for-byte unchanged, no wrapping (`RECOV-10`); a transport-raised `CancellationError` with no caller signal still reaches the recovery steps rather than escaping the orchestrator (`RECOV-2`/`RECOV-11`). +**Type-level tests.** `Outcome` is an exported generic type, so it ships `expectTypeOf` assertions +(styleguide 11.6): the `kind` union is closed, each variant's payload is reachable only after narrowing, and two +`@ts-expect-error` lines prove the negative — a narrowed `Success` has no `error` and a narrowed `Failure` has +no `value`. `statusMappingStep`'s conformance to `ResponseStep` is asserted the same way, in the test file +rather than as a module-level `satisfies` statement: `satisfies` erases to its operand, not to nothing, so the +module-level form leaves a dead `statusMappingStep;` expression statement in the published `dist/`. + +**Node-runtime conformance.** `SuppressedError`'s presence is exactly the kind of runtime divergence +`test/node-conformance/`'s membership rule exists for — Bun and current Node ship it, the declared floor does +not — so `recovery-chain.test.mjs` forces the guarded branch from real Node and re-runs `RECOV-12`'s +release-exactly-once over Node's own Web Streams. `bun test` alone would only ever exercise whichever branch +Bun's runtime happens to take. + **A `Response` is frozen** (Phase 1's `Object.freeze(this)`, preserved by 3b's retrofit), so no test may patch `response.close` — the assignment throws `TypeError` under an ES module's strict mode. `RECOV-12`/`RECOV-13`'s close assertions observe the body stream's `cancel()` hook instead, the way 3b's own `response.test.ts` does. diff --git a/docs/superpowers/specs/2026-07-28-phase6a-serde-design.md b/docs/superpowers/specs/2026-07-28-phase6a-serde-design.md index 264c7c7..fc96789 100644 --- a/docs/superpowers/specs/2026-07-28-phase6a-serde-design.md +++ b/docs/superpowers/specs/2026-07-28-phase6a-serde-design.md @@ -261,10 +261,12 @@ serde semantics) rather than methods. **Runtime-floor note on the close-failure path.** Preserving a decode failure as primary while carrying the close failure alongside it is what `SuppressedError` is for, and `SuppressedError` is **not available on the - declared `engines.node` floor** — it is a V8 global from the full Explicit Resource Management proposal, - absent on every 18.x runtime, and `esnext.disposable` in `lib` supplies only the type. This is the open - cross-phase decision recorded at `plans/2026-07-25-phase4b-recovery-chain.md:22-47`; 6a is a fourth site - alongside 5a, 6b and 6c, and whichever option lands there lands here unchanged. + declared `engines.node` floor** — it belongs to the full Explicit Resource Management proposal, which reached + Node only in 24.0.0, against a floor of `>=20.3`, and this package's `lib` does not supply its type either. + The cross-phase decision is **closed**: Phase 4b resolved it to branch (b) and shipped + `suppress(error, suppressed, message)` in `packages/core/src/suppress.ts` — native class where the runtime has + one, shape-compatible stand-in where it does not. 6a calls that helper and asserts its shape, never + `instanceof SuppressedError`. - `SERDE-28`: 2xx decodes. **4xx/5xx delegates to 3b's `toHttpError()`** — that function already buffers a bounded error body inside the response's own close-guaranteeing scope, at the shared 1 MiB cap `BODY-30`/`HTTP-52` define and `§14` itself points at. Building a second cap here would be a defect. Other non-2xx (1xx, an diff --git a/docs/superpowers/specs/2026-07-28-phase6b-sse-design.md b/docs/superpowers/specs/2026-07-28-phase6b-sse-design.md index 460183d..8be7cf6 100644 --- a/docs/superpowers/specs/2026-07-28-phase6b-sse-design.md +++ b/docs/superpowers/specs/2026-07-28-phase6b-sse-design.md @@ -193,9 +193,9 @@ internal release routine, not inferred from context: consumer already received, which is the specific harm `SSE-30` names. - Release on an **explicit `close()`** — a failing close propagates. The caller asked; the caller hears. - Release with **an error already in flight** (`SSE-29`, `SSE-36`) — the primary error propagates with the close - failure attached via native `SuppressedError`, the same mechanism 5a uses. + failure attached via Phase 4b's guarded `suppress()` helper, the same mechanism 5a uses. -> **`SuppressedError` is blocked on a cross-phase decision, and 6b does not get to make it.** +> **`SuppressedError` was blocked on a cross-phase decision; it is now closed and 6b just calls the helper.** > `plans/2026-07-25-phase4b-recovery-chain.md:24-48` establishes that `SuppressedError` is a V8 global from the > full Explicit Resource Management proposal, absent on every 18.x runtime — Node backported > `Symbol.dispose`/`Symbol.asyncDispose` alone — while `engines.node` is `">=18.17"` and `verify:node-floor` @@ -325,7 +325,7 @@ Phase 9's sweep reads this table rather than re-deriving it. |---|---|---| | `BufferedSource` (`overStream`, `exhausted`, `readByte`, `readExactly`, `peek`, `close`) | 3a | `peek()` is `SSE-12`'s lookahead; `close()` is already idempotent and already rejects later reads, covering most of `SSE-27`/`SSE-28`. `exhausted()` — not a sentinel from `readByte()` — is how end of stream is detected, because `readByte()` rejects rather than returning one | | `IoError` | 3a | Every read-path failure surfaces as one shape, including the torn-down-mid-read case | -| `SuppressedError` usage pattern | 5a | `SSE-29`/`SSE-36`'s "close failure attached to the primary" is the identical mechanism — **and inherits 5a's unresolved blocker**, see below | +| `suppress()` usage pattern | 4b, 5a | `SSE-29`/`SSE-36`'s "close failure attached to the primary" is the identical mechanism, over 4b's guarded helper. The former cross-phase blocker is closed | | `Response.body` / `Response.close()` | 3b | `sseStreamFrom` binds to them; it does not reach for a transport — which is also half of why `SSE-38` holds by construction | | The `kind`-discriminated union idiom | 4b | `MapperOutcome` mirrors `Outcome`'s shape without extending its type | @@ -408,4 +408,4 @@ would publish a way to violate `SSE-17`'s non-ownership contract by accident. | `SSE-37` and `SSE-38` enforced by a build script, not by module-graph structure | `SSE-37`, `SSE-38` | The reference gets it free from package boundaries; this port puts serde in the same package, so the invariant needs a mechanical guard or it is only a convention. The script strips comments before the `SSE-38` marker scan and skips `*.test.ts` for markers only — a gate that failed on a TSDoc *documenting* the absence of reconnection would be deleted rather than obeyed | | `[Symbol.asyncDispose]` on `SseStream` is optional and runtime-guarded, not an `implements AsyncDisposable` | `styleguide/typescript/13` §13.1–13.2 | The symbol postdates the declared `>=18.17` floor that `verify:node-floor` pins, and TypeScript does not polyfill it for a declaring library. `close()` stays the supported path everywhere; dispose delegates to it. Cost: `await using` does not type-check against an optional member. Unconditional once the floor moves past 18.18 | | Byte-at-a-time line framing via `readByte()` | `docs/knowledge/performance.md:16-17` | `readByte()` is `readExactly(1)` underneath, so framing allocates per byte on the parse path. Deferred deliberately on the guide's own terms (`performance.md:4,24` — no micro-fix before a profile names the bottleneck) and recorded so Phase 10 revisits it with a `*.bench.ts` instead of rediscovering it. A bulk-`read()` framing is the fix if a profile calls for one; no observable contract changes | -| `SSE-29`/`SSE-36` construct a native `SuppressedError` | `NFR-10` / the declared `>=18.17` floor | Not 6b's deviation to take or reverse — inherited from the cross-phase blocker at `plans/2026-07-25-phase4b-recovery-chain.md:24-48`, which must resolve across 4b/5a/6a/6b/6c together. Listed here so Phase 10 sees 6b in that set | +| `SSE-29`/`SSE-36` pair errors through `suppress()` rather than native `SuppressedError` | none — a runtime-floor constraint, not a spec deviation | Inherited from 4b's F1 resolution (branch (b), 2026-08-26): the native class reached Node in 24.0.0, against a `>=20.3` floor. Listed here so Phase 10 sees 6b in that set | diff --git a/docs/superpowers/specs/2026-07-28-phase6c-pagination-design.md b/docs/superpowers/specs/2026-07-28-phase6c-pagination-design.md index f57f798..b540d3b 100644 --- a/docs/superpowers/specs/2026-07-28-phase6c-pagination-design.md +++ b/docs/superpowers/specs/2026-07-28-phase6c-pagination-design.md @@ -197,7 +197,7 @@ one of them would get an error instead of a walk. So `pages()` mints a fresh sin `items()` is deliberately not single-use at either level: `PAGE-14` scopes the restriction to the page-level view, and `PAGE-8` requires two independent iterations to work. -`PAGE-15`'s two-close-failures case uses native `SuppressedError`, the same mechanism 5a and 6b use — and it +`PAGE-15`'s two-close-failures case uses Phase 4b's guarded `suppress()` helper (never native `SuppressedError`, which reached Node only in 24.0.0 against a `>=20.3` floor), the same mechanism 5a and 6b use — and it applies to the walk's *own* failure paths too, not only to back-to-back close failures. A generator whose body throws and whose `finally` then fails to close the held page would surface the close error and lose the transport or parse failure the caller actually needs. So the drive routine catches, releases with the walk's failure kept @@ -377,7 +377,7 @@ async generator *is* the engine.** | `Transport` | 2 | `PAGE-25`'s transport-agnosticism; `Runtime` (4c) satisfies it, so a resilience pipeline drops in unchanged | | `RequestOptions` | 1 | `PAGE-36` threads the caller's instance; the engine never constructs one | | `FakeTransport`, `countingResponse()` | 5a | Scripted multi-response sequences, wire-send counting, and per-response close observation are exactly what `PAGE-6`/`PAGE-9`/`PAGE-27` need. 5a's design names `countingResponse()`'s `cancel()` hook as the **only** sanctioned way to observe a close — responses are frozen, so a spy assignment throws | -| `SuppressedError` usage pattern | 5a, 6b | `PAGE-13` and `PAGE-15` both need primary-plus-suppressed | +| `suppress()` usage pattern (never native `SuppressedError`) | 4b, 5a, 6b | `PAGE-13` and `PAGE-15` both need primary-plus-suppressed | ## File Layout @@ -442,7 +442,7 @@ implementation details; publishing them would publish a second URL-manipulation - The same iterator-level single-use assertion on `paginateWithFetchers()`' returned view, paired with a `firstCalls === 1` check — an unguarded view re-runs the first-page fetcher and breaks `PAGE-34` outright. - A transport failure surfacing unwrapped (`PAGE-28`), and a transport failure whose held-page release *also* - fails surfacing as `SuppressedError` with the transport failure primary (`PAGE-15`). + fails surfacing as a `suppress()` pairing with the transport failure primary (`PAGE-15`). - The capped fetcher walk asserting the next-page fetcher ran `N-1` times, not `N`, and that every page fetched was also closed — a cap checked at the wrong end of the loop over-fetches by one and leaks the page it refuses to deliver. diff --git a/packages/core/src/invariant.test.ts b/packages/core/src/invariant.test.ts index be3fd82..118975c 100644 --- a/packages/core/src/invariant.test.ts +++ b/packages/core/src/invariant.test.ts @@ -1,8 +1,10 @@ // SPDX-License-Identifier: MIT // packages/core/src/invariant.test.ts -// Exercises: the project's sole assertion primitive (styleguide 5.6) and its error class. +// Exercises: the project's sole assertion primitive (styleguide 5.6), its error class, and the +// discriminated-union exhaustiveness helper docs/knowledge/data-modeling.md requires every switch to +// close with. import {describe, expect, test} from 'bun:test'; -import {invariant, InvariantViolation} from './invariant.js'; +import {assertNever, invariant, InvariantViolation} from './invariant.js'; describe('invariant', () => { test('does not throw when the condition is truthy', () => { @@ -27,3 +29,40 @@ describe('invariant', () => { expect(error.message).toBe('boom'); }); }); + +describe('assertNever', () => { + test('throws InvariantViolation naming the unreachable value', () => { + expect(() => { + // @ts-expect-error -- deliberately calling with a value that is not `never`, to exercise the + // runtime path a newly-added union variant would reach. + assertNever('unexpected-variant'); + }).toThrow(InvariantViolation); + expect(() => { + // @ts-expect-error -- same as above. + assertNever('unexpected-variant'); + }).toThrow('unexpected-variant'); + }); + + test('does not throw from its own message construction on an unstringifiable value', () => { + // `String()` is not total: a null-prototype object has no `toString` to reach, and a value + // whose `toString` throws propagates that throw. An assertion helper that reported THOSE + // instead of the invariant violation would name the wrong failure at the worst moment. + expect(() => { + assertNever(Object.create(null) as never); + }).toThrow(InvariantViolation); + expect(() => { + assertNever({ + toString() { + throw new Error('boom'); + }, + } as never); + }).toThrow(InvariantViolation); + }); + + test('accepts a custom message', () => { + expect(() => { + // @ts-expect-error -- same as above. + assertNever('x', 'custom message'); + }).toThrow('custom message'); + }); +}); diff --git a/packages/core/src/invariant.ts b/packages/core/src/invariant.ts index f0da862..d450d00 100644 --- a/packages/core/src/invariant.ts +++ b/packages/core/src/invariant.ts @@ -29,3 +29,32 @@ export class InvariantViolation extends Error { export function invariant(cond: unknown, msg: string): asserts cond { if (!cond) throw new InvariantViolation(msg); } + +/** + * Closes an exhaustive discriminated-union `switch`'s `default` case + * (`docs/knowledge/data-modeling.md`). If a new union variant is ever added without a matching + * `case`, the call stops type-checking; if one reaches this at runtime anyway — a value crossing a + * seam that the type says cannot exist — it crashes loudly rather than falling through silently. + * + * @internal + */ +export function assertNever(value: never, message?: string): never { + throw new InvariantViolation( + message ?? `unreachable case: ${describe(value)}`, + ); +} + +/** + * `String(value)` is not total: it throws on a null-prototype object (no `toString` to reach) and + * on any value whose `toString`/`Symbol.toPrimitive` throws — the same hazard + * `docs/knowledge/error-handling.md:18` makes `toError` guard. An assertion helper that throws from + * its own message construction reports the wrong failure at the worst moment, so the fallback is a + * fixed string. + */ +function describe(value: unknown): string { + try { + return String(value); + } catch { + return 'an unstringifiable value'; + } +} diff --git a/packages/core/src/recovery/cancellation.test.ts b/packages/core/src/recovery/cancellation.test.ts new file mode 100644 index 0000000..c275b53 --- /dev/null +++ b/packages/core/src/recovery/cancellation.test.ts @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/cancellation.test.ts +// Exercises: RECOV-11 (wrapping a cancellation throwable into a Failure), reframed for Node — an +// AbortSignal is durable once aborted and the SDK never holds the caller's AbortController, so +// there is nothing to re-assert. What the requirement still buys is the guarantee that a +// cancellation surfaces through the SAME Failure channel as every other throwable, never through a +// side exit (RECOV-2). +import {describe, expect, test} from 'bun:test'; +import {CancellationError} from '../seams/transport.js'; +import {wrapCancellation} from './cancellation.js'; + +describe('wrapCancellation (RECOV-11)', () => { + test('wraps a CancellationError into a Failure carrying it unchanged', () => { + const error = new CancellationError('cancelled by caller'); + + const outcome = wrapCancellation(error); + + expect(outcome.kind).toBe('failure'); + expect(outcome.kind === 'failure' && outcome.error).toBe(error); + }); + + test('wraps an ordinary error into a Failure carrying it unchanged', () => { + const error = new Error('an ordinary failure'); + + const outcome = wrapCancellation(error); + + expect(outcome.kind).toBe('failure'); + expect(outcome.kind === 'failure' && outcome.error).toBe(error); + }); + + test('wraps a non-Error throw unchanged — a JS throw can raise any value', () => { + const outcome = wrapCancellation('a string throw'); + + expect(outcome.kind === 'failure' && outcome.error).toBe('a string throw'); + }); + + test('never throws, for any input', () => { + // RECOV-2 depends on this: dispatchWithRecovery calls it from inside its own catch, so a throw + // here would let a transport failure bypass the response and recovery chains entirely. + expect(() => wrapCancellation(new CancellationError('x'))).not.toThrow(); + expect(() => wrapCancellation(undefined)).not.toThrow(); + }); +}); diff --git a/packages/core/src/recovery/cancellation.ts b/packages/core/src/recovery/cancellation.ts new file mode 100644 index 0000000..edd053b --- /dev/null +++ b/packages/core/src/recovery/cancellation.ts @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/cancellation.ts +import {failure, type Outcome} from './outcome.js'; + +/** + * Wraps a cancellation or interruption throwable into a Failure (RECOV-11). + * + * The reference requires re-asserting the cancellation signal on the current context when wrapping, + * so code later blocked on the outcome still observes cancellation — a concern specific to a + * clearable `Thread.interrupt()` flag. Node has nothing to re-assert: an `AbortSignal` stays + * aborted once fired, and the SDK holds a signal, never the caller's `AbortController`, so it could + * not set one anyway. The helper therefore degenerates to `failure(error)`, and exists as the one + * named, findable site where RECOV-11's Node disposition lives. + * + * It deliberately does **not** crash on a `CancellationError` whose paired signal never aborted. + * `Transport` is a pluggable seam, so that mismatch is a misbehaving third-party implementation — + * an operational failure, not a violated precondition of this codebase, and crash-loud treatment is + * reserved for the latter (`docs/knowledge/error-handling.md`). It would also break RECOV-2: this + * runs inside `dispatchWithRecovery`'s own `catch`, so throwing here would let a transport failure + * skip the response and recovery chains entirely, which is precisely what RECOV-2 forbids. A + * transport that aborts its in-flight requests from `close()` — which SEAM-14 permits — produces + * exactly that shape while the caller passed no signal at all. + * + * This function never throws, for any input. + * + * @param error - whatever the request chain or the transport raised. + * @returns a failure outcome carrying `error` unchanged. + * + * @internal + */ +export function wrapCancellation(error: unknown): Outcome { + return failure(error); +} diff --git a/packages/core/src/recovery/orchestrator.test.ts b/packages/core/src/recovery/orchestrator.test.ts new file mode 100644 index 0000000..aae4ec0 --- /dev/null +++ b/packages/core/src/recovery/orchestrator.test.ts @@ -0,0 +1,370 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/orchestrator.test.ts +// Exercises: RECOV-2 (one try/catch wraps the request chain AND the transport invocation; no +// throwable from either bypasses the recovery hooks), RECOV-10 (unwrap: a Success returns the +// response, a Failure rethrows the throwable unchanged, no wrapping or substitution), RECOV-11 (the +// catch routes every throwable through wrapCancellation, so the helper sits on the real dispatch +// path rather than being an unwired primitive) +import {describe, expect, test} from 'bun:test'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {RequestOptions} from '../http/request-options.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {CancellationError, type Transport} from '../seams/transport.js'; +import {dispatchWithRecovery} from './orchestrator.js'; +import {success} from './outcome.js'; +import {RequestRecoveryChain, type RequestStep} from './request-chain.js'; +import { + ResponseRecoveryChain, + type RecoveryStep, + type ResponseStep, +} from './response-chain.js'; + +function aRequest(): Request { + return Request.newBuilder().url('https://example.com').build(); +} + +function aResponse( + request: Request, + body: ReadableStream | null = null, +): Response { + return Response.newBuilder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .body(body) + .build(); +} + +/** Close is observed through the body stream's `cancel()` — `Response` is frozen. */ +function countingCloseBody(): { + body: ReadableStream; + closeCount: () => number; +} { + let cancels = 0; + return { + body: new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + cancels += 1; + }, + }), + closeCount: () => cancels, + }; +} + +/** Awaits `promise`, returning whatever it rejected with — `undefined` when it resolved. */ +async function rejection(promise: Promise): Promise { + try { + await promise; + return undefined; + } catch (error) { + return error; + } +} + +type SendImpl = ( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, +) => Promise; + +/** A minimal, file-local Transport stub — no shared FakeTransport exists yet. */ +class StubTransport implements Transport { + readonly #impl: SendImpl; + + constructor(impl: SendImpl) { + this.#impl = impl; + } + + send( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, + ): Promise { + return this.#impl(request, options, signal); + } + + close(): Promise { + return Promise.resolve(); + } +} + +function emptyChains(): { + requestChain: RequestRecoveryChain; + responseChain: ResponseRecoveryChain; +} { + return { + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], []), + }; +} + +describe('dispatchWithRecovery happy path', () => { + test('returns the transport response when everything succeeds', async () => { + const request = aRequest(); + const response = aResponse(request); + const transport = new StubTransport(() => Promise.resolve(response)); + + const result = await dispatchWithRecovery(request, { + transport, + ...emptyChains(), + }); + + expect(result).toBe(response); + }); + + test('sends the request the request chain produced, not the one the caller handed in', async () => { + const tagStep: RequestStep = request => + Promise.resolve( + request + .newBuilder() + .headers( + request.headers.newBuilder().set('X-Trace', 'tagged').build(), + ) + .build(), + ); + let sent: Request | undefined; + const transport = new StubTransport(request => { + sent = request; + return Promise.resolve(aResponse(request)); + }); + + await dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([tagStep]), + responseChain: new ResponseRecoveryChain([], []), + }); + + expect(sent?.headers.get('X-Trace')).toBe('tagged'); + }); + + test('threads per-call options and signal through to the transport unchanged', async () => { + const request = aRequest(); + const options = RequestOptions.EMPTY; + const controller = new AbortController(); + let receivedOptions: RequestOptions | undefined; + let receivedSignal: AbortSignal | undefined; + const transport = new StubTransport((req, opts, signal) => { + receivedOptions = opts; + receivedSignal = signal; + return Promise.resolve(aResponse(req)); + }); + + await dispatchWithRecovery(request, { + transport, + ...emptyChains(), + options, + signal: controller.signal, + }); + + expect(receivedOptions).toBe(options); + expect(receivedSignal).toBe(controller.signal); + }); +}); + +describe('RECOV-2: every throwable from the request chain or the transport is caught', () => { + test('a throwing request step surfaces as a Failure to a recovery hook, not an unhandled throw', async () => { + const thrownError = new Error('request step failed'); + const failingStep: RequestStep = () => { + throw thrownError; + }; + const seenByRecovery: unknown[] = []; + const recoveryStep: RecoveryStep = outcome => { + if (outcome.kind === 'failure') seenByRecovery.push(outcome.error); + return Promise.resolve(outcome); + }; + const transport = new StubTransport(() => { + throw new Error('must not run — the request chain already failed'); + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([failingStep]), + responseChain: new ResponseRecoveryChain([], [recoveryStep]), + }), + ); + + expect(error).toBe(thrownError); + expect(seenByRecovery).toEqual([thrownError]); + }); + + test('a throwing transport surfaces as a Failure to a recovery hook, not an unhandled throw', async () => { + const thrownError = new Error('transport failed'); + const seenByRecovery: unknown[] = []; + const recoveryStep: RecoveryStep = outcome => { + if (outcome.kind === 'failure') seenByRecovery.push(outcome.error); + return Promise.resolve(outcome); + }; + const transport = new StubTransport(() => { + throw thrownError; + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], [recoveryStep]), + }), + ); + + expect(error).toBe(thrownError); + expect(seenByRecovery).toEqual([thrownError]); + }); + + test('a recovery step can turn a transport failure back into a Success', async () => { + const request = aRequest(); + const fallback = aResponse(request); + const recoverStep: RecoveryStep = outcome => + Promise.resolve(outcome.kind === 'failure' ? success(fallback) : outcome); + const transport = new StubTransport(() => { + throw new Error('transport failed'); + }); + + const result = await dispatchWithRecovery(request, { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], [recoverStep]), + }); + + expect(result).toBe(fallback); + }); +}); + +describe('RECOV-10: the final unwrap is unchanged, no wrapping or substitution', () => { + test('a response step throwing a typed error surfaces exactly that error, by identity', async () => { + class MyTypedError extends Error {} + const typedError = new MyTypedError('mapped'); + const mapToTypedError: ResponseStep = () => { + throw typedError; + }; + const transport = new StubTransport(request => + Promise.resolve(aResponse(request)), + ); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([mapToTypedError], []), + }), + ); + + expect(error).toBe(typedError); + }); + + test('a non-Error throwable is rethrown as-is, not coerced into an Error', async () => { + const transport = new StubTransport(() => { + // The point of the case: a JS throw can legally raise any value, and RECOV-10 requires the + // orchestrator to rethrow it by identity rather than coercing it into an Error. Re-enable if + // the transport seam ever narrows what an implementation may reject with. + // eslint-disable-next-line @typescript-eslint/only-throw-error -- see the comment above + throw 'a string throw'; + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), {transport, ...emptyChains()}), + ); + + expect(error).toBe('a string throw'); + }); +}); + +describe('RECOV-11: the catch routes every throwable through wrapCancellation', () => { + test('a transport CancellationError paired with an aborted signal surfaces unchanged', async () => { + const controller = new AbortController(); + const cancellation = new CancellationError('aborted by caller'); + const transport = new StubTransport(() => { + controller.abort(); + throw cancellation; + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], []), + signal: controller.signal, + }), + ); + + expect(error).toBe(cancellation); + }); + + test('a CancellationError raised with no caller signal still reaches the recovery chain (RECOV-2)', async () => { + // A transport may abort its own in-flight requests for reasons the caller never signalled — + // SEAM-14 permits close() to cancel them. That must surface as an ordinary Failure through the + // recovery hooks, not as a side exit: RECOV-2 admits no throwable from the transport bypassing + // them. + const cancellation = new CancellationError( + 'aborted by the transport itself', + ); + const seenByRecovery: unknown[] = []; + const recoveryStep: RecoveryStep = outcome => { + if (outcome.kind === 'failure') seenByRecovery.push(outcome.error); + return Promise.resolve(outcome); + }; + const transport = new StubTransport(() => { + throw cancellation; + }); + + const error = await rejection( + dispatchWithRecovery(aRequest(), { + transport, + requestChain: new RequestRecoveryChain([]), + responseChain: new ResponseRecoveryChain([], [recoveryStep]), + }), + ); + + expect(error).toBe(cancellation); + expect(seenByRecovery).toEqual([cancellation]); + }); +}); + +describe('negative space: the orchestrator releases nothing of its own', () => { + test('the response it hands back is left open for the caller to close', async () => { + // RECOV-10 returns the contained response; ownership passes to the caller. A future + // "helpful" close here would hand back a response whose body is already cancelled. + const request = aRequest(); + const {body, closeCount} = countingCloseBody(); + const transport = new StubTransport(() => + Promise.resolve(aResponse(request, body)), + ); + + const result = await dispatchWithRecovery(request, { + transport, + ...emptyChains(), + }); + + expect(closeCount()).toBe(0); + expect(result.body).not.toBeNull(); + }); + + test('it never closes the transport it was handed', async () => { + // SEAM-14/PIPE-27's discipline, asserted early: the orchestrator borrows the transport, it + // does not own it. + let closeCalls = 0; + const request = aRequest(); + const transport = new StubTransport(() => + Promise.resolve(aResponse(request)), + ); + const countingTransport: Transport = { + send: (...args) => transport.send(...args), + close: () => { + closeCalls += 1; + return Promise.resolve(); + }, + }; + + await dispatchWithRecovery(request, { + transport: countingTransport, + ...emptyChains(), + }); + + expect(closeCalls).toBe(0); + }); +}); diff --git a/packages/core/src/recovery/orchestrator.ts b/packages/core/src/recovery/orchestrator.ts new file mode 100644 index 0000000..a8acac4 --- /dev/null +++ b/packages/core/src/recovery/orchestrator.ts @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/orchestrator.ts +import type {Request} from '../http/request.js'; +import type {RequestOptions} from '../http/request-options.js'; +import type {Response} from '../http/response.js'; +import type {Transport} from '../seams/transport.js'; +import {wrapCancellation} from './cancellation.js'; +import {fold, success, type Outcome} from './outcome.js'; +import type {RequestRecoveryChain} from './request-chain.js'; +import type {ResponseRecoveryChain} from './response-chain.js'; + +/** + * Everything {@link dispatchWithRecovery} needs beyond the request itself, bundled into one + * trailing object. Five positional parameters would fail ESLint's `max-params: 3`. + * + * @internal + */ +export interface DispatchConfig { + /** The terminal transport hop. */ + readonly transport: Transport; + /** Run before the transport hop; its throwables become a Failure (RECOV-2). */ + readonly requestChain: RequestRecoveryChain; + /** Run on the outcome, whatever it is (RECOV-4 … RECOV-8). */ + readonly responseChain: ResponseRecoveryChain; + /** Per-call operational overrides, threaded to the transport unchanged. */ + readonly options?: RequestOptions | undefined; + /** The caller's abort signal, threaded to the transport unchanged. */ + readonly signal?: AbortSignal | undefined; +} + +/** + * The unified recovery-chain orchestrator (RECOV-2, RECOV-10, RECOV-11). + * + * One `try`/`catch` wraps both the request chain's `apply()` and the transport invocation, so every + * throwable from either is caught and converted into a Failure before the response chain runs — a + * before-request throw cannot skip after-error handling. That conversion goes through + * {@link wrapCancellation} (RECOV-11), this orchestrator's catch being its only call site; RECOV-2's + * guarantee rests on that helper never throwing, since this catch clause is the last place a + * throwable could escape without meeting the recovery hooks. + * + * The final unwrap returns the response on a Success, or rethrows the Failure's throwable + * **unchanged** — no wrapping, no substitution (RECOV-10). Surfacing a typed exception is a + * recovery step's own responsibility, never this function's. + * + * @param request - the request to prepare and send. + * @param config - transport, chains, and the per-call options and signal. + * @returns the response the terminal outcome carries. + * @throws Whatever the terminal Failure carries, by identity — any value, not necessarily an + * `Error`. + * + * @internal + */ +export async function dispatchWithRecovery( + request: Request, + config: DispatchConfig, +): Promise { + let outcome: Outcome; + try { + const preparedRequest = await config.requestChain.apply(request); + outcome = success( + await config.transport.send( + preparedRequest, + config.options, + config.signal, + ), + ); + } catch (error) { + // RECOV-11: `Outcome` widens to `Outcome` without a cast, and never throws, + // which is what keeps RECOV-2 absolute. + outcome = wrapCancellation(error); + } + const finalOutcome = await config.responseChain.apply(outcome); + return fold( + finalOutcome, + response => response, + error => { + throw error; + }, + ); +} diff --git a/packages/core/src/recovery/outcome.test.ts b/packages/core/src/recovery/outcome.test.ts new file mode 100644 index 0000000..9cc38d4 --- /dev/null +++ b/packages/core/src/recovery/outcome.test.ts @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/outcome.test.ts +// Exercises: RECOV-1 (closed two-variant sum type, mutually exclusive and jointly exhaustive, with a +// fold that applies exactly one of two branches at most once per call) +import {describe, expect, test} from 'bun:test'; +import {expectTypeOf} from 'expect-type'; +import fc from 'fast-check'; +import {failure, fold, success, type Outcome} from './outcome.js'; + +describe('success / failure (RECOV-1)', () => { + test('success carries its value under kind "success"', () => { + const outcome = success(42); + + expect(outcome.kind).toBe('success'); + expect(outcome.kind === 'success' && outcome.value).toBe(42); + }); + + test('failure carries its error under kind "failure", typed unknown', () => { + // A JS throw can legally raise any value, not only an Error (sdk-design-nodejs/05). + const outcome = failure('a string throw'); + + expect(outcome.kind).toBe('failure'); + expect(outcome.kind === 'failure' && outcome.error).toBe('a string throw'); + }); +}); + +describe('fold (RECOV-1)', () => { + test('applies onSuccess for a success outcome', () => { + const result = fold( + success(10), + v => v * 2, + () => -1, + ); + + expect(result).toBe(20); + }); + + test('applies onFailure for a failure outcome', () => { + const error = new Error('boom'); + + const result = fold( + failure(error), + () => 'unreachable', + e => e, + ); + + expect(result).toBe(error); + }); + + test('invokes exactly one branch, never both, for either variant', () => { + let successCalls = 0; + let failureCalls = 0; + const onSuccess = (): string => { + successCalls += 1; + return 'ok'; + }; + const onFailure = (): string => { + failureCalls += 1; + return 'err'; + }; + + fold(success(1), onSuccess, onFailure); + fold(failure(new Error('x')), onSuccess, onFailure); + + expect(successCalls).toBe(1); + expect(failureCalls).toBe(1); + }); +}); + +describe('fold identity law (RECOV-1)', () => { + // Canonical law for an invariant-bearing function (docs/knowledge/testing.md): folding a success + // through the identity success-handler, and a failure through the identity failure-handler, must + // each recover the original payload, for arbitrary values. + test('fold(success(x), id, _) === x for arbitrary x', () => { + fc.assert( + fc.property(fc.anything(), value => { + expect( + fold( + success(value), + v => v, + () => 'unreachable', + ), + ).toBe(value); + }), + ); + }); + + test('fold(failure(e), _, id) === e for arbitrary e', () => { + fc.assert( + fc.property(fc.anything(), error => { + expect( + fold( + failure(error), + () => 'unreachable', + e => e, + ), + ).toBe(error); + }), + ); + }); +}); + +describe('Outcome as a type (RECOV-1)', () => { + // An exported generic type ships with a type-level test (styleguide 11.6). These only fire under + // `bun run typecheck` — `bun test` executes this file but strips its types without checking them. + test('the two variants are closed and jointly exhaustive', () => { + expectTypeOf['kind']>().toEqualTypeOf< + 'success' | 'failure' + >(); + }); + + test('narrowing on kind reaches the variant payload, and only that payload', () => { + expectTypeOf< + Extract, {kind: 'success'}>['value'] + >().toEqualTypeOf(); + expectTypeOf< + Extract, {kind: 'failure'}>['error'] + >().toEqualTypeOf(); + }); + + test('a narrowed success has no error field (negative case)', () => { + const outcome: Outcome = success(1); + if (outcome.kind !== 'success') throw new Error('unreachable seed'); + + // @ts-expect-error -- RECOV-1: the variants are mutually exclusive, so a narrowed success has + // no `error` to read. If this line ever compiles, the union has stopped being closed. + const read: unknown = outcome.error; + + expect(read).toBeUndefined(); + }); + + test('a narrowed failure has no value field (negative case)', () => { + const outcome: Outcome = failure(new Error('x')); + if (outcome.kind !== 'failure') throw new Error('unreachable seed'); + + // @ts-expect-error -- the mirror of the case above. + const read: unknown = outcome.value; + + expect(read).toBeUndefined(); + }); + + test('fold collapses both branches to one result type', () => { + expectTypeOf( + fold( + success(1), + v => v, + () => 0, + ), + ).toEqualTypeOf(); + }); + + test('failure() infers the caller-declared payload type, not the error type', () => { + expectTypeOf(failure(new Error('x'))).toEqualTypeOf< + Outcome + >(); + }); +}); diff --git a/packages/core/src/recovery/outcome.ts b/packages/core/src/recovery/outcome.ts new file mode 100644 index 0000000..671bf4a --- /dev/null +++ b/packages/core/src/recovery/outcome.ts @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/outcome.ts +import {assertNever} from '../invariant.js'; + +/** + * The recovery chain's closed two-variant outcome (RECOV-1): a success carrying a value, or a + * failure carrying whatever was thrown. + * + * `error` is `unknown`, not `Error` — a JavaScript `throw` can legally raise any value, and this + * type sits directly under a `catch`. The discriminated union is what RECOV-1's "derivable + * accessors" buys in TypeScript: narrowing on `kind` is compiler-checked, so no `isSuccess()` / + * `getOrThrow()` pair is shipped. + * + * @internal + */ +export type Outcome = + | {readonly kind: 'success'; readonly value: T} + | {readonly kind: 'failure'; readonly error: unknown}; + +/** + * Builds the success variant. + * + * @param value - the value carried by the outcome. + * @returns a success outcome holding `value`. + * + * @internal + */ +export function success(value: T): Outcome { + return {kind: 'success', value}; +} + +/** + * Builds the failure variant. + * + * @param error - whatever was thrown; any value, not necessarily an `Error`. + * @returns a failure outcome holding `error`. + * + * @internal + */ +export function failure(error: unknown): Outcome { + return {kind: 'failure', error}; +} + +/** + * Applies exactly one of `onSuccess` / `onFailure`, never both, satisfying RECOV-1's "a fold that + * applies exactly one of two branches at most once per call." + * + * Three positional parameters rather than an options object: `max-params` errors at four, and this + * matches Phase 2's already-shipped `Transport.send(request, options?, signal?)`. Recorded as a + * corpus deviation in the phase design's ledger. + * + * @param outcome - the outcome to fold. + * @param onSuccess - applied to the value of a success outcome. + * @param onFailure - applied to the error of a failure outcome. + * @returns whichever branch ran. + * + * @internal + */ +export function fold( + outcome: Outcome, + onSuccess: (value: T) => R, + onFailure: (error: unknown) => R, +): R { + switch (outcome.kind) { + case 'success': + return onSuccess(outcome.value); + case 'failure': + return onFailure(outcome.error); + default: + return assertNever(outcome); + } +} diff --git a/packages/core/src/recovery/request-chain.test.ts b/packages/core/src/recovery/request-chain.test.ts new file mode 100644 index 0000000..a63a69a --- /dev/null +++ b/packages/core/src/recovery/request-chain.test.ts @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/request-chain.test.ts +// Exercises: RECOV-3 (sequential left-to-right fold, empty chain is the identity, a throwing step +// aborts the remainder and propagates), RECOV-14 (defensive copy of the step list at construction) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {Request} from '../http/request.js'; +import {RequestRecoveryChain, type RequestStep} from './request-chain.js'; + +function aRequest(): Request { + return Request.newBuilder().url('https://example.com').build(); +} + +function tagAppendStep(char: string): RequestStep { + return request => { + const current = request.headers.get('X-Trace') ?? ''; + return Promise.resolve( + request + .newBuilder() + .headers( + request.headers + .newBuilder() + .set('X-Trace', current + char) + .build(), + ) + .build(), + ); + }; +} + +/** Awaits `promise`, returning whatever it rejected with — `undefined` when it resolved. */ +async function rejection(promise: Promise): Promise { + try { + await promise; + return undefined; + } catch (error) { + return error; + } +} + +describe('RequestRecoveryChain.apply (RECOV-3)', () => { + test('an empty chain returns the input unchanged', async () => { + const chain = new RequestRecoveryChain([]); + const request = aRequest(); + + const result = await chain.apply(request); + + expect(result).toBe(request); + }); + + test('applies steps as a sequential left-to-right fold', async () => { + const chain = new RequestRecoveryChain([ + tagAppendStep('a'), + tagAppendStep('b'), + tagAppendStep('c'), + ]); + + const result = await chain.apply(aRequest()); + + expect(result.headers.get('X-Trace')).toBe('abc'); + }); + + test('a throwing step aborts the remainder and propagates', async () => { + const reached: string[] = []; + const thrownError = new Error('step failed'); + const failingStep: RequestStep = () => { + throw thrownError; + }; + const laterStep: RequestStep = request => { + reached.push('later'); + return Promise.resolve(request); + }; + const chain = new RequestRecoveryChain([ + tagAppendStep('a'), + failingStep, + laterStep, + ]); + + expect(await rejection(chain.apply(aRequest()))).toBe(thrownError); + expect(reached).toEqual([]); + }); +}); + +describe('RequestRecoveryChain construction (RECOV-14)', () => { + test('defensively copies its step list — mutating the source after construction has no effect', async () => { + const steps: RequestStep[] = [tagAppendStep('a')]; + const chain = new RequestRecoveryChain(steps); + steps.push(tagAppendStep('b')); + + const result = await chain.apply(aRequest()); + + expect(result.headers.get('X-Trace')).toBe('a'); + }); +}); + +describe('RequestRecoveryChain.apply fold law', () => { + // Canonical law for an invariant-bearing function: applying the chain equals manually reducing + // the same steps in order, for an arbitrary sequence of single-character append steps. + test('apply() equals a manual left-to-right reduce, for arbitrary step sequences', async () => { + await fc.assert( + // `fc.string({minLength: 1, maxLength: 1})` rather than `fc.char()`: the latter is deprecated + // in fast-check 3.22+ and would print a deprecation warning on every run. + fc.asyncProperty( + fc.array(fc.string({minLength: 1, maxLength: 1}), {maxLength: 10}), + async chars => { + const steps = chars.map(tagAppendStep); + const chain = new RequestRecoveryChain(steps); + + const chained = await chain.apply(aRequest()); + let manual = aRequest(); + for (const step of steps) manual = await step(manual); + + expect(chained.headers.get('X-Trace')).toBe( + manual.headers.get('X-Trace'), + ); + }, + ), + ); + }); +}); + +describe('RECOV-14: steps are safe for concurrent invocation', () => { + // RECOV-14's second normative clause binds both chains, not only the response one: per-request + // state lives in the value being transformed, never on the step or the chain instance. Guards + // the structural property that `apply()`'s only per-call state is its `current` local. + test('two interleaved apply() calls on ONE chain instance do not observe each other', async () => { + const gate: (() => void)[] = []; + const slowStep: RequestStep = async request => { + await new Promise(resolve => gate.push(resolve)); + return request; + }; + const chain = new RequestRecoveryChain([slowStep, tagAppendStep('x')]); + const first = Request.newBuilder().url('https://example.com/first').build(); + const second = Request.newBuilder() + .url('https://example.com/second') + .build(); + + const firstCall = chain.apply(first); + const secondCall = chain.apply(second); + await Promise.resolve(); + for (const release of gate) release(); + const [firstResult, secondResult] = await Promise.all([ + firstCall, + secondCall, + ]); + + expect(firstResult.url.pathname).toBe('/first'); + expect(secondResult.url.pathname).toBe('/second'); + expect(firstResult.headers.get('X-Trace')).toBe('x'); + expect(secondResult.headers.get('X-Trace')).toBe('x'); + }); +}); diff --git a/packages/core/src/recovery/request-chain.ts b/packages/core/src/recovery/request-chain.ts new file mode 100644 index 0000000..ca40301 --- /dev/null +++ b/packages/core/src/recovery/request-chain.ts @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/request-chain.ts +import type {Request} from '../http/request.js'; + +/** + * One link of the request-preparation chain. Async like every other step type in this layer — Node + * has a single execution model, so the phase does not mix sync and async step shapes. + * + * @internal + */ +export type RequestStep = (request: Request) => Promise; + +/** + * A sequential left-to-right fold over request steps (RECOV-3): the output of step N is the input + * of step N+1, an empty chain returns its input unchanged, and a throwing step aborts the remainder + * and propagates — `dispatchWithRecovery` (`orchestrator.ts`) converts that propagation into a + * `Failure` per RECOV-2, which is the only reason propagating here is safe. + * + * Safe under concurrent `apply()` calls (RECOV-14): after construction the instance holds nothing + * but its step array, and every piece of per-call state lives in `apply()`'s locals. A later phase + * must not move per-call bookkeeping onto a field here. + * + * @internal + */ +export class RequestRecoveryChain { + readonly #steps: readonly RequestStep[]; + + /** + * Defensively copies `steps` (RECOV-14). The reference implementation retains the caller's array + * by reference on this chain only — an asymmetry the requirement's own text recommends a port not + * reproduce. + * + * @param steps - the ordered request steps. + */ + constructor(steps: readonly RequestStep[]) { + this.#steps = [...steps]; + } + + /** + * Folds the request through every step in order. + * + * @param request - the request to prepare. + * @returns the request produced by the last step, or the input when the chain is empty. + * @throws Whatever a step throws, aborting the remaining steps (RECOV-3). + */ + async apply(request: Request): Promise { + let current = request; + for (const step of this.#steps) { + current = await step(current); + } + return current; + } +} diff --git a/packages/core/src/recovery/response-chain.test.ts b/packages/core/src/recovery/response-chain.test.ts new file mode 100644 index 0000000..a9e14ae --- /dev/null +++ b/packages/core/src/recovery/response-chain.test.ts @@ -0,0 +1,426 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/response-chain.test.ts +// Exercises: RECOV-4 (response steps run only on a Success), RECOV-5/RECOV-6 (recovery steps run on +// every outcome; fold order is all response steps then all recovery steps), RECOV-7 (a throwing +// response step becomes a Failure fed to recovery, never propagated), RECOV-8 (a throwing recovery +// step becomes a Failure fed to the NEXT recovery step; apply() never throws), RECOV-12 +// (close-on-throw while holding a Success, close failure attached as `suppressed` with the original +// throwable staying primary), RECOV-13 (a deliberately returned substitute outcome is never +// auto-closed), RECOV-14 (both step lists defensively copied, and steps safe for concurrent +// invocation — no per-call state on the chain instance) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {failure, success, type Outcome} from './outcome.js'; +import { + ResponseRecoveryChain, + type RecoveryStep, + type ResponseStep, +} from './response-chain.js'; + +function aResponse(body: ReadableStream | null = null): Response { + return Response.newBuilder() + .request(Request.newBuilder().url('https://example.com').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .body(body) + .build(); +} + +/** + * Close is observed through the body stream's `cancel()`, exactly the way Phase 3b's own + * `response.test.ts` observes it — NOT by patching `response.close`. `Response` calls + * `Object.freeze(this)` at the end of its constructor, so `response.close = ...` throws + * `TypeError: Cannot add property close, object is not extensible` in an ES module's strict mode. + * `Response.close()` is memoized and cancels the body at most once, so the cancel count IS the + * effective-close count RECOV-12's "released exactly once" asks about. + */ +function countingCloseResponse(): { + response: Response; + closeCount: () => number; +} { + let cancels = 0; + const body = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + cancels += 1; + }, + }); + return {response: aResponse(body), closeCount: () => cancels}; +} + +/** + * A response whose `close()` rejects. `Response.close()` awaits `body.cancel()` and swallows only + * `TypeError` (the locked-stream case), so a plain `Error` propagates out of `close()`. + */ +function failingCloseResponse(closeError: Error): Response { + return aResponse( + new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + throw closeError; + }, + }), + ); +} + +describe('response-step phase (RECOV-4, RECOV-6)', () => { + test('response steps run in order on a Success outcome', async () => { + const seen: string[] = []; + const stepA: ResponseStep = r => { + seen.push('a'); + return Promise.resolve(r); + }; + const stepB: ResponseStep = r => { + seen.push('b'); + return Promise.resolve(r); + }; + const chain = new ResponseRecoveryChain([stepA, stepB], []); + + await chain.apply(success(aResponse())); + + expect(seen).toEqual(['a', 'b']); + }); + + test('response steps do not run when the input outcome is already a Failure', async () => { + const original = new Error('original'); + const stepShouldNotRun: ResponseStep = () => { + throw new Error('must not run'); + }; + const chain = new ResponseRecoveryChain([stepShouldNotRun], []); + + const result = await chain.apply(failure(original)); + + expect(result.kind).toBe('failure'); + expect(result.kind === 'failure' && result.error).toBe(original); + }); +}); + +describe('recovery-step phase (RECOV-5, RECOV-6)', () => { + test('recovery steps run on every outcome, successes and failures, in order', async () => { + const seenKinds: string[] = []; + const record: RecoveryStep = outcome => { + seenKinds.push(outcome.kind); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain([], [record, record]); + + await chain.apply(success(aResponse())); + await chain.apply(failure(new Error('x'))); + + expect(seenKinds).toEqual(['success', 'success', 'failure', 'failure']); + }); + + test('fold order is all response steps first, then all recovery steps', async () => { + const order: string[] = []; + const responseStep: ResponseStep = r => { + order.push('response'); + return Promise.resolve(r); + }; + const recoveryStep: RecoveryStep = outcome => { + order.push('recovery'); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain([responseStep], [recoveryStep]); + + await chain.apply(success(aResponse())); + + expect(order).toEqual(['response', 'recovery']); + }); +}); + +describe('RECOV-7: a throwing response step converts to a Failure fed to recovery', () => { + test('the throwable never propagates out of apply(), and recovery observes the Failure', async () => { + const stepAfterThatMustNotRun: ResponseStep = () => { + throw new Error( + 'must not run — the response phase stops after the throw', + ); + }; + const thrownError = new Error('response step failed'); + const throwingStep: ResponseStep = () => { + throw thrownError; + }; + const seenByRecovery: Outcome[] = []; + const recoveryStep: RecoveryStep = outcome => { + seenByRecovery.push(outcome); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain( + [throwingStep, stepAfterThatMustNotRun], + [recoveryStep], + ); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + expect(result.kind === 'failure' && result.error).toBe(thrownError); + expect(seenByRecovery).toHaveLength(1); + expect(seenByRecovery[0]?.kind).toBe('failure'); + }); +}); + +describe('RECOV-8: a throwing recovery step wraps into a Failure fed to the next step', () => { + test('apply() never throws, and the remaining recovery steps still run', async () => { + const secondStepSeen: Outcome[] = []; + const throwingRecoveryStep: RecoveryStep = () => { + throw new Error('recovery step failed'); + }; + const secondRecoveryStep: RecoveryStep = outcome => { + secondStepSeen.push(outcome); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain( + [], + [throwingRecoveryStep, secondRecoveryStep], + ); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + expect(secondStepSeen).toHaveLength(1); + expect(secondStepSeen[0]?.kind).toBe('failure'); + }); +}); + +describe('RECOV-12: close-on-throw while holding a Success', () => { + test('closes the in-hand response exactly once before wrapping the throwable', async () => { + const {response, closeCount} = countingCloseResponse(); + const thrownError = new Error('step failed'); + const throwingStep: ResponseStep = () => { + throw thrownError; + }; + const chain = new ResponseRecoveryChain([throwingStep], []); + + const result = await chain.apply(success(response)); + + expect(closeCount()).toBe(1); + expect(result.kind === 'failure' && result.error).toBe(thrownError); + }); + + test('a close failure is attached as suppressed, with the original throwable staying primary', async () => { + const closeError = new Error('close failed'); + const response = failingCloseResponse(closeError); + const originalError = new Error('step failed'); + const throwingStep: ResponseStep = () => { + throw originalError; + }; + const chain = new ResponseRecoveryChain([throwingStep], []); + + const result = await chain.apply(success(response)); + + expect(result.kind).toBe('failure'); + const wrapped = result.kind === 'failure' ? result.error : undefined; + expect(wrapped).toBeInstanceOf(Error); + expect((wrapped as SuppressedErrorShape).name).toBe('SuppressedError'); + expect((wrapped as SuppressedErrorShape).error).toBe(originalError); + expect((wrapped as SuppressedErrorShape).suppressed).toBe(closeError); + }); + + test('a throwing recovery step holding a Success also closes it exactly once', async () => { + const {response, closeCount} = countingCloseResponse(); + const thrownError = new Error('recovery step failed'); + const throwingRecoveryStep: RecoveryStep = () => { + throw thrownError; + }; + const chain = new ResponseRecoveryChain([], [throwingRecoveryStep]); + + const result = await chain.apply(success(response)); + + expect(closeCount()).toBe(1); + expect(result.kind === 'failure' && result.error).toBe(thrownError); + }); + + test('a throwing step holding a Failure closes nothing — there is no response in hand', async () => { + const thrownError = new Error('recovery step failed'); + const throwingRecoveryStep: RecoveryStep = () => { + throw thrownError; + }; + const chain = new ResponseRecoveryChain([], [throwingRecoveryStep]); + + const result = await chain.apply(failure(new Error('seed'))); + + expect(result.kind === 'failure' && result.error).toBe(thrownError); + }); +}); + +interface SuppressedErrorShape extends Error { + readonly error: unknown; + readonly suppressed: unknown; +} + +describe('RECOV-13: a deliberate outcome substitution is never auto-closed', () => { + test('a recovery step returning a different Failure does not trigger a close', async () => { + const {response, closeCount} = countingCloseResponse(); + const substituteStep: RecoveryStep = () => + Promise.resolve(failure(new Error('substituted, not thrown'))); + const chain = new ResponseRecoveryChain([], [substituteStep]); + + await chain.apply(success(response)); + + expect(closeCount()).toBe(0); + }); + + test('a recovery step substituting a different Success does not trigger a close', async () => { + const {response: original, closeCount} = countingCloseResponse(); + const substitute = aResponse(); + const substituteStep: RecoveryStep = () => + Promise.resolve(success(substitute)); + const chain = new ResponseRecoveryChain([], [substituteStep]); + + const result = await chain.apply(success(original)); + + expect(closeCount()).toBe(0); + expect(result.kind === 'success' && result.value).toBe(substitute); + }); +}); + +describe('RECOV-14: both step lists are defensively copied', () => { + test('mutating the source arrays after construction has no effect on apply()', async () => { + const responseSteps: ResponseStep[] = []; + const recoverySteps: RecoveryStep[] = []; + const chain = new ResponseRecoveryChain(responseSteps, recoverySteps); + responseSteps.push(() => { + throw new Error('must not run — pushed after construction'); + }); + recoverySteps.push(outcome => Promise.resolve(outcome)); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('success'); + }); +}); + +describe('RECOV-8: apply() never throws, including on a step that lies about its type', () => { + // RECOV-8 is absolute — "MUST NOT throw under any input" — and `recovery/` is plumbing a later + // phase (and eventually a caller) installs steps into. TypeScript cannot enforce the return type + // across that seam, so the two shapes a mistyped step produces are pinned here: a step whose + // return value is not an outcome at all, and a step that then trips over it. Before this was + // guarded, the second case raised `TypeError: undefined is not an object` out of `apply()`. + test('a recovery step returning a non-outcome does not make apply() throw', async () => { + const returnsNothing = (() => + Promise.resolve(undefined)) as unknown as RecoveryStep; + const readsTheOutcome: RecoveryStep = outcome => + Promise.resolve(outcome.kind === 'failure' ? outcome : outcome); + const chain = new ResponseRecoveryChain( + [], + [returnsNothing, readsTheOutcome], + ); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + }); + + test('a response step returning a non-response does not make apply() throw', async () => { + const returnsNothing = (() => + Promise.resolve(undefined)) as unknown as ResponseStep; + const readsTheResponse: ResponseStep = response => + Promise.resolve(response.status.isError ? response : response); + const chain = new ResponseRecoveryChain( + [returnsNothing, readsTheResponse], + [], + ); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + }); + + test("the step's own throwable stays primary when the plumbing also fails", async () => { + const returnsNothing = (() => + Promise.resolve(undefined)) as unknown as ResponseStep; + const thrownError = new Error('step failed on a poisoned outcome'); + const throwingStep: ResponseStep = () => { + throw thrownError; + }; + const chain = new ResponseRecoveryChain([returnsNothing, throwingStep], []); + + const result = await chain.apply(success(aResponse())); + + expect(result.kind).toBe('failure'); + const error = result.kind === 'failure' ? result.error : undefined; + expect((error as SuppressedErrorShape).name).toBe('SuppressedError'); + expect((error as SuppressedErrorShape).error).toBe(thrownError); + }); +}); + +describe('apply() never throws (RECOV-8 property)', () => { + // Canonical law for an invariant-bearing function: for an arbitrary mix of throwing and + // non-throwing RESPONSE AND RECOVERY steps, over a seed outcome that is arbitrarily a Success or + // a Failure, apply() always settles and never re-raises a step's throw (RECOV-8) — and no + // response step runs on any generated case whose seed was already a Failure (RECOV-4). + // + // BOTH phases and BOTH seed variants must be generated: a generator emitting recovery steps only, + // or seeding Success only, proves the RECOV-8 law and silently leaves RECOV-4 to the example + // tests above. + test('apply() settles and skips the response phase on a Failure seed, for arbitrary step sequences', async () => { + await fc.assert( + fc.asyncProperty( + fc.array(fc.boolean(), {maxLength: 4}), + fc.array(fc.boolean(), {maxLength: 4}), + fc.boolean(), + async (responseFlags, recoveryFlags, seedIsSuccess) => { + let responseStepRuns = 0; + const responseSteps: ResponseStep[] = responseFlags.map( + (shouldThrow, index) => response => { + responseStepRuns += 1; + if (shouldThrow) + throw new Error(`response step ${String(index)} failed`); + return Promise.resolve(response); + }, + ); + const recoverySteps: RecoveryStep[] = recoveryFlags.map( + (shouldThrow, index) => outcome => { + if (shouldThrow) + throw new Error(`recovery step ${String(index)} failed`); + return Promise.resolve(outcome); + }, + ); + const chain = new ResponseRecoveryChain(responseSteps, recoverySteps); + const seed = seedIsSuccess + ? success(aResponse()) + : failure(new Error('seed failure')); + + const result = await chain.apply(seed); + + expect(['success', 'failure']).toContain(result.kind); + if (!seedIsSuccess) expect(responseStepRuns).toBe(0); // RECOV-4 + }, + ), + ); + }); +}); + +describe('RECOV-14: steps are safe for concurrent invocation', () => { + // RECOV-14's SECOND normative clause: per-request state lives in the passed value, never on the + // step or the chain. Guards the structural property that apply()'s only per-call state is its + // `current` local — a later phase adding per-call bookkeeping to a chain field would fail here. + test('two interleaved apply() calls on ONE chain instance do not observe each other', async () => { + const gate: (() => void)[] = []; + const slowStep: RecoveryStep = async outcome => { + await new Promise(resolve => gate.push(resolve)); + return outcome; + }; + const chain = new ResponseRecoveryChain([], [slowStep]); + const successSeed = success(aResponse()); + const failureSeed = failure(new Error('second call')); + + const first = chain.apply(successSeed); + const second = chain.apply(failureSeed); + await Promise.resolve(); + for (const release of gate) release(); + const [firstResult, secondResult] = await Promise.all([first, second]); + + expect(firstResult.kind).toBe('success'); + expect( + secondResult.kind === 'failure' && (secondResult.error as Error).message, + ).toBe('second call'); + }); +}); diff --git a/packages/core/src/recovery/response-chain.ts b/packages/core/src/recovery/response-chain.ts new file mode 100644 index 0000000..f62349f --- /dev/null +++ b/packages/core/src/recovery/response-chain.ts @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/response-chain.ts +import type {Response} from '../http/response.js'; +import {suppress} from '../suppress.js'; +import {failure, success, type Outcome} from './outcome.js'; + +/** + * One link of the response phase, run only while the outcome is a Success (RECOV-4). + * + * @internal + */ +export type ResponseStep = (response: Response) => Promise; + +/** + * One link of the recovery phase, run on every outcome, success or failure (RECOV-5). + * + * A recovery step SHOULD return a `Failure` rather than throw (RECOV-9); both are handled + * identically, so this is a convention rather than something the chain enforces. + * + * @internal + */ +export type RecoveryStep = ( + outcome: Outcome, +) => Promise>; + +/** + * The response and recovery step folds (RECOV-4 … RECOV-9, RECOV-12, RECOV-13). + * + * Response steps run first and only while the outcome is a Success, in declared order; a throwing + * response step becomes a Failure fed to the recovery phase (RECOV-7) rather than propagating. + * Recovery steps then run on whatever the outcome is by then, always, in declared order; a throwing + * recovery step becomes a Failure fed to the NEXT recovery step (RECOV-8). `apply()` itself never + * throws, for any input. + * + * A step that *returns* a substitute outcome is never auto-closed (RECOV-13) — only a caught throw + * reaches {@link toFailureClosingSuccess}. A transforming step owns releasing whatever it drops. + * + * Safe under concurrent `apply()` calls (RECOV-14): after construction the instance holds nothing + * but its two step arrays, and all per-call state lives in the phase methods' locals. + * + * @internal + */ +export class ResponseRecoveryChain { + readonly #responseSteps: readonly ResponseStep[]; + readonly #recoverySteps: readonly RecoveryStep[]; + + /** + * Defensively copies both lists (RECOV-14). + * + * @param responseSteps - steps run on a Success, in order. + * @param recoverySteps - steps run on every outcome, in order. + */ + constructor( + responseSteps: readonly ResponseStep[], + recoverySteps: readonly RecoveryStep[], + ) { + this.#responseSteps = [...responseSteps]; + this.#recoverySteps = [...recoverySteps]; + } + + /** + * Folds `outcome` through the response phase and then the recovery phase (RECOV-6). + * + * @param outcome - the outcome produced by the transport, or by an earlier failure. + * @returns the terminal outcome. Never throws (RECOV-8). + */ + async apply(outcome: Outcome): Promise> { + const afterResponsePhase = await this.#runResponsePhase(outcome); + return this.#runRecoveryPhase(afterResponsePhase); + } + + async #runResponsePhase( + outcome: Outcome, + ): Promise> { + let current = outcome; + for (const step of this.#responseSteps) { + // RECOV-4: the whole response phase is skipped once the outcome is not a Success. + if (current.kind !== 'success') break; + try { + current = success(await step(current.value)); + } catch (thrownError) { + current = await toFailureClosingSuccess(thrownError, current); // RECOV-7, RECOV-12 + break; // the remaining response steps do not run once converted to a Failure + } + } + return current; + } + + async #runRecoveryPhase( + outcome: Outcome, + ): Promise> { + let current = outcome; + for (const step of this.#recoverySteps) { + try { + // RECOV-13: a normal return substituting the outcome is never auto-closed. + current = await step(current); + } catch (thrownError) { + current = await toFailureClosingSuccess(thrownError, current); // RECOV-8, RECOV-12 + // RECOV-8: the remaining recovery steps still run — deliberately no `break` here. + } + } + return current; + } +} + +/** + * Shared close-on-throw handling for both phases (RECOV-12): when the outcome held at the moment of + * the throw was a Success, its response is released before the throwable is wrapped into a Failure, + * exactly once. + * + * A close failure rides along as `suppressed` on the ORIGINAL throwable — built by hand through + * {@link suppress}, original first — never via `using` / `await using`, whose auto-generated + * `SuppressedError` puts the *teardown* failure first and would silently invert which error the + * caller ends up seeing (`docs/knowledge/resource-management.md:72`). + * + * **This function is total: it never throws, for any argument.** RECOV-8 makes "`apply()` MUST NOT + * throw under any input" absolute, and this runs inside both phases' `catch` blocks — the last place + * a throwable could escape the chain. The discriminant read and the `close()` call are therefore + * inside the same `try`, not just the `close()`: a step that lies about its return type (a JS caller, + * or one returning `undefined`) can leave `current` holding something with no `kind` and no + * `close()`, and reading through it would otherwise raise a `TypeError` out of `apply()`. Handling it + * here rather than crashing is the same call `wrapCancellation` makes — a step is a pluggable seam, + * so a step that misbehaves is an operational failure, not a violated precondition of this codebase. + * The step's own throwable stays primary either way; the plumbing failure rides along as + * `suppressed`. + */ +async function toFailureClosingSuccess( + thrownError: unknown, + current: Outcome, +): Promise> { + try { + if (current.kind === 'success') { + // Awaited, not fire-and-forget: `Response.close()` returns a promise, so an un-awaited call + // would settle outside this try with nothing to catch its rejection. + await current.value.close(); + } + } catch (closeError) { + return failure( + suppress( + thrownError, + closeError, + 'response close failed while handling a step error', + ), + ); + } + return failure(thrownError); +} diff --git a/packages/core/src/recovery/status-mapping.test.ts b/packages/core/src/recovery/status-mapping.test.ts new file mode 100644 index 0000000..0963257 --- /dev/null +++ b/packages/core/src/recovery/status-mapping.test.ts @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/status-mapping.test.ts +// Exercises: RECOV-15 (only 400..599 map to the matching typed exception; every other status passes +// through unchanged, and §8.2's conformance clause that an error status reaches a recovery hook as +// a Failure), RECOV-16 (the mapping reuses Phase 3b's already-bounded, replayable buffering — this +// file proves the wiring only; the 1 MiB cap and its truncation are 3b's own suite's job, at +// `body/http-status-error.test.ts`), RECOV-7 and RECOV-4 where the step meets the chain +import {describe, expect, test} from 'bun:test'; +import {expectTypeOf} from 'expect-type'; +import {HttpStatusError} from '../body/http-status-error.js'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {failure, success, type Outcome} from './outcome.js'; +import { + ResponseRecoveryChain, + type RecoveryStep, + type ResponseStep, +} from './response-chain.js'; +import {statusMappingStep} from './status-mapping.js'; + +function aResponse( + status: number, + body: ReadableStream | null = null, +): Response { + return Response.newBuilder() + .request(Request.newBuilder().url('https://example.com').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(status)) + .body(body) + .build(); +} + +function bodyOf(text: string): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(text)); + controller.close(); + }, + }); +} + +/** Awaits `promise`, returning whatever it rejected with — `undefined` when it resolved. */ +async function rejection(promise: Promise): Promise { + try { + await promise; + return undefined; + } catch (error) { + return error; + } +} + +describe('statusMappingStep (RECOV-15)', () => { + test('returns a 2xx response unchanged', async () => { + const response = aResponse(200); + + const result = await statusMappingStep(response); + + expect(result).toBe(response); + }); + + test('returns a 3xx response unchanged', async () => { + const response = aResponse(304); + + const result = await statusMappingStep(response); + + expect(result).toBe(response); + }); + + test('returns a non-standard 6xx response unchanged — only 400..599 map', async () => { + const response = aResponse(600); + + const result = await statusMappingStep(response); + + expect(result).toBe(response); + }); + + test('throws HttpStatusError naming the status for a 404', async () => { + const error = await rejection(statusMappingStep(aResponse(404))); + + expect(error).toBeInstanceOf(HttpStatusError); + expect((error as HttpStatusError).status).toBe(404); + }); + + test('throws HttpStatusError for a 500', async () => { + const error = await rejection(statusMappingStep(aResponse(500))); + + expect(error).toBeInstanceOf(HttpStatusError); + expect((error as HttpStatusError).status).toBe(500); + }); +}); + +describe('statusMappingStep buffering (RECOV-16)', () => { + test('the error body survives on the thrown exception, replayable after the response is closed', async () => { + const error = await rejection( + statusMappingStep(aResponse(422, bodyOf('{"detail":"nope"}'))), + ); + + expect(error).toBeInstanceOf(HttpStatusError); + expect((error as HttpStatusError).preview()).toBe('{"detail":"nope"}'); + }); +}); + +describe('statusMappingStep inside a recovery chain (RECOV-15, RECOV-7)', () => { + // §8.2's own conformance clause for RECOV-15 is about the outcome the chain produces, not about + // the step in isolation: a 400..599 must reach a recovery hook as a Failure carrying the typed + // exception, exactly the way a transport error does. The step throwing is the mechanism; this is + // the requirement. + test('a 404 surfaces to a recovery hook as a Failure carrying the typed exception', async () => { + const seen: Outcome[] = []; + const recorder: RecoveryStep = outcome => { + seen.push(outcome); + return Promise.resolve(outcome); + }; + const chain = new ResponseRecoveryChain([statusMappingStep], [recorder]); + + const result = await chain.apply(success(aResponse(404, bodyOf('nope')))); + + expect(result.kind).toBe('failure'); + const error = result.kind === 'failure' ? result.error : undefined; + expect(error).toBeInstanceOf(HttpStatusError); + expect((error as HttpStatusError).status).toBe(404); + expect((error as HttpStatusError).preview()).toBe('nope'); + expect(seen).toEqual([result]); + }); + + test('a 200 passes through the chain as a Success carrying the same response', async () => { + const response = aResponse(200); + const chain = new ResponseRecoveryChain([statusMappingStep], []); + + const result = await chain.apply(success(response)); + + expect(result.kind === 'success' && result.value).toBe(response); + }); + + test('the step never runs on a Failure input — RECOV-4 governs, not the status', async () => { + const seedError = new Error('transport failed'); + const chain = new ResponseRecoveryChain([statusMappingStep], []); + + const result = await chain.apply(failure(seedError)); + + expect(result.kind === 'failure' && result.error).toBe(seedError); + }); +}); + +describe('statusMappingStep conforms to ResponseStep', () => { + // The compile-time proof the discarded `: ResponseStep` annotation used to provide, kept out of + // the module so nothing dead reaches `dist/`. Only fires under `bun run typecheck` — `bun test` + // executes this file but strips its types without checking them (styleguide 11.6). + test('its signature is exactly the ResponseStep signature', () => { + expectTypeOf().toEqualTypeOf(); + }); +}); diff --git a/packages/core/src/recovery/status-mapping.ts b/packages/core/src/recovery/status-mapping.ts new file mode 100644 index 0000000..40678ff --- /dev/null +++ b/packages/core/src/recovery/status-mapping.ts @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/recovery/status-mapping.ts +import {toHttpError} from '../body/http-status-error.js'; +import type {Response} from '../http/response.js'; + +/** + * The status → typed-exception mapping response step (RECOV-15, RECOV-16). + * + * Phase 3b's `toHttpError()` already satisfies both requirements in full: it treats only 400..599 + * as errors and hands every other status back unchanged (RECOV-15), and it buffers the error body + * into a bounded, replayable in-memory copy inside the response's own close-guaranteeing scope + * before mapping, sharing the same 1 MiB cap 3b's logging tees use (RECOV-16). `HttpStatusError` — + * flat, carrying `status` and the buffered body — IS the "matching typed exception"; no new + * buffering, no per-status class hierarchy. + * + * The `throw` is deliberate: it lets RECOV-7 in `response-chain.ts` convert an error status into a + * Failure exactly the way any other response-step throw is handled, rather than this step + * special-casing its own error path. + * + * @param response - the response to inspect. + * @returns the response unchanged when its status is not an error status. + * @throws HttpStatusError when the status is in 400..599. + * + * @internal + */ +export async function statusMappingStep(response: Response): Promise { + const httpError = await toHttpError(response); + if (httpError === null) return response; + throw httpError; +} + +// A named declaration, not `const statusMappingStep: ResponseStep = async response => ...`: arrows +// are reserved for inline callbacks (docs/knowledge/function-design.md:18-21), and a named +// declaration survives in stack traces — which a function whose whole job is to throw actually +// depends on. `func-style`'s `allowArrowFunctions: true` would not have flagged the arrow form, so +// this is on the author, not the gate. +// +// The proof that the signature still conforms to `ResponseStep` lives in the test file, as an +// `expectTypeOf` assertion. A module-level `statusMappingStep satisfies ResponseStep;` would do the +// same job, but `satisfies` erases to its operand rather than to nothing, leaving a dead +// `statusMappingStep;` expression statement in the published `dist/`. diff --git a/packages/core/src/suppress.test.ts b/packages/core/src/suppress.test.ts new file mode 100644 index 0000000..e675796 --- /dev/null +++ b/packages/core/src/suppress.test.ts @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/suppress.test.ts +// Exercises: the runtime-guarded stand-in for `SuppressedError` that RECOV-12 (and, later, Phases +// 5a/6a/6b/6c) need to attach a teardown failure to a primary throwable without inverting their +// priority. +// +// Neither branch of the guard is forced here by mutating `globalThis` — a test that deletes a +// global does not survive parallel execution, which docs/knowledge/testing.md:50 requires. The +// branch selection is covered where it is real instead: `suppress()` is asserted on its shape, +// which holds on either runtime, `FallbackSuppressedError` is constructed directly, and the +// `test:node` matrix runs both legs — `lts/*` has the native class, the pinned `20.3.0` floor does +// not. +import {describe, expect, test} from 'bun:test'; +import {expectTypeOf} from 'expect-type'; +import { + FallbackSuppressedError, + suppress, + type SuppressedErrorLike, +} from './suppress.js'; + +describe('suppress', () => { + test('keeps the primary error primary and the secondary suppressed', () => { + const primary = new Error('primary'); + const secondary = new Error('secondary'); + + const result = suppress(primary, secondary, 'teardown failed'); + + expect(result.error).toBe(primary); + expect(result.suppressed).toBe(secondary); + expect(result.message).toBe('teardown failed'); + }); + + test('reports the same identity on either branch of the guard', () => { + const result = suppress( + new Error('primary'), + new Error('secondary'), + 'msg', + ); + + expect(result).toBeInstanceOf(Error); + expect(result.name).toBe('SuppressedError'); + }); + + test('carries non-Error throwables unchanged — a JS throw can raise any value', () => { + const result = suppress('a string throw', undefined, 'teardown failed'); + + expect(result.error).toBe('a string throw'); + expect(result.suppressed).toBeUndefined(); + }); + + test('uses the native SuppressedError when the runtime provides one', () => { + const native = (globalThis as {SuppressedError?: unknown}).SuppressedError; + if (typeof native !== 'function') return; // the floor runtime has no native class to use + + const result = suppress(new Error('a'), new Error('b'), 'msg'); + + expect(result).toBeInstanceOf(native); + }); +}); + +describe('FallbackSuppressedError — the branch the declared floor takes', () => { + test('mirrors the native shape rather than reporting its own class name', () => { + const primary = new Error('primary'); + const secondary = new Error('secondary'); + + const result = new FallbackSuppressedError( + primary, + secondary, + 'teardown failed', + ); + + expect(result).toBeInstanceOf(Error); + expect(result.name).toBe('SuppressedError'); + expect(result.error).toBe(primary); + expect(result.suppressed).toBe(secondary); + expect(result.message).toBe('teardown failed'); + }); + + test('satisfies the SuppressedErrorLike shape suppress() promises', () => { + expectTypeOf().toExtend(); + expectTypeOf< + ReturnType + >().toEqualTypeOf(); + }); +}); diff --git a/packages/core/src/suppress.ts b/packages/core/src/suppress.ts new file mode 100644 index 0000000..99c407f --- /dev/null +++ b/packages/core/src/suppress.ts @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/suppress.ts + +/** + * The shape of an error carrying a suppressed secondary throwable — structurally identical to the + * ECMAScript `SuppressedError` this module produces when the runtime has one. + * + * @internal + */ +export interface SuppressedErrorLike extends Error { + /** The primary throwable — the one the caller actually cares about. */ + readonly error: unknown; + /** The secondary throwable raised while unwinding, riding along rather than masking. */ + readonly suppressed: unknown; +} + +type SuppressedErrorConstructor = new ( + error: unknown, + suppressed: unknown, + message: string, +) => SuppressedErrorLike; + +/** + * Pairs a primary throwable with a secondary one raised while unwinding, keeping the **primary** + * primary (RECOV-12). + * + * `SuppressedError` is a V8 global from the full Explicit Resource Management proposal, absent on + * this package's declared floor (`engines.node >=20.3`, set by `AbortSignal.any()`) and absent from + * the `lib` this package compiles against. Raising the floor to reach it would drop every Node 20 + * and 22 consumer for one error class, so the class is used when the runtime happens to provide it + * and {@link FallbackSuppressedError} is built when it does not — the same guarded shape the + * roadmap already sanctioned for `Symbol.asyncDispose`. The global is read per call, not captured at + * module load, so the choice tracks the runtime rather than the import order. + * + * Both branches return the same observable shape, so no caller branches on which one it got, and no + * caller may test `instanceof SuppressedError` — that would silently assert nothing on the floor. + * CI covers both: the `lts/*` leg of the `test:node` matrix takes the native branch, the pinned + * `20.3.0` leg takes the fallback. + * + * Never built via `using`/`await using`: native disposal constructs + * `new SuppressedError(disposalError, originalError)`, making the *teardown* failure primary + * (`docs/knowledge/resource-management.md:72`) — the inverse of what RECOV-12 requires. + * + * @param error - the primary throwable; stays primary. + * @param suppressed - the secondary throwable raised while unwinding. + * @param message - describes the unwinding that produced `suppressed`. + * @returns an error carrying both, with `error` primary. + * + * @internal + */ +export function suppress( + error: unknown, + suppressed: unknown, + message: string, +): SuppressedErrorLike { + const {SuppressedError: native} = globalThis as typeof globalThis & { + SuppressedError?: SuppressedErrorConstructor; + }; + return typeof native === 'function' + ? new native(error, suppressed, message) + : new FallbackSuppressedError(error, suppressed, message); +} + +/** + * The stand-in {@link suppress} builds on runtimes without the native class. Mirrors its observable + * shape — `name`, `error`, `suppressed` — so a caller never has to branch on which one it received. + * + * `name` is pinned to `'SuppressedError'` rather than following `docs/knowledge/error-handling.md`'s + * `this.name = new.target.name`: the point of this class is to be indistinguishable from the native + * one, and reporting `FallbackSuppressedError` in a stack trace would make the runtime the reader is + * on part of the error's identity. + * + * Exported so its shape is unit-testable directly. The alternative — deleting + * `globalThis.SuppressedError` inside a test to force the fallback branch — would not survive + * parallel execution, which `docs/knowledge/testing.md:50` requires of every test. + * + * @internal + */ +export class FallbackSuppressedError + extends Error + implements SuppressedErrorLike +{ + override readonly name = 'SuppressedError'; + readonly error: unknown; + readonly suppressed: unknown; + + constructor(error: unknown, suppressed: unknown, message: string) { + super(message); + this.error = error; + this.suppressed = suppressed; + } +} diff --git a/test/node-conformance/recovery-chain.test.mjs b/test/node-conformance/recovery-chain.test.mjs new file mode 100644 index 0000000..de0a215 --- /dev/null +++ b/test/node-conformance/recovery-chain.test.mjs @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: MIT +// test/node-conformance/recovery-chain.test.mjs +// +// Phase 4b (`RECOV-12`) is a runtime-divergent surface for one specific reason: the `SuppressedError` +// global. Bun ships it and so does current Node, but it is a V8 global from the full Explicit Resource +// Management proposal and is absent on this package's declared floor (`engines.node ">=20.3"`). A +// `new SuppressedError(...)` written straight into `response-chain.ts` would pass `bun test` and then throw +// `ReferenceError: SuppressedError is not defined` at a consumer's call time — exactly the `NFR-10` trap +// `docs/knowledge/tooling-and-quality-gates.md:60-61` describes. `suppress()` guards on the global; this +// file is what proves the guarded path actually works on the runtime the SDK ships to, at both ends of the +// matrix. +// +// The close-on-throw half also exercises `RECOV-12`'s "released exactly once" over Node's own Web Streams +// implementation, whose `cancel()` and reader-lock timing are independent of Bun's. +import assert from 'node:assert/strict'; +import {describe, it} from 'node:test'; +import {Protocol, Request, Response, Status} from '@dexpace/core'; +import { + FallbackSuppressedError, + suppress, +} from '../../packages/core/dist/suppress.js'; +import {ResponseRecoveryChain} from '../../packages/core/dist/recovery/response-chain.js'; +import {success} from '../../packages/core/dist/recovery/outcome.js'; + +function aResponse(body = null) { + return Response.newBuilder() + .request(Request.newBuilder().url('https://example.com').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .body(body) + .build(); +} + +describe('suppress() on the declared Node floor', () => { + it('produces a shape-compatible error whether or not the runtime has SuppressedError', () => { + const primary = new Error('primary'); + const secondary = new Error('secondary'); + + const result = suppress(primary, secondary, 'teardown failed'); + + assert.ok(result instanceof Error, 'suppress() must return an Error'); + assert.equal(result.name, 'SuppressedError'); + assert.equal( + result.error, + primary, + 'the primary throwable must stay primary', + ); + assert.equal(result.suppressed, secondary); + assert.equal(result.message, 'teardown failed'); + }); + + it('takes the branch this runtime actually has, and both legs of the matrix are covered', () => { + // Not forced by deleting the global — that would not survive parallel execution + // (docs/knowledge/testing.md:50). The matrix is the forcing function: the pinned 20.3.0 leg has + // no native class and takes the fallback, `lts/*` has one and takes the native branch. Either + // way the result must be usable without the caller knowing which. + const native = globalThis.SuppressedError; + const result = suppress( + new Error('primary'), + new Error('secondary'), + 'teardown failed', + ); + + if (typeof native === 'function') { + assert.ok( + result instanceof native, + 'a runtime with SuppressedError must produce the native class', + ); + } else { + assert.ok( + result instanceof FallbackSuppressedError, + 'a runtime without SuppressedError must produce the stand-in', + ); + } + assert.equal(result.name, 'SuppressedError'); + assert.equal(result.message, 'teardown failed'); + }); + + it('builds the fallback stand-in with the same observable shape', () => { + const primary = new Error('primary'); + const secondary = new Error('secondary'); + + const result = new FallbackSuppressedError( + primary, + secondary, + 'teardown failed', + ); + + assert.ok(result instanceof Error); + assert.equal(result.name, 'SuppressedError'); + assert.equal(result.error, primary); + assert.equal(result.suppressed, secondary); + }); +}); + +describe('RECOV-12 close-on-throw over Node Web Streams', () => { + it('closes the in-hand response exactly once and keeps the step error primary', async () => { + let cancels = 0; + const body = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + cancels += 1; + }, + }); + const thrownError = new Error('step failed'); + const chain = new ResponseRecoveryChain( + [ + () => { + throw thrownError; + }, + ], + [], + ); + + const result = await chain.apply(success(aResponse(body))); + + assert.equal(cancels, 1, 'the response must be released exactly once'); + assert.equal(result.kind, 'failure'); + assert.equal( + result.error, + thrownError, + 'the step error must survive by identity', + ); + }); + + it('attaches a close failure as suppressed without displacing the step error', async () => { + const closeError = new Error('close failed'); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + throw closeError; + }, + }); + const originalError = new Error('step failed'); + const chain = new ResponseRecoveryChain( + [ + () => { + throw originalError; + }, + ], + [], + ); + + const result = await chain.apply(success(aResponse(body))); + + assert.equal(result.kind, 'failure'); + assert.equal(result.error.name, 'SuppressedError'); + assert.equal( + result.error.error, + originalError, + 'RECOV-12: the original stays primary', + ); + assert.equal(result.error.suppressed, closeError); + }); +});