From e9c4b4bd8a5bafeedc24365a495a8a37d97bc686 Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Sat, 29 Aug 2026 01:37:38 +0300 Subject: [PATCH 1/4] =?UTF-8?q?feat(transport):=20phase=208a=20=E2=80=94?= =?UTF-8?q?=20the=20fetch=20and=20undici=20adapters,=20and=20the=20file-ba?= =?UTF-8?q?cked=20body.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships the two transport adapters, the file-backed request body, and the one conformance suite both adapters are proven against — the first code in this SDK that puts bytes on the wire, per product-spec/17-transport-adapter-conformance-contract.md (TRANSPORT-1..30), appendix C's SEAM-12/14/15/16/30, NFR-2/15 and BODY-11/12/13, and docs/superpowers/plans/2026-07-28-phase8a-transport-checklist.md. Four published packages and one private: - `@dexpace/transport-fetch` — a `Transport` over the runtime's global `fetch`, zero dependencies beyond its `@dexpace/core` peer. There is no `proxy` option at all: an absent option, not a silently ignored one, because Node's bare `fetch` exposes no proxy hook that does not route through `undici` internals (TRANSPORT-30, scoped out). `close()` is a sanctioned no-op over a runtime global it does not own, so `send()` keeps working after it — this adapter's documented SEAM-15 mode. - `@dexpace/transport-undici` — the full-featured one, taking exactly one external dependency. Ownership-aware `close()` over the dispatchers it constructed and never a bring-your-own one (SEAM-14), `NO_PROXY` bypass routed over a separate direct `Agent`, direct file-body dispatch honoring `start`/`count` (TRANSPORT-28), and a native-internal cancel told apart from a timeout by undici's own codes (TRANSPORT-8). An `Agent`, not a `Pool`: a `Pool` binds to one origin at construction, and a general-purpose transport must reach whatever origin each `Request` names. `undici` is loaded through `createRequire` by path, because Bun resolves the bare specifier to its own shim, whose `Agent` constructs but has no `request`. - `@dexpace/body-file` — the concrete `fileBody()` factory, fail-fast `node:fs` construction validation, a fresh handle per write, short-write detection. Cannot live in core, which imports no `node:` module. Neither transport depends on it: they recognize it structurally through `body.kind === 'file'` and core's type-only `FileBodyDescriptor`, never a cross-package `instanceof`. - `@dexpace/transport-shared` — the header drop/degrade pass, drop-log dedup policy, abort-to-SDK-error mapping, request-body pump, and delivery-detached signal fork. `@internal` exports only; published because a transport's `dependencies` must resolve for consumers. Exists so neither transport has to depend on its sibling. - `@dexpace/transport-conformance` — unpublished. The single `TRANSPORT-N` suite plus its `node:http` fixture server, run once per transport through each package's own `*.conformance.test.ts`, so no requirement is proven for one adapter and assumed for the other. Core gains `TransportFailureError` (TRANSPORT-20's canonical retryable no-response failure) and the type-only `FileBodyDescriptor`, plus a `'file'` member on `Body['kind']`. `IoError` is promoted from `@internal` to `@public` as its base class. The subtyping is the requirement, not modelling convenience: `classify.ts`'s cause-walk already returns true for every `IoError`, so a no-response failure is retryable with no edit to the retry layer. It costs a third hierarchy level against the styleguide's two-level cap, recorded as Deviation Ledger row 17 rather than left silent. SEAM-16 drove `signal-fork.ts`. Both native clients tie the response body's lifetime to whatever signal they were handed, so passing the caller's straight through would let a later `abort()` truncate a body the caller is still reading. Each transport dispatches over a fork it detaches at delivery: cancellation stays live for the whole in-flight window and goes inert afterwards. Five defects found reviewing the staged phase against the plan. One would have taken a consumer's process down. `transport-undici` never kept a handler on a streaming request body's producer. When a server answers before the body finishes — an early 413, a redirect — `send()` has already resolved, and a producer that then fails reaches Node's default `unhandledRejection` policy and terminates the process. `transport-fetch` was immune only incidentally, through the `Promise.race` it uses to surface producer failures. That race is now `producerFailure` in `transport-shared`, used by both, and the guarantee is a conformance row driven by a new `/early-response` fixture — verified to fail on the reintroduced defect and pass once reverted. Its absence is why the suite missed this: TRANSPORT-19 had no undici row at all, against the suite's own rule that no requirement is proven for one adapter and assumed for the other. Every non-abort dispatch failure was classified `TransportFailureError`, and `classify.ts` is an allow-list that returns true for every `IoError` — so undici's argument-validation codes, which are permanent and perfectly reproducible, were reported as always-retryable and would have spent a caller's whole retry budget re-proving the same rejection. `UND_ERR_INVALID_ARG` and `UND_ERR_NOT_SUPPORTED` now leave the `IoError` tree as a `TypeError`, matching `selectDispatchers`, which already reports caller misconfiguration that way. Reachable through a bring-your-own `ProxyAgent`, whose per-request `Proxy-Authorization` the owned-proxy drop set does not cover. The adaptation-throw path released the response body but not the request producer, leaving it parked on backpressure — both adapters, and the one non-delivering exit the TRANSPORT-19 audit trail claimed was covered. `verify:seam-1` had quietly weakened. Generalizing it to an NFR-2 allow-list replaced `deepEqual(dependencies, {})` with a key scan, so a package that omits `dependencies` entirely passed; an omitted field is not a hard-committed empty one. Every package outside the allow-list is held to the original assertion again, with the banner comment rewritten to describe the allow-list model it now implements. BODY-11 and TRANSPORT-28 were each tested in isolation and never together: no test sent a real `fileBody()` through a real transport, which matters most for undici, whose file path bypasses `writeTo` entirely for its own `createReadStream`. Covered now in `test/node-conformance/`, the only layer where a Node-only package and a transport can meet, whole and ranged, for both adapters. Gates: `verify:seam-1` becomes a per-package allow-list, because NFR-2 grants each optional capability core plus at most one external library — `transport-undici` takes `undici`, the rest take none. `verify:dual-consumption` exercises all five new packages under plain `node`; `verify:consumer-types` references every symbol the three consumer-facing ones promote and asserts only that `transport-shared`'s artifact exists, since no consumer is meant to import its `@internal` surface. `lint:publish` and `api` extend to all four published packages. Tests: 72 colocated cases across the four packages, 26 conformance rows run once per transport (capability-gated where §17 scopes a clause to one reference implementation), and eleven Node-runtime cases per adapter under `node --test`, because Bun's `fetch`, `AbortSignal`, and Web Streams are an independent implementation of the surfaces a transport is made of. Each header cites the IDs it exercises. Deliberate gaps, recorded rather than silent. TRANSPORT-18's re-subscribable producer is unbuildable here — neither client drives writes through one, so there is no native internal resend to make idempotent, and 5a's replayability gate covers the SDK's own retries. TRANSPORT-28's literal zero-copy path has no `sendfile`-shaped API in Node's HTTP client stack. TRANSPORT-27's Content-Length half is N/A: `Response.body` is a raw `ReadableStream`, with no declared-length field for a -1 sentinel to live in. TRANSPORT-14's degrade path is tested at its source, not end to end, because both native parsers reject a control byte in a header value at the wire first. TRANSPORT-30's custom `challengeHandler` cannot be dispatched on undici at all — `ProxyAgent` takes its credential solely from its constructor, which runs before any challenge is seen — so it warns at construction and again on the first real 407, and proxy auth falls back to Basic. That last one is a deviation from the phase plan, which had specified a retry-with-stamped-credential flow; Deviation Ledger row 13. --- .changeset/2026-08-28-transport-adapters.md | 15 + bun.lock | 100 +++ .../02-package-and-workspace-layout.md | 7 +- ...-deviations-from-the-reference-contract.md | 22 +- .../2026-07-28-phase8a-transport-checklist.md | 80 +++ package.json | 13 +- packages/body-file/README.md | 44 ++ packages/body-file/api-extractor.json | 22 + packages/body-file/etc/body-file.api.md | 20 + packages/body-file/package.json | 46 ++ packages/body-file/src/file-body.test.ts | 125 ++++ packages/body-file/src/file-body.ts | 88 +++ packages/body-file/src/index.ts | 4 + packages/body-file/src/invariant.ts | 9 + packages/body-file/tsconfig.build.json | 11 + packages/body-file/tsconfig.json | 16 + packages/core/etc/core.api.md | 24 +- packages/core/src/body/body.test.ts | 21 + packages/core/src/body/body.ts | 21 +- packages/core/src/body/index.ts | 2 +- packages/core/src/index.ts | 3 +- packages/core/src/io/errors.test.ts | 15 + packages/core/src/io/errors.ts | 16 +- packages/transport-conformance/package.json | 16 + .../transport-conformance/src/fixtures.ts | 127 ++++ packages/transport-conformance/src/index.ts | 7 + .../transport-conformance/src/run-suite.ts | 624 ++++++++++++++++++ packages/transport-conformance/tsconfig.json | 15 + packages/transport-fetch/README.md | 57 ++ packages/transport-fetch/api-extractor.json | 22 + .../etc/transport-fetch.api.md | 27 + packages/transport-fetch/package.json | 49 ++ .../src/fetch-transport.conformance.test.ts | 14 + .../src/fetch-transport.test.ts | 243 +++++++ .../transport-fetch/src/fetch-transport.ts | 332 ++++++++++ packages/transport-fetch/src/index.ts | 4 + packages/transport-fetch/tsconfig.build.json | 11 + packages/transport-fetch/tsconfig.json | 16 + packages/transport-shared/README.md | 22 + packages/transport-shared/api-extractor.json | 22 + .../etc/transport-shared.api.md | 93 +++ packages/transport-shared/package.json | 46 ++ .../src/abort-mapping.test.ts | 24 + .../transport-shared/src/abort-mapping.ts | 26 + .../transport-shared/src/body-pump.test.ts | 183 +++++ packages/transport-shared/src/body-pump.ts | 153 +++++ .../transport-shared/src/drop-log.test.ts | 73 ++ packages/transport-shared/src/drop-log.ts | 65 ++ .../src/header-mapping.test.ts | 98 +++ .../transport-shared/src/header-mapping.ts | 79 +++ packages/transport-shared/src/index.ts | 17 + .../transport-shared/src/signal-fork.test.ts | 41 ++ packages/transport-shared/src/signal-fork.ts | 52 ++ packages/transport-shared/tsconfig.build.json | 11 + packages/transport-shared/tsconfig.json | 17 + packages/transport-undici/README.md | 88 +++ packages/transport-undici/api-extractor.json | 22 + .../etc/transport-undici.api.md | 27 + packages/transport-undici/package.json | 50 ++ .../src/challenge-handler.test.ts | 145 ++++ .../transport-undici/src/challenge-handler.ts | 92 +++ packages/transport-undici/src/index.ts | 4 + .../src/undici-transport.conformance.test.ts | 12 + .../src/undici-transport.test.ts | 467 +++++++++++++ .../transport-undici/src/undici-transport.ts | 533 +++++++++++++++ packages/transport-undici/tsconfig.build.json | 11 + packages/transport-undici/tsconfig.json | 16 + scripts/verify-consumer-types.mjs | 66 ++ scripts/verify-dual-consumption.mjs | 32 +- scripts/verify-seam-1.mjs | 48 +- scripts/verify-seam-1.test.mjs | 52 +- test/node-conformance/README.md | 1 + test/node-conformance/transport.test.mjs | 340 ++++++++++ 73 files changed, 5292 insertions(+), 24 deletions(-) create mode 100644 .changeset/2026-08-28-transport-adapters.md create mode 100644 docs/superpowers/plans/2026-07-28-phase8a-transport-checklist.md create mode 100644 packages/body-file/README.md create mode 100644 packages/body-file/api-extractor.json create mode 100644 packages/body-file/etc/body-file.api.md create mode 100644 packages/body-file/package.json create mode 100644 packages/body-file/src/file-body.test.ts create mode 100644 packages/body-file/src/file-body.ts create mode 100644 packages/body-file/src/index.ts create mode 100644 packages/body-file/src/invariant.ts create mode 100644 packages/body-file/tsconfig.build.json create mode 100644 packages/body-file/tsconfig.json create mode 100644 packages/core/src/body/body.test.ts create mode 100644 packages/transport-conformance/package.json create mode 100644 packages/transport-conformance/src/fixtures.ts create mode 100644 packages/transport-conformance/src/index.ts create mode 100644 packages/transport-conformance/src/run-suite.ts create mode 100644 packages/transport-conformance/tsconfig.json create mode 100644 packages/transport-fetch/README.md create mode 100644 packages/transport-fetch/api-extractor.json create mode 100644 packages/transport-fetch/etc/transport-fetch.api.md create mode 100644 packages/transport-fetch/package.json create mode 100644 packages/transport-fetch/src/fetch-transport.conformance.test.ts create mode 100644 packages/transport-fetch/src/fetch-transport.test.ts create mode 100644 packages/transport-fetch/src/fetch-transport.ts create mode 100644 packages/transport-fetch/src/index.ts create mode 100644 packages/transport-fetch/tsconfig.build.json create mode 100644 packages/transport-fetch/tsconfig.json create mode 100644 packages/transport-shared/README.md create mode 100644 packages/transport-shared/api-extractor.json create mode 100644 packages/transport-shared/etc/transport-shared.api.md create mode 100644 packages/transport-shared/package.json create mode 100644 packages/transport-shared/src/abort-mapping.test.ts create mode 100644 packages/transport-shared/src/abort-mapping.ts create mode 100644 packages/transport-shared/src/body-pump.test.ts create mode 100644 packages/transport-shared/src/body-pump.ts create mode 100644 packages/transport-shared/src/drop-log.test.ts create mode 100644 packages/transport-shared/src/drop-log.ts create mode 100644 packages/transport-shared/src/header-mapping.test.ts create mode 100644 packages/transport-shared/src/header-mapping.ts create mode 100644 packages/transport-shared/src/index.ts create mode 100644 packages/transport-shared/src/signal-fork.test.ts create mode 100644 packages/transport-shared/src/signal-fork.ts create mode 100644 packages/transport-shared/tsconfig.build.json create mode 100644 packages/transport-shared/tsconfig.json create mode 100644 packages/transport-undici/README.md create mode 100644 packages/transport-undici/api-extractor.json create mode 100644 packages/transport-undici/etc/transport-undici.api.md create mode 100644 packages/transport-undici/package.json create mode 100644 packages/transport-undici/src/challenge-handler.test.ts create mode 100644 packages/transport-undici/src/challenge-handler.ts create mode 100644 packages/transport-undici/src/index.ts create mode 100644 packages/transport-undici/src/undici-transport.conformance.test.ts create mode 100644 packages/transport-undici/src/undici-transport.test.ts create mode 100644 packages/transport-undici/src/undici-transport.ts create mode 100644 packages/transport-undici/tsconfig.build.json create mode 100644 packages/transport-undici/tsconfig.json create mode 100644 test/node-conformance/transport.test.mjs diff --git a/.changeset/2026-08-28-transport-adapters.md b/.changeset/2026-08-28-transport-adapters.md new file mode 100644 index 0000000..6ba8114 --- /dev/null +++ b/.changeset/2026-08-28-transport-adapters.md @@ -0,0 +1,15 @@ +--- +"@dexpace/core": minor +"@dexpace/transport-fetch": minor +"@dexpace/transport-undici": minor +"@dexpace/transport-shared": minor +"@dexpace/body-file": minor +--- + +Add the transport adapters (Phase 8a) — the first code in this SDK that puts bytes on the wire: +- `@dexpace/transport-fetch`: a `Transport` over the runtime's global `fetch`, with zero dependencies beyond its `@dexpace/core` peer. No `proxy` option exists at all (an absent option, not a silently ignored one), and `close()` is a sanctioned no-op over a runtime global it does not own. +- `@dexpace/transport-undici`: the full-featured `Transport`, taking exactly one external dependency. Ownership-aware `close()` over the dispatchers it constructed (never a bring-your-own one), `NO_PROXY` bypass routed over a separate direct `Agent`, direct file-body dispatch honoring `start`/`count`, and a native-internal cancel told apart from a timeout. +- `@dexpace/body-file`: the concrete `fileBody()` factory, with fail-fast `node:fs` construction validation, a fresh handle per write, and short-write detection. Transports recognize it structurally through `body.kind === 'file'`, never a cross-package `instanceof`. +- `@dexpace/transport-shared`: the header drop/degrade pass, drop-log dedup policy, abort-to-SDK-error mapping, request-body pump, and delivery-detached signal fork — `@internal` exports both transports share so the one algorithm exists once rather than twice. +- `@dexpace/core` gains `TransportFailureError` (the canonical retryable no-response failure, an `IoError` subtype) and the type-only `FileBodyDescriptor` plus a `'file'` member on `Body['kind']`. `IoError` is promoted from `@internal` to `@public` as its base class. Note for TypeScript consumers: widening `Body['kind']` is additive for anyone *implementing* `Body`, but an exhaustive `switch (body.kind)` with a `never` default will stop compiling until it handles `'file'`. +- Both transports are proven against one shared `TRANSPORT-N` conformance suite and are `AsyncDisposable`, so `await using` is a single teardown path. Both also keep a handler on a streaming request body's producer for the whole send: a producer that fails *after* the response was delivered (an early `413`, say) is an observed rejection rather than one that reaches the runtime's default `unhandledRejection` policy. `@dexpace/transport-undici` additionally reports undici's argument-validation failures outside the `IoError` tree, so a permanent misconfiguration is terminal rather than retried to exhaustion. diff --git a/bun.lock b/bun.lock index 6edb83a..cdf64ef 100644 --- a/bun.lock +++ b/bun.lock @@ -7,10 +7,15 @@ "devDependencies": { "@arethetypeswrong/cli": "^0.18", "@changesets/cli": "^2", + "@dexpace/body-file": "workspace:*", "@dexpace/codec-json": "workspace:*", "@dexpace/core": "workspace:*", "@dexpace/logging-debug": "workspace:*", "@dexpace/logging-pino": "workspace:*", + "@dexpace/transport-conformance": "workspace:*", + "@dexpace/transport-fetch": "workspace:*", + "@dexpace/transport-shared": "workspace:*", + "@dexpace/transport-undici": "workspace:*", "@eslint-community/eslint-plugin-eslint-comments": "^4", "@microsoft/api-extractor": "catalog:", "@types/bun": "latest", @@ -24,6 +29,20 @@ "typescript-eslint": "^8", }, }, + "packages/body-file": { + "name": "@dexpace/body-file", + "version": "0.0.0", + "devDependencies": { + "@dexpace/core": "workspace:*", + "@microsoft/api-extractor": "catalog:", + "expect-type": "catalog:", + "fast-check": "catalog:", + "typescript": "catalog:", + }, + "peerDependencies": { + "@dexpace/core": "workspace:*", + }, + }, "packages/codec-json": { "name": "@dexpace/codec-json", "version": "0.0.0", @@ -57,7 +76,11 @@ }, "peerDependencies": { "@dexpace/core": "workspace:*", + "debug": ">=4.0.0", }, + "optionalPeers": [ + "debug", + ], }, "packages/logging-pino": { "name": "@dexpace/logging-pino", @@ -69,6 +92,71 @@ "fast-check": "catalog:", "typescript": "catalog:", }, + "peerDependencies": { + "@dexpace/core": "workspace:*", + "pino": ">=8.0.0", + }, + "optionalPeers": [ + "pino", + ], + }, + "packages/transport-conformance": { + "name": "@dexpace/transport-conformance", + "version": "0.0.0", + "devDependencies": { + "@dexpace/core": "workspace:*", + }, + "peerDependencies": { + "@dexpace/core": "workspace:*", + }, + }, + "packages/transport-fetch": { + "name": "@dexpace/transport-fetch", + "version": "0.0.0", + "dependencies": { + "@dexpace/transport-shared": "workspace:*", + }, + "devDependencies": { + "@dexpace/core": "workspace:*", + "@dexpace/transport-conformance": "workspace:*", + "@microsoft/api-extractor": "catalog:", + "expect-type": "catalog:", + "fast-check": "catalog:", + "typescript": "catalog:", + }, + "peerDependencies": { + "@dexpace/core": "workspace:*", + }, + }, + "packages/transport-shared": { + "name": "@dexpace/transport-shared", + "version": "0.0.0", + "devDependencies": { + "@dexpace/core": "workspace:*", + "@microsoft/api-extractor": "catalog:", + "expect-type": "catalog:", + "fast-check": "catalog:", + "typescript": "catalog:", + }, + "peerDependencies": { + "@dexpace/core": "workspace:*", + }, + }, + "packages/transport-undici": { + "name": "@dexpace/transport-undici", + "version": "0.0.0", + "dependencies": { + "@dexpace/transport-shared": "workspace:*", + "undici": "^6.21.1", + }, + "devDependencies": { + "@dexpace/core": "workspace:*", + "@dexpace/transport-conformance": "workspace:*", + "@microsoft/api-extractor": "catalog:", + "expect-type": "catalog:", + "fast-check": "catalog:", + "typescript": "catalog:", + }, "peerDependencies": { "@dexpace/core": "workspace:*", }, @@ -134,6 +222,8 @@ "@colors/colors": ["@colors/colors@1.5.0", "", {}, "sha512-ooWCrlZP11i8GImSjTHYHLkvFDP48nS4+204nGb1RiX/WXYHmJA2III9/e2DWVabCESdW7hBAEzHRqUn9OUVvQ=="], + "@dexpace/body-file": ["@dexpace/body-file@workspace:packages/body-file"], + "@dexpace/codec-json": ["@dexpace/codec-json@workspace:packages/codec-json"], "@dexpace/core": ["@dexpace/core@workspace:packages/core"], @@ -142,6 +232,14 @@ "@dexpace/logging-pino": ["@dexpace/logging-pino@workspace:packages/logging-pino"], + "@dexpace/transport-conformance": ["@dexpace/transport-conformance@workspace:packages/transport-conformance"], + + "@dexpace/transport-fetch": ["@dexpace/transport-fetch@workspace:packages/transport-fetch"], + + "@dexpace/transport-shared": ["@dexpace/transport-shared@workspace:packages/transport-shared"], + + "@dexpace/transport-undici": ["@dexpace/transport-undici@workspace:packages/transport-undici"], + "@eslint-community/eslint-plugin-eslint-comments": ["@eslint-community/eslint-plugin-eslint-comments@4.7.2", "", { "dependencies": { "escape-string-regexp": "^4.0.0", "ignore": "^7.0.5" }, "peerDependencies": { "eslint": "^6.0.0 || ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0" } }, "sha512-LF03qURSwEWm2dz5wtdDCzNk+7Opl0X7q6I3undsaIuNsEiNvRV3BCtqu14Q/6Pzg1tBj44LcxpW2EpSLZStZw=="], "@eslint-community/eslint-utils": ["@eslint-community/eslint-utils@4.10.1", "", { "dependencies": { "eslint-visitor-keys": "^3.4.3" }, "peerDependencies": { "eslint": "^6.0.0 || ^7.0.0 || >=8.0.0" } }, "sha512-cuadcxVFE8sDK6iWJbs8Sn0av2Nrh2QSGQhVlBW9AaAHqHwjWsZHT8LJ4hFGPh7ASBV2deFdM7H/DPjulmh8rg=="], @@ -740,6 +838,8 @@ "typescript-eslint": ["typescript-eslint@8.65.0", "", { "dependencies": { "@typescript-eslint/eslint-plugin": "8.65.0", "@typescript-eslint/parser": "8.65.0", "@typescript-eslint/typescript-estree": "8.65.0", "@typescript-eslint/utils": "8.65.0" }, "peerDependencies": { "eslint": "^8.57.0 || ^9.0.0 || ^10.0.0", "typescript": ">=4.8.4 <6.1.0" } }, "sha512-/ggrHAwyjENDusvyxbuqxAC2dTnZg/Z8F+fgQtYIz+L6n/9HfSlEZcFGV/NsMNa6CkGk0xUjUAFwC0vHOflvIA=="], + "undici": ["undici@6.28.0", "", {}, "sha512-LIY910g9TI13YS95lrMFrs8Rm/u/irgHeTWoKCoteeJ04CUJ92eEfj0rVn+7VKMPBpUPiUoBKfhNyLI23EE/KA=="], + "undici-types": ["undici-types@8.3.0", "", {}, "sha512-j375ScV60dom+YkPFIfTLcOiPxkN/buHz5GobjLhixFuANaNs3C9l4GmrWqejgXWJ7BbJcFYpTEUkS1Ge8bpZQ=="], "unicode-emoji-modifier-base": ["unicode-emoji-modifier-base@1.0.0", "", {}, "sha512-yLSH4py7oFH3oG/9K+XWrz1pSi3dfUrWEnInbxMfArOfc1+33BlGPQtLsOYwvdMy11AwUBetYuaRxSPqgkq+8g=="], diff --git a/docs/sdk-design-nodejs/02-package-and-workspace-layout.md b/docs/sdk-design-nodejs/02-package-and-workspace-layout.md index 6b86915..bcaf7f7 100644 --- a/docs/sdk-design-nodejs/02-package-and-workspace-layout.md +++ b/docs/sdk-design-nodejs/02-package-and-workspace-layout.md @@ -9,8 +9,11 @@ of Gradle's multi-module build graph. |---|---|---|---| | `@dexpace/core` | Domain model, I/O contracts (built directly on Web Streams, not pluggable — see §3.1), execution context, both pipeline layers, retry/redirect/auth, pagination, SSE parsing, the serde SPI + `Tristate`, the instrumentation SPI, configuration. | Any runtime with Web Streams, `fetch`-shaped `AbortSignal`, and `globalThis.crypto` (Node ≥20.3, current evergreen browsers, Deno, Bun, Cloudflare Workers). **Node ≥18.17 was the claim until 2026-08-26 and it was wrong twice over:** Node exposes `globalThis.crypto` unflagged only from 19.0.0 and never to an ES module on any 18.x release, and `AbortSignal.any()` reached the 20.x line in 20.3.0. | none | | `@dexpace/codec-json` | Reference wire codec: `JSON.parse`/`JSON.stringify` plus `Tristate` wiring and Standard-Schema decode glue (§7.3). | same as core | none beyond a `@dexpace/core` peer | -| `@dexpace/transport-fetch` | Minimal transport built on the global `fetch`. The zero-dependency, built-into-the-runtime option — the Node analog of `sdk-transport-jdkhttp`'s "no extra library, but less low-level control" trade-off. | same as core | none beyond a `@dexpace/core` peer | -| `@dexpace/transport-undici` | Full-featured transport built on `undici`'s `Client`/`Pool`/`request()` API: connection-pool tuning, trailers, explicit socket-level cancellation. The Node analog of `sdk-transport-okhttp`'s "richer, but pulls in a real library" trade-off. | Node only | `undici` | +| `@dexpace/transport-fetch` | Minimal transport built on the global `fetch`. The zero-dependency, built-into-the-runtime option — the Node analog of `sdk-transport-jdkhttp`'s "no extra library, but less low-level control" trade-off. | Node/Bun. Its dependency list would run anywhere, but `redirect: 'manual'` — **TRANSPORT-1**'s mechanism — returns the raw 3xx only on an `undici`-backed runtime; a browser returns an opaque-redirect response (status `0`, no headers) the pipeline cannot redirect with. | none beyond a `@dexpace/core` peer | +| `@dexpace/transport-undici` | Full-featured transport built on `undici`'s `Dispatcher`/`request()` API: connection-pool tuning, proxy routing, ownership-aware close, explicit socket-level cancellation. The Node analog of `sdk-transport-okhttp`'s "richer, but pulls in a real library" trade-off. The owned dispatcher is an `Agent`, not a `Pool` — a `Pool` is bound to one origin at construction, and a general-purpose transport must reach whatever origin each `Request` names (Phase 8a design §4). | Node only | `undici` | +| `@dexpace/body-file` | The concrete `fileBody()` factory: a file-backed request `Body` with fail-fast `node:fs` construction validation. Cannot live in core (zero-`node:`-import invariant); is **not** an upstream of either transport, which recognize it structurally through `@dexpace/core`'s type-only `FileBodyDescriptor` and a `body.kind === 'file'` check (Phase 8a design §5). | Node only | none beyond a `@dexpace/core` peer | +| `@dexpace/transport-shared` | Internal plumbing both transports need identically — header drop/degrade, drop-log dedup, abort→SDK-error mapping, request-body pumping, and the delivery-detached signal fork. `@internal` exports only; published because `NFR-4` snapshots every published unit and because a transport's `dependencies` must resolve for consumers. Exists so neither transport has to depend on its sibling (Phase 8a design §7). | same as core | none beyond a `@dexpace/core` peer | +| `@dexpace/transport-conformance` | Unpublished. The one `TRANSPORT-N` conformance suite plus its `node:http` fixture server, run once per transport package so the two adapters cannot drift (Phase 8a design §8). | — | dev-only | | `@dexpace/logging-pino` | Bridges the core `Logger` seam to a caller-supplied `pino` instance. | Node/any pino-compatible runtime | `pino` (peer) | | `@dexpace/logging-debug` | Bridges the core `Logger` seam to the ubiquitous zero-config `debug` package, for consumers who want a logger with no configuration story at all. | any | `debug` (peer) | | `@dexpace/rx` | Thin optional sugar exposing pagination and SSE as RxJS `Observable`s for teams already standardized on RxJS (notably Angular shops). Not a bridge for the request/response pivot itself — see §3.2. | any | `rxjs` (peer) | diff --git a/docs/sdk-design-nodejs/10-deliberate-deviations-from-the-reference-contract.md b/docs/sdk-design-nodejs/10-deliberate-deviations-from-the-reference-contract.md index 5b4346e..62846d2 100644 --- a/docs/sdk-design-nodejs/10-deliberate-deviations-from-the-reference-contract.md +++ b/docs/sdk-design-nodejs/10-deliberate-deviations-from-the-reference-contract.md @@ -128,7 +128,17 @@ Phase 10 (`docs/superpowers/specs/2026-07-28-phase10-deviation-reconciliation-de path. Neither transport retries a partial send internally (**TRANSPORT-18**); the SDK's own retry layer handles it via the replayability gate instead. `Response.protocol` is a hardcoded `HTTP_1_1` best-effort default because neither `fetch`'s `Response` nor undici's `ResponseData` surface the negotiated protocol - version (all: Phase 8a). + version (all: Phase 8a). Phase 8a's implementation added one more, found only by building it: **a custom + proxy `challengeHandler` cannot be dispatched by `transport-undici` either** — undici's `ProxyAgent` takes + its credential solely from its own constructor and rejects any per-request `Proxy-Authorization` with + `InvalidArgumentError` (a deliberate security fix on their side), and the constructor runs before any + challenge has been seen, so no handler-minted credential can reach the exchange that provoked it. This is + the case **TRANSPORT-30**'s own text anticipates: the handler is surfaced with a WARN at construction and + again on the first real `407`, proxy auth falls back to Basic (`ProxyOptions.credentials`, which *is* + passed to the `ProxyAgent` constructor), the `407` reaches the caller untouched, and a per-request + `Proxy-Authorization` is dropped from the outbound pass — logged by name like any other drop — rather than + turning every proxied send into a hard failure. The Phase 8a *plan* had specified a retry-with-stamped- + credential flow instead; that flow is not implementable on this platform (Phase 8a). 14. **Reproducible builds and publish provenance stay open, unblocking only at first real release.** **NFR-12** (byte-identical builds from identical source) and **NFR-16** (publish provenance enforced on the release path) are soft gaps: `bun install --frozen-lockfile` and plain `tsc` are deterministic by construction, and @@ -155,3 +165,13 @@ Phase 10 (`docs/superpowers/specs/2026-07-28-phase10-deviation-reconciliation-de different data shape — push-based `Observable`s — not plumbing for the request/response pivot; its `sseEvents$`/`typedSse$` are single-subscription, not standard cold/repeatable Observables, because `SseStream` wraps an already-consumed-once HTTP response body (Phase 8b). +17. **`TransportFailureError` adds a third level to an error tree the styleguide caps at two.** The + styleguide holds custom error hierarchies to two levels deep, and Phase 3a flattened this very tree to + obey it — the four I/O leaves extend `DexpaceError` directly, and `isIoError` exists to group them + without reintroducing a middle tier (`packages/core/src/io/errors.ts`). Phase 8a's **TRANSPORT-20** + reintroduces one: `TransportFailureError extends IoError extends DexpaceError`. The subtyping *is* the + requirement rather than an accident of modelling — `classify.ts`'s cause-walk returns `true` for every + `IoError`, so extending it is what makes a no-response failure retryable with no edit to the retry + layer, and a flat sibling would have to be named there by hand and again for every transport added + later. One level of depth buys the canonical-subtype clause. Held at exactly three: a fourth level is + not sanctioned by this row (Phase 8a). diff --git a/docs/superpowers/plans/2026-07-28-phase8a-transport-checklist.md b/docs/superpowers/plans/2026-07-28-phase8a-transport-checklist.md new file mode 100644 index 0000000..27c68fd --- /dev/null +++ b/docs/superpowers/plans/2026-07-28-phase8a-transport-checklist.md @@ -0,0 +1,80 @@ +# Phase 8a — Transport Adapters — Checklist + +**Status: EXECUTED.** Every ✅ below names code and tests that exist on this branch, not a plan step. Verified +against `docs/product-spec/17-transport-adapter-conformance-contract.md` over every requirement ID +(`TRANSPORT-1` through `TRANSPORT-30`), plus the `SEAM`/`NFR` rows the roadmap parks on this phase. + +**Legend:** ✅ Implemented and tested — 🚫 Not built (permanent simplification, named reason) — ⏳ Deferred +(named target phase) — N/A Not applicable in this port. + +Paths are relative to the repo root. `run-suite.ts` means +`packages/transport-conformance/src/run-suite.ts`, the single suite both transports run through their own +`*.conformance.test.ts`, so no row below is proven for one transport and assumed for the other. + +## 17.1 Pipeline authority + +| ID | Level | Requirement gist | Status | Where | +|---|---|---|---|---| +| TRANSPORT-1 | MUST | Native redirect following disabled; default off | ✅ | `fetch-transport.ts` pins `redirect: 'manual'`, `undici-transport.ts` pins `maxRedirections: 0` even behind a BYO dispatcher; asserted in `run-suite.ts` ("a 302 is returned raw") and per package in `fetch-transport.test.ts` / `undici-transport.test.ts` | +| TRANSPORT-2 | MUST | Native automatic retry disabled | ✅ | Satisfied by construction, not by a flag: `fetch` has no automatic-retry feature to disable, and `undici-transport.ts` composes a plain `Agent`/`ProxyAgent` — never a `RetryAgent` and never a retry interceptor. The only path to a retrying dispatcher is a caller supplying one as `dispatcher`, which is their own decision about their own client (SEAM-14). No standalone assertion: there is no observable knob to read back, and a test asserting "we did not import `RetryAgent`" would be a tautology over the import list | + +## 17.2 Cancellation and timeout classification + +| ID | Level | Requirement gist | Status | Where | +|---|---|---|---|---| +| TRANSPORT-3 | MUST | Caller cancellation is terminal, never the retryable type; discriminated out-of-band | ✅ | `transport-shared/src/abort-mapping.ts` `abortToSdkError` branches on `isTimeoutSignal` (a structured `reason.name` check, not a message match); asserted in `abort-mapping.test.ts` and `run-suite.ts` | +| TRANSPORT-4 | MUST | Read/response timeout is a RETRYABLE transport failure, cancellation flag clear | ✅ | Same mapping returns `TransportFailureError` (an `IoError` subtype) for a timeout signal; asserted in `run-suite.ts` and `test/node-conformance/transport.test.mjs` | +| TRANSPORT-5 | MUST | Per-call timeout applies to that call only | ✅ | `composeSignal(signal, options?.timeoutMs ?? defaultTimeoutMs)` per send, never on the instance; asserted in `run-suite.ts` ("two concurrent calls are each bounded by their own timeout") | +| TRANSPORT-6 | SHOULD | A sub-resolution positive timeout is not truncated to zero | ✅ | N/A in mechanism — `AbortSignal.timeout(ms)` is already millisecond-resolution with no zero-means-no-timeout coercion — but asserted anyway in `run-suite.ts` ("a sub-resolution 1ms timeout still times out rather than hanging") so a future coarser implementation cannot regress it silently | +| TRANSPORT-7 | MUST | Cancelling in flight propagates into the native exchange and releases it | ✅ | The composed signal is forwarded to `fetch`/undici through `forkSignal`; asserted in `run-suite.ts` ("a cancelled exchange leaves no handle that stalls close()") | +| TRANSPORT-8 | MUST | A native-internal cancel completes terminal while a timeout on the same path stays retryable | ✅ (undici) / N/A (fetch) | `undici-transport.ts` maps `UND_ERR_DESTROYED`/`UND_ERR_ABORTED`/`UND_ERR_CLOSED` to `CancellationError`; asserted in `undici-transport.test.ts` (destroying the dispatcher mid-flight) with the timeout twin alongside it, and gated in `run-suite.ts` on `supportsInternalCancel`. `fetch` has no internal-cancel path distinct from an abort — the requirement's own text scopes it out | +| TRANSPORT-9 | MUST | An adaptation-race response is still closed | ✅ | Both transports re-check the composed signal after dispatch and cancel/`dump()` the native body before rejecting; asserted in `run-suite.ts` ("a timeout while headers are still pending releases the connection") | + +## 17.3 Header and body mapping + +| ID | Level | Requirement gist | Status | Where | +|---|---|---|---|---| +| TRANSPORT-10 | MUST | Caller Content-Type authoritative; body-derived emitted only when none set | ✅ | `transport-shared/src/header-mapping.ts` `mapOutboundHeaders` checks `headers.get('content-type')` before stamping; asserted in `header-mapping.test.ts` (both directions) and end to end in `run-suite.ts` | +| TRANSPORT-11 | MUST | Framing headers dropped before dispatch, each drop logged at verbose | ✅ | `FETCH_FORBIDDEN_HEADERS` (adds `connection`) / `UNDICI_FORBIDDEN_HEADERS` (does not — §17's own note); drops routed through `createDropLogger`. Asserted in `header-mapping.test.ts`, per package in each transport's unit test, and through the drop log itself in `run-suite.ts` | +| TRANSPORT-12 | MUST | A wire-invalid header degrades to a drop, never a failed send | ✅ | `mapOutboundHeaders` catches per header; `fetch-transport.ts` additionally catches `Headers.append` rejections. Asserted in `header-mapping.test.ts` | +| TRANSPORT-13 | SHOULD | Configurable drop-log policy; case-insensitive, bounded dedup | ✅ | `transport-shared/src/drop-log.ts`: `'all' \| 'first-per-name' \| 'quiet'`, lower-cased keys, drain-to-cap at `MAX_LOGGED_DROP_NAMES`; asserted in `drop-log.test.ts` including the synthesised-name burst | +| TRANSPORT-14 | MUST | Lenient inbound copy; control-byte header dropped, obs-text value preserved | ✅ | `degradeInboundHeaders` writes through `Headers`'s lenient `addInbound` path; asserted in `header-mapping.test.ts`. **Not** asserted end to end: both native HTTP parsers reject a control byte in a header value at the wire (`Malformed_HTTP_Response`) before a transport ever sees it, so the fixture that would drive it is unbuildable — the degrade path is a real defence for hostile/synthetic responses and is tested at its source | + +## 17.4 Lifecycle and ownership + +| ID | Level | Requirement gist | Status | Where | +|---|---|---|---|---| +| TRANSPORT-15 | MUST | Ownership-aware close; a BYO client is never shut down | ✅ | `undici-transport.ts` `selectDispatchers` returns an `owned` list that is empty for a BYO dispatcher; `close()` iterates only that list, in reverse acquisition order, and includes a transport-constructed `ProxyAgent`. Asserted in `undici-transport.test.ts`. `fetch`'s `close()` is a sanctioned no-op over a runtime global it does not own | +| TRANSPORT-16 | MUST | `close()` idempotent, non-blocking, interrupt-safe | ✅ | `#closing` memoizes one teardown so concurrent calls share it. undici's dispatchers are `destroy()`ed, not gracefully `close()`d, precisely because of the "no unbounded await" clause — a graceful close waits out every enqueued request; asserted in `undici-transport.test.ts` ("close does not wait out an in-flight request"). Idempotency asserted in `run-suite.ts` and both unit tests. Both transports are also `AsyncDisposable`, so `await using` is a single teardown path | +| TRANSPORT-17 | MUST | A single-use body is written to the wire exactly once | ✅ | `transport-shared/src/body-pump.ts` runs `writeTo` once per send and neither transport re-invokes it; asserted in `body-pump.test.ts`, `run-suite.ts` (a counting body whose bytes are read back off the wire), and `test/node-conformance/transport.test.mjs` | +| TRANSPORT-18 | MUST | Re-subscribable producer replays identical bytes | 🚫 | Deviation Ledger: neither `fetch` nor undici drives writes through a re-subscribable producer, so there is no native internal resend to make idempotent. The SDK's own retry layer (5a) re-invokes `send()` against the original `Body`, already gated by `RETRY-5`/`RETRY-7`'s replayability check | +| TRANSPORT-19 | SHOULD | An abandoned streaming subscription unblocks its producer; teardown idempotent | ✅ | `BodyPump.abandon` aborts the writer and awaits the producer's unwind; both transports call it on every non-delivering exit path, the adaptation-throw path included. Separately, both hold a handler on the producer's settlement through `producerFailure` for the whole send — without one, a producer that fails *after* the response was delivered (an early `413`, say) reaches the runtime's default `unhandledRejection` policy and takes the process down. Asserted in `body-pump.test.ts` (a producer parked on backpressure forever is released, twice over), `fetch-transport.test.ts` (a producer failure races the pending fetch and fails the send), and — for both transports — `run-suite.ts` ("a producer that fails after delivery does not escape as an unhandled rejection", driven by the `/early-response` fixture) | + +## 17.5 Failure and response mapping + +| ID | Level | Requirement gist | Status | Where | +|---|---|---|---|---| +| TRANSPORT-20 | MUST | A no-response failure is the canonical retryable I/O subtype | ✅ | `packages/core/src/io/errors.ts` `TransportFailureError extends IoError`, so 5a's `classify.ts` cause-walk already treats it as always-retryable; asserted in `errors.test.ts`, `run-suite.ts`, and the Node layer. The subtyping costs a third hierarchy level against the styleguide's two-level cap — Deviation Ledger row 17. The converse is enforced too: `undici-transport.ts` `toDispatchError` keeps undici's argument-validation codes (`UND_ERR_INVALID_ARG`, `UND_ERR_NOT_SUPPORTED`) *outside* the `IoError` tree, because `classify.ts` is an allow-list and a permanent misconfiguration classified as retryable would spend a caller's whole budget re-proving itself. Asserted in `undici-transport.test.ts` with its retryable twin alongside | +| TRANSPORT-21 | MUST | A pre-dispatch failure arrives through the promise, never a synchronous throw | ✅ | `send` is `async` throughout; asserted in `run-suite.ts` | +| TRANSPORT-22 | MUST | An adaptation throw closes the native response first | ✅ | Both transports wrap `adaptResponse` and cancel (`body.cancel()`) / destroy (`body.destroy()`) before rethrowing; asserted by injection in `fetch-transport.test.ts` and `undici-transport.test.ts`, which is the only way to reach it — a conforming wire response has no field whose adaptation can fail | +| TRANSPORT-23 | MUST | Success never resolves to null | ✅ | The return type is `Promise` and `Response.newBuilder().build()` enforces its required fields; asserted in `run-suite.ts` | +| TRANSPORT-24 | MUST | Vendor status codes surfaced faithfully, body readable and closeable | ✅ | `Status.of` is total by construction (HTTP-10); asserted with a 520 in `run-suite.ts` and the Node layer | +| TRANSPORT-25 | MUST | Response body is a lazily-read stream; close cascades and releases the connection | ✅ | `fetch` hands over `Response.body` unbuffered; undici goes through `toDemandDrivenStream`, a pull-based adapter (deliberately not `Readable.toWeb`, which throws `ERR_INVALID_STATE` on Bun when closed without being drained, and deliberately not a `'data'`-listener adapter, which would buffer eagerly). Asserted in `run-suite.ts` against a dripping fixture — a first chunk in hand while the stream is still open — plus close-without-reading and idempotent close | +| TRANSPORT-26 | MUST | A body-less request is valid for any method; zero-length body substituted where required | ✅ | Neither native client rejects a null body, so no substitution is needed; asserted in `run-suite.ts` that a body-less POST dispatches with `Content-Length: 0` on the wire | +| TRANSPORT-27 | SHOULD | Malformed inbound Content-Type downgrades; absent Content-Length maps to -1 | ✅ / N/A | The Content-Type half is asserted in `run-suite.ts` (an unparseable type still delivers a 200 and a readable body — nothing parses it at the transport layer, so nothing can fail on it). The Content-Length half is **N/A in this port**: `Response.body` is a raw `ReadableStream`, and this port has no response-side declared-length field for a -1 sentinel to live in | +| TRANSPORT-28 | SHOULD | File body streams directly, honoring start/count; treated as replayable | ✅ (direct stream) / 🚫 (zero-copy) | `undici-transport.ts` `isFileBody` narrows structurally on `kind === 'file'` and dispatches `createReadStream(path, {start, end})`; asserted byte-exactly over the wire in `undici-transport.test.ts`. `fileBody()` is always `replayable: true` with a fresh handle per write (`body-file/src/file-body.ts`, `file-body.test.ts`). A literal kernel zero-copy path is a Deviation Ledger row — Node's HTTP client stack exposes no `sendfile`-shaped API for outbound bodies | +| TRANSPORT-29 | MUST | Concurrent-safe, effectively immutable after construction | ✅ | All per-request state lives in locals and the returned promise graph; every instance field is `readonly` except the memoized `#closing`. Asserted in `run-suite.ts` (20 concurrent sends, each response matched to its own request by a per-call header) and the Node layer | +| TRANSPORT-30 | SHOULD | Unsupported proxy features discoverable; credentials never logged, never answered to a 401 | ✅ | `undici-transport.ts` + `challenge-handler.ts`: a custom `challengeHandler` warns at construction and again on the first real 407, proxy auth falls back to Basic (`ProxyOptions.credentials`, passed to the `ProxyAgent` constructor), a 401 is never treated as a proxy challenge, and no credential reaches the logger on any path. A per-request `Proxy-Authorization` is dropped when a proxy is configured, because `ProxyAgent.dispatch` rejects one outright. Asserted in `challenge-handler.test.ts` and `undici-transport.test.ts`. **This is a deviation from the Phase 8a plan**, which specified a retry-with-stamped-credential flow; that flow is not implementable on undici — see `docs/sdk-design-nodejs/10-deliberate-deviations-from-the-reference-contract.md` row 13. `transport-fetch` has no `proxy` option at all (design §6) | + +## Roadmap rows this phase closes + +| ID | Requirement gist | Status | Where | +|---|---|---|---| +| SEAM-12 | Concurrent-call conformance test | ✅ | Collapses onto `TRANSPORT-29`; `run-suite.ts` and `test/node-conformance/transport.test.mjs` | +| SEAM-14 | Close behavior: idempotent, ownership-aware, releases only self-created resources | ✅ | Collapses onto `TRANSPORT-15`/`TRANSPORT-16`; `undici-transport.test.ts` | +| SEAM-15 | Post-close `send()` behavior documented per adapter | ✅ | `fetch` keeps working (no-op close, nothing was released); undici rejects with the terminal `CancellationError`, since the dispatcher the send would route over no longer exists and no retry over it can succeed. Stated in each `close()` TSDoc and each package README | +| SEAM-16 | An abort after the promise resolved must not close the delivered body | ✅ | `transport-shared/src/signal-fork.ts`: both clients tie the body's lifetime to the signal they were handed, so each transport dispatches over a fork it detaches at delivery. Asserted in `signal-fork.test.ts`, `run-suite.ts`, and the Node layer | +| SEAM-30 | Cancel an orphaned response on the completion race | ✅ | Collapses onto `TRANSPORT-9`; `run-suite.ts` | +| NFR-2 | Each optional capability separately installable (core + ≤1 external lib) | ✅ | `transport-fetch`/`body-file`/`transport-shared` take zero external libs; `transport-undici` takes exactly one (`undici`). Gate-enforced by `scripts/verify-seam-1.mjs`'s per-package allow-list, which is now the *only* way to declare a runtime dependency: every package absent from it is still held to a hard-committed empty `dependencies` object, an omitted field included. `scripts/verify-seam-1.test.mjs` drives both halves | +| NFR-15 | The stamped identity actually reaches the wire | ✅ | `run-suite.ts` sends `getBuildInfo().identityTokens` as `User-Agent` and reads it back off the fixture server unmangled, for both transports | +| BODY-11/12/13 | File-backed body: fail-fast validation, recognizable by type, short-write detection | ✅ | `body-file/src/file-body.ts` + `file-body.test.ts`; recognition through `@dexpace/core`'s type-only `FileBodyDescriptor`. Neither transport depends on `@dexpace/body-file`, so a real `fileBody()` crossing a real transport has no home in either package's own suite — it is asserted in `test/node-conformance/transport.test.mjs` instead, whole and ranged, for both adapters. That is the only place the two halves meet: `transport-undici` bypasses `writeTo` entirely for its own `createReadStream` | diff --git a/package.json b/package.json index 9c1ace2..85ced8f 100644 --- a/package.json +++ b/package.json @@ -16,10 +16,15 @@ "devDependencies": { "@arethetypeswrong/cli": "^0.18", "@changesets/cli": "^2", + "@dexpace/body-file": "workspace:*", "@dexpace/codec-json": "workspace:*", "@dexpace/core": "workspace:*", "@dexpace/logging-debug": "workspace:*", "@dexpace/logging-pino": "workspace:*", + "@dexpace/transport-conformance": "workspace:*", + "@dexpace/transport-fetch": "workspace:*", + "@dexpace/transport-shared": "workspace:*", + "@dexpace/transport-undici": "workspace:*", "@eslint-community/eslint-plugin-eslint-comments": "^4", "@microsoft/api-extractor": "catalog:", "@types/bun": "latest", @@ -39,16 +44,16 @@ "lint": "bun run build:core && gts lint .", "fix": "bun run build:core && gts fix .", "build:core": "tsc -b packages/core/tsconfig.build.json", - "typecheck": "bun run build:core && tsc -p packages/core/tsconfig.json --noEmit && tsc -p packages/codec-json/tsconfig.json --noEmit && tsc -p packages/logging-pino/tsconfig.json --noEmit && tsc -p packages/logging-debug/tsconfig.json --noEmit", + "typecheck": "bun run build:core && tsc -p packages/core/tsconfig.json --noEmit && tsc -p packages/codec-json/tsconfig.json --noEmit && tsc -p packages/logging-pino/tsconfig.json --noEmit && tsc -p packages/logging-debug/tsconfig.json --noEmit && tsc -p packages/body-file/tsconfig.json --noEmit && tsc -p packages/transport-shared/tsconfig.json --noEmit && tsc -p packages/transport-conformance/tsconfig.json --noEmit && tsc -p packages/transport-fetch/tsconfig.json --noEmit && tsc -p packages/transport-undici/tsconfig.json --noEmit", "prebuild": "bun run --cwd packages/core prebuild", - "build": "bun run build:core && tsc -p packages/codec-json/tsconfig.build.json && tsc -p packages/logging-pino/tsconfig.build.json && tsc -p packages/logging-debug/tsconfig.build.json", + "build": "bun run build:core && tsc -p packages/codec-json/tsconfig.build.json && tsc -p packages/logging-pino/tsconfig.build.json && tsc -p packages/logging-debug/tsconfig.build.json && tsc -p packages/body-file/tsconfig.build.json && tsc -p packages/transport-shared/tsconfig.build.json && tsc -p packages/transport-fetch/tsconfig.build.json && tsc -p packages/transport-undici/tsconfig.build.json", "test": "bun test", "knowledge": "node scripts/knowledge.mjs", "test:scripts": "node --test 'scripts/*.test.mjs'", "test:node": "node --test test/node-conformance/*.test.mjs", "bench": "bun run packages/core/src/io/byte-queue.bench.ts", - "api": "cd packages/core && bun run api:ci && cd ../codec-json && bun run api:ci && cd ../logging-pino && bun run api:ci && cd ../logging-debug && bun run api:ci", - "lint:publish": "publint packages/core && attw --pack packages/core --ignore-rules cjs-resolves-to-esm && publint packages/codec-json && attw --pack packages/codec-json --ignore-rules cjs-resolves-to-esm && publint packages/logging-pino && attw --pack packages/logging-pino --ignore-rules cjs-resolves-to-esm && publint packages/logging-debug && attw --pack packages/logging-debug --ignore-rules cjs-resolves-to-esm", + "api": "cd packages/core && bun run api:ci && cd ../codec-json && bun run api:ci && cd ../logging-pino && bun run api:ci && cd ../logging-debug && bun run api:ci && cd ../body-file && bun run api:ci && cd ../transport-shared && bun run api:ci && cd ../transport-fetch && bun run api:ci && cd ../transport-undici && bun run api:ci", + "lint:publish": "publint packages/core && attw --pack packages/core --ignore-rules cjs-resolves-to-esm && publint packages/codec-json && attw --pack packages/codec-json --ignore-rules cjs-resolves-to-esm && publint packages/logging-pino && attw --pack packages/logging-pino --ignore-rules cjs-resolves-to-esm && publint packages/logging-debug && attw --pack packages/logging-debug --ignore-rules cjs-resolves-to-esm && publint packages/body-file && attw --pack packages/body-file --ignore-rules cjs-resolves-to-esm && publint packages/transport-shared && attw --pack packages/transport-shared --ignore-rules cjs-resolves-to-esm && publint packages/transport-fetch && attw --pack packages/transport-fetch --ignore-rules cjs-resolves-to-esm && publint packages/transport-undici && attw --pack packages/transport-undici --ignore-rules cjs-resolves-to-esm", "audit": "bun audit --audit-level=high --prod", "changeset": "node scripts/changeset.mjs", "verify:dual-consumption": "node scripts/verify-dual-consumption.mjs", diff --git a/packages/body-file/README.md b/packages/body-file/README.md new file mode 100644 index 0000000..d29692f --- /dev/null +++ b/packages/body-file/README.md @@ -0,0 +1,44 @@ +# @dexpace/body-file + +A file-backed request `Body` for the dexpace SDK. Zero dependencies beyond a `@dexpace/core` peer — +`node:fs` is a runtime API, not an npm package, which is exactly why this lives here and not in +`@dexpace/core` (whose zero-`node:`-import invariant is hard). + +```sh +bun add @dexpace/body-file @dexpace/core +``` + +```typescript +import {Request} from '@dexpace/core'; +import {fileBody} from '@dexpace/body-file'; + +// Validated at construction, not at send time (HTTP-40, BODY-11). +const body = fileBody('./upload.bin', {start: 1024, count: 4096}); + +const request = Request.newBuilder() + .method('POST') + .url('https://example.com/v1/uploads') + .body(body) + .build(); +``` + +## Fail-fast construction + +`fileBody()` stats the path immediately and rejects all four ways it can be wrong, none of which +follows from another: the path must exist and be a **regular** file; `start >= 0`; `start <= size`; +`count >= 0`; and `start + count <= size`. The `start <= size` check earns its place — `count` +defaults to `size - start`, which goes *negative* for a start past end-of-file and then satisfies +the sum check, silently producing a zero-byte upload instead of an error. + +## Behavior worth knowing + +- `replayable` is always `true`, and `writeTo()` opens a **fresh** handle per call, so a retry + re-sends the same bytes (`HTTP-40`). +- `writeTo()` does not close the sink it was handed — closing belongs to whoever created it + (`BODY-8`) — and aborts it on failure so a consumer sees the error rather than a silently + truncated stream. The read handle is destroyed on every exit path, so a failed send strands no + file descriptor. +- A short read raises rather than reporting success (`BODY-13`). +- Transports recognize the result **structurally**, through `body.kind === 'file'`, never an + `instanceof` against this package: `@dexpace/transport-undici` dispatches straight off the file, + and neither transport depends on this package. diff --git a/packages/body-file/api-extractor.json b/packages/body-file/api-extractor.json new file mode 100644 index 0000000..455423c --- /dev/null +++ b/packages/body-file/api-extractor.json @@ -0,0 +1,22 @@ +{ + "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json", + "mainEntryPointFilePath": "/dist/index.d.ts", + "apiReport": { + "enabled": true, + "reportFolder": "/etc/" + }, + "docModel": { + "enabled": false + }, + "dtsRollup": { + "enabled": false + }, + "messages": { + "extractorMessageReporting": { + "ae-forgotten-export": { + "logLevel": "error", + "addToApiReportFile": false + } + } + } +} diff --git a/packages/body-file/etc/body-file.api.md b/packages/body-file/etc/body-file.api.md new file mode 100644 index 0000000..24f20dc --- /dev/null +++ b/packages/body-file/etc/body-file.api.md @@ -0,0 +1,20 @@ +## API Report File for "@dexpace/body-file" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts + +import type { FileBodyDescriptor } from '@dexpace/core'; + +// @public +export function fileBody(path: string, options?: FileBodyOptions): FileBodyDescriptor; + +// @public +export interface FileBodyOptions { + readonly count?: number; + readonly start?: number; +} + +// (No @packageDocumentation comment for this package) + +``` diff --git a/packages/body-file/package.json b/packages/body-file/package.json new file mode 100644 index 0000000..74de24e --- /dev/null +++ b/packages/body-file/package.json @@ -0,0 +1,46 @@ +{ + "name": "@dexpace/body-file", + "version": "0.0.0", + "description": "File body adapter with fail-fast node:fs validation for the dexpace Node.js SDK.", + "license": "MIT", + "type": "module", + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "engines": { + "node": ">=20.3" + }, + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + } + }, + "files": [ + "dist" + ], + "sideEffects": false, + "dependencies": {}, + "peerDependencies": { + "@dexpace/core": "workspace:*" + }, + "peerDependenciesMeta": { + "@dexpace/core": { + "optional": false + } + }, + "devDependencies": { + "@dexpace/core": "workspace:*", + "@microsoft/api-extractor": "catalog:", + "expect-type": "catalog:", + "fast-check": "catalog:", + "typescript": "catalog:" + }, + "scripts": { + "build": "tsc -p tsconfig.build.json", + "typecheck": "tsc --noEmit", + "test": "bun test", + "api:local": "api-extractor run --local", + "api:ci": "api-extractor run", + "prepublishOnly": "bun run build && bun run api:ci && publint . && attw --pack . --ignore-rules cjs-resolves-to-esm" + } +} diff --git a/packages/body-file/src/file-body.test.ts b/packages/body-file/src/file-body.test.ts new file mode 100644 index 0000000..1f48a9e --- /dev/null +++ b/packages/body-file/src/file-body.test.ts @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: MIT +// packages/body-file/src/file-body.test.ts +// Exercises: HTTP-40/BODY-11 (fail-fast construction validation, fresh handle per write, replayable), +// BODY-13 (short-write detection), BODY-12/TRANSPORT-28 (recognizable by type) +/* eslint-disable max-lines-per-function -- file body tests need full I/O lifecycle setup */ +import {afterEach, beforeEach, describe, expect, test} from 'bun:test'; +import {mkdtemp, rm, writeFile} from 'node:fs/promises'; +import {tmpdir} from 'node:os'; +import {join} from 'node:path'; +import {fileBody} from './file-body.js'; + +let dir: string; +let filePath: string; + +beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), 'body-file-')); + filePath = join(dir, 'payload.bin'); + await writeFile(filePath, new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8])); +}); + +afterEach(async () => { + await rm(dir, {recursive: true, force: true}); +}); + +describe('fileBody (HTTP-40, BODY-11)', () => { + test('is recognizable by kind and replayable', () => { + const body = fileBody(filePath); + expect(body.kind).toBe('file'); + expect(body.replayable).toBe(true); + expect(body.contentLength).toBe(8); + expect(body.mediaType).toBeUndefined(); + }); + + test('rejects a nonexistent path at construction', () => { + expect(() => fileBody(join(dir, 'missing.bin'))).toThrow(); + }); + + test('rejects a directory path at construction', () => { + expect(() => fileBody(dir)).toThrow(); + }); + + test('rejects a negative start or out-of-range count at construction', () => { + expect(() => fileBody(filePath, {start: -1})).toThrow(); + expect(() => fileBody(filePath, {start: 4, count: 10})).toThrow(); + expect(() => fileBody(filePath, {count: -1})).toThrow(); + expect(() => fileBody(filePath, {start: 100})).toThrow(); + }); + + test('writeTo does not close the caller-owned sink', async () => { + const body = fileBody(filePath); + let closed = false; + const sink = new WritableStream({ + close() { + closed = true; + }, + write() { + // no-op: we only care about close tracking + }, + }); + await body.writeTo(sink); + expect(closed).toBe(false); + }); + + test('writeTo streams exactly the declared byte range', async () => { + const body = fileBody(filePath, {start: 2, count: 4}); + const chunks: Uint8Array[] = []; + const sink = new WritableStream({ + write(chunk) { + chunks.push(chunk); + }, + }); + await body.writeTo(sink); + const totalLength = chunks.reduce((acc, c) => acc + c.byteLength, 0); + const written = new Uint8Array(totalLength); + let offset = 0; + for (const chunk of chunks) { + written.set(chunk, offset); + offset += chunk.byteLength; + } + expect(written).toEqual(new Uint8Array([3, 4, 5, 6])); + }); + + test('writeTo handles 0 count', async () => { + const body = fileBody(filePath, {start: 0, count: 0}); + const chunks: Uint8Array[] = []; + const sink = new WritableStream({ + write(chunk) { + chunks.push(chunk); + }, + }); + await body.writeTo(sink); + expect(chunks.length).toBe(0); + }); + + test('writeTo opens a fresh handle on each call (replayable)', async () => { + const body = fileBody(filePath); + const first: number[] = []; + const second: number[] = []; + await body.writeTo( + new WritableStream({ + write(c) { + first.push(...c); + }, + }), + ); + await body.writeTo( + new WritableStream({ + write(c) { + second.push(...c); + }, + }), + ); + expect(second).toEqual(first); + }); + + test('writeTo propagates error from stream read or write', () => { + const body = fileBody(filePath); + const sink = new WritableStream({ + write: () => { + throw new Error('sink write error'); + }, + }); + expect(body.writeTo(sink)).rejects.toThrow('sink write error'); + }); +}); diff --git a/packages/body-file/src/file-body.ts b/packages/body-file/src/file-body.ts new file mode 100644 index 0000000..273e903 --- /dev/null +++ b/packages/body-file/src/file-body.ts @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: MIT +// packages/body-file/src/file-body.ts +import {createReadStream, statSync} from 'node:fs'; +import type {FileBodyDescriptor} from '@dexpace/core'; +import {invariant} from './invariant.js'; + +/** + * Options for configuring a file-backed request body. + * + * @public + */ +export interface FileBodyOptions { + /** The starting byte offset within the file (default 0). */ + readonly start?: number; + /** The number of bytes to stream (default: remaining bytes from start to end of file). */ + readonly count?: number; +} + +/** + * Creates a file-backed request body descriptor with fail-fast construction validation (HTTP-40, BODY-11). + * + * @param path - the absolute or relative path to the regular file. + * @param options - optional byte range (start offset and count). + * @returns an immutable `FileBodyDescriptor`. + * @throws Error if the file does not exist, is not a regular file, or if the byte range is invalid. + * + * @public + */ +export function fileBody( + path: string, + options: FileBodyOptions = {}, +): FileBodyDescriptor { + const stats = statSync(path); + invariant(stats.isFile(), `not a regular file: ${path}`); + const start = options.start ?? 0; + invariant(start >= 0, `start must be non-negative, got ${String(start)}`); + invariant( + start <= stats.size, + `start (${String(start)}) exceeds file size (${String(stats.size)})`, + ); + const count = options.count ?? stats.size - start; + invariant(count >= 0, `count must be non-negative, got ${String(count)}`); + invariant( + start + count <= stats.size, + `start + count (${String(start + count)}) exceeds file size (${String(stats.size)})`, + ); + + return Object.freeze({ + kind: 'file' as const, + mediaType: undefined, + contentLength: count, + replayable: true, + path, + start, + count, + async writeTo(sink: WritableStream): Promise { + const writer = sink.getWriter(); + if (count === 0) { + writer.releaseLock(); + return; + } + let transferred = 0; + const stream = createReadStream(path, { + start, + end: start + count - 1, + }); + try { + for await (const chunk of stream) { + const bytes = chunk as Buffer; + await writer.write( + new Uint8Array(bytes.buffer, bytes.byteOffset, bytes.byteLength), + ); + transferred += bytes.byteLength; + } + invariant( + transferred === count, + `short write: transferred ${String(transferred)} of ${String(count)} bytes`, + ); + } catch (error) { + await writer.abort(error); + throw error; + } finally { + stream.destroy(); + writer.releaseLock(); + } + }, + }); +} diff --git a/packages/body-file/src/index.ts b/packages/body-file/src/index.ts new file mode 100644 index 0000000..aa1bb59 --- /dev/null +++ b/packages/body-file/src/index.ts @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: MIT +// packages/body-file/src/index.ts +export {fileBody} from './file-body.js'; +export type {FileBodyOptions} from './file-body.js'; diff --git a/packages/body-file/src/invariant.ts b/packages/body-file/src/invariant.ts new file mode 100644 index 0000000..88a16de --- /dev/null +++ b/packages/body-file/src/invariant.ts @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: MIT +// packages/body-file/src/invariant.ts + +export function invariant( + condition: boolean, + message: string, +): asserts condition { + if (!condition) throw new Error(message); +} diff --git a/packages/body-file/tsconfig.build.json b/packages/body-file/tsconfig.build.json new file mode 100644 index 0000000..d39dc7e --- /dev/null +++ b/packages/body-file/tsconfig.build.json @@ -0,0 +1,11 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "declaration": true, + "declarationMap": true, + "sourceMap": true + }, + "exclude": [ + "src/**/*.test.ts" + ] +} diff --git a/packages/body-file/tsconfig.json b/packages/body-file/tsconfig.json new file mode 100644 index 0000000..5c03021 --- /dev/null +++ b/packages/body-file/tsconfig.json @@ -0,0 +1,16 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "outDir": "./dist", + "rootDir": "./src", + "target": "ES2023", + "lib": [ + "ES2023", + "DOM", + "DOM.AsyncIterable" + ] + }, + "include": [ + "src/**/*.ts" + ] +} diff --git a/packages/core/etc/core.api.md b/packages/core/etc/core.api.md index bfee71f..e88a83e 100644 --- a/packages/core/etc/core.api.md +++ b/packages/core/etc/core.api.md @@ -116,7 +116,7 @@ export function bearerTokensEqual(a: BearerToken, b: BearerToken): boolean; // @public interface Body_2 { readonly contentLength: number; - readonly kind: 'byte-array' | 'string' | 'stream' | 'form-urlencoded' | 'multipart'; + readonly kind: 'byte-array' | 'string' | 'stream' | 'form-urlencoded' | 'multipart' | 'file'; readonly mediaType: string | undefined; readonly replayable: boolean; writeTo(sink: WritableStream): Promise; @@ -360,6 +360,18 @@ export interface FetcherPaginationInit { next: (key: string, options: PagingOptions) => Promise | undefined>; } +// @public +export interface FileBodyDescriptor extends Body_2 { + // (undocumented) + readonly count: number; + // (undocumented) + readonly kind: 'file'; + // (undocumented) + readonly path: string; + // (undocumented) + readonly start: number; +} + // @public export function foldTristate(tristate: Tristate, branches: TristateBranches): R; @@ -487,6 +499,11 @@ export interface InstrumentationBundle { readonly traceState: string; } +// @public +export class IoError extends DexpaceError { + constructor(message: string, options?: ErrorOptions); +} + // @public export function isAbsent(tristate: Tristate): tristate is { readonly [TRISTATE_BRAND]: true; @@ -1281,6 +1298,11 @@ export interface Transport { send(request: Request_2, options?: RequestOptions, signal?: AbortSignal): Promise; } +// @public +export class TransportFailureError extends IoError { + constructor(message: string, options?: ErrorOptions); +} + // @public export type Tristate = { readonly [TRISTATE_BRAND]: true; diff --git a/packages/core/src/body/body.test.ts b/packages/core/src/body/body.test.ts new file mode 100644 index 0000000..9f65ef4 --- /dev/null +++ b/packages/core/src/body/body.test.ts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/body.test.ts +// Exercises: BODY-11/TRANSPORT-28 (FileBodyDescriptor recognition contract) +import {describe, expect, test} from 'bun:test'; +import {expectTypeOf} from 'expect-type'; +import type {Body, FileBodyDescriptor} from './body.js'; + +describe('FileBodyDescriptor (BODY-11/TRANSPORT-28 recognition contract)', () => { + test('is a Body with a discriminated file kind and structural fields', () => { + expectTypeOf().toExtend(); + expectTypeOf().toEqualTypeOf<'file'>(); + expectTypeOf().toEqualTypeOf(); + expectTypeOf().toEqualTypeOf(); + expectTypeOf().toEqualTypeOf(); + }); + + test("Body['kind'] accepts 'file' without a cast", () => { + const kind: Body['kind'] = 'file'; + expect(kind).toBe('file'); + }); +}); diff --git a/packages/core/src/body/body.ts b/packages/core/src/body/body.ts index 04db52b..e2d9778 100644 --- a/packages/core/src/body/body.ts +++ b/packages/core/src/body/body.ts @@ -12,7 +12,12 @@ export interface Body { * discriminated-union-over-independent-classes pattern -- there is deliberately no base class. */ readonly kind: - 'byte-array' | 'string' | 'stream' | 'form-urlencoded' | 'multipart'; + | 'byte-array' + | 'string' + | 'stream' + | 'form-urlencoded' + | 'multipart' + | 'file'; /** * The media type to send as `Content-Type`, or `undefined` when the body declares none. * @@ -45,3 +50,17 @@ export interface Body { */ writeTo(sink: WritableStream): Promise; } + +/** + * The structural recognition contract a transport narrows on (`body.kind === 'file'`) to dispatch a + * file-specific send path (TRANSPORT-28). Type-only — `\@dexpace/core` never constructs one; the concrete + * factory lives in `\@dexpace/body-file`, which can depend on `node:fs` precisely because it is not core. + * + * @public + */ +export interface FileBodyDescriptor extends Body { + readonly kind: 'file'; + readonly path: string; + readonly start: number; + readonly count: number; +} diff --git a/packages/core/src/body/index.ts b/packages/core/src/body/index.ts index 902ce74..8d4f328 100644 --- a/packages/core/src/body/index.ts +++ b/packages/core/src/body/index.ts @@ -3,7 +3,7 @@ // Internal-facing barrel for product-spec §6. Everything except the two logging tees is also promoted to // packages/core/src/index.ts (Step 2) -- this file is the superset a future in-tree consumer (e.g. Phase // 7's pipeline) imports from directly. -export type {Body} from './body.js'; +export type {Body, FileBodyDescriptor} from './body.js'; export { ConsumedBodyError, FormBodyValidationError, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index b339c32..32a67b8 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -31,6 +31,7 @@ export { isTimeoutSignal, CancellationError, } from './seams/transport.js'; +export {IoError, TransportFailureError} from './io/errors.js'; export type {OperationDescriptor} from './seams/operation.js'; export {buildRequest, OperationAssemblyError} from './seams/operation.js'; @@ -41,7 +42,7 @@ export {buildRequest, OperationAssemblyError} from './seams/operation.js'; // `new ByteArrayBody(...)` as a field-wise constructor, which HTTP-2 forbids ("constructible only // through their builder or dedicated factory") and which duplicates the factory functions for no // stated need (NFR-3). Callers construct via the factories and annotate with the types. -export type {Body} from './body/body.js'; +export type {Body, FileBodyDescriptor} from './body/body.js'; export { ConsumedBodyError, FormBodyValidationError, diff --git a/packages/core/src/io/errors.test.ts b/packages/core/src/io/errors.test.ts index c7f16bb..bc4a453 100644 --- a/packages/core/src/io/errors.test.ts +++ b/packages/core/src/io/errors.test.ts @@ -11,6 +11,7 @@ import { IoError, isIoError, SourceContractViolationError, + TransportFailureError, } from './errors.js'; describe('IoError tree', () => { @@ -74,3 +75,17 @@ describe('IoError tree', () => { expect(isIoError(new Error('plain'))).toBe(false); }); }); + +describe('TransportFailureError (TRANSPORT-20)', () => { + test('is an IoError subtype', () => { + const error = new TransportFailureError('connect ECONNREFUSED'); + expect(error).toBeInstanceOf(IoError); + expect(error.name).toBe('TransportFailureError'); + }); + + test('carries an optional cause', () => { + const cause = new Error('ECONNREFUSED'); + const error = new TransportFailureError('connect failed', {cause}); + expect(error.cause).toBe(cause); + }); +}); diff --git a/packages/core/src/io/errors.ts b/packages/core/src/io/errors.ts index abe1557..cd84cfc 100644 --- a/packages/core/src/io/errors.ts +++ b/packages/core/src/io/errors.ts @@ -8,7 +8,7 @@ import {DexpaceError} from '../http/errors.js'; * Error messages in this tree carry counts and limits, never buffer contents — these buffers hold request * and response bodies, which routinely contain credentials and PII (styleguide 8.8). * - * @internal + * @public */ export class IoError extends DexpaceError { // bun's coverage tool never marks a bodiless subclass's implicit constructor as covered @@ -115,3 +115,17 @@ export function isIoError( error instanceof AllocationLimitError ); } + +/** + * The canonical retryable transport-failure exception (TRANSPORT-20): any send that produced no HTTP + * response — connection refused, DNS/TLS failure, peer reset, connect/read timeout. A subtype of IoError + * so 5a's `classify.ts` cause-walk already treats it as always-retryable with no change to that file. + * + * @public + */ +export class TransportFailureError extends IoError { + // eslint-disable-next-line @typescript-eslint/no-useless-constructor -- load-bearing for Bun function coverage (see IoError) + constructor(message: string, options?: ErrorOptions) { + super(message, options); + } +} diff --git a/packages/transport-conformance/package.json b/packages/transport-conformance/package.json new file mode 100644 index 0000000..c7b4f85 --- /dev/null +++ b/packages/transport-conformance/package.json @@ -0,0 +1,16 @@ +{ + "name": "@dexpace/transport-conformance", + "version": "0.0.0", + "private": true, + "type": "module", + "exports": { + ".": "./src/index.ts" + }, + "dependencies": {}, + "peerDependencies": { + "@dexpace/core": "workspace:*" + }, + "devDependencies": { + "@dexpace/core": "workspace:*" + } +} diff --git a/packages/transport-conformance/src/fixtures.ts b/packages/transport-conformance/src/fixtures.ts new file mode 100644 index 0000000..06cb45d --- /dev/null +++ b/packages/transport-conformance/src/fixtures.ts @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: MIT +// packages/transport-conformance/src/fixtures.ts +import { + createServer, + type IncomingMessage, + type Server, + type ServerResponse, +} from 'node:http'; + +/** A running fixture server, addressable by URL and shut down through {@link TestServer.close}. */ +export interface TestServer { + /** The origin every fixture path is resolved against, e.g. `http://127.0.0.1:38211`. */ + readonly url: string; + /** Stops listening and resolves once the server has released its port. */ + close(): Promise; +} + +/** How long `/slow` stalls before answering -- long enough that no timeout under test wins the race by luck. */ +const SLOW_RESPONSE_MS = 5_000; +/** `/drip`'s inter-chunk gap: long enough that a close-without-read happens mid-body, short enough not to pace the suite. */ +const DRIP_INTERVAL_MS = 50; +const DRIP_CHUNKS = 20; + +function route( + pathname: string, + req: IncomingMessage, + res: ServerResponse, +): void { + switch (pathname) { + case '/echo-headers': + res.writeHead(200, {'content-type': 'application/json'}); + res.end(JSON.stringify(req.headers)); + return; + case '/echo-body': { + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer) => chunks.push(chunk)); + req.on('end', () => { + res.writeHead(200, {'content-type': 'application/octet-stream'}); + res.end(Buffer.concat(chunks)); + }); + return; + } + case '/early-response': + // Answers without ever draining the request body, so a streaming producer is still running + // when the response is delivered -- the window TRANSPORT-19's post-delivery clause lives in. + res.writeHead(413, {'content-type': 'text/plain'}); + res.end('too large'); + return; + case '/vendor-status': + res.writeHead(520, {'content-type': 'text/plain'}); + res.end('vendor status body'); + return; + case '/malformed-content-type': + // TRANSPORT-27: a syntactically invalid media type and a chunked (length-less) body. + res.writeHead(200, { + 'content-type': 'not-a-media-type', + 'transfer-encoding': 'chunked', + }); + res.end('body'); + return; + case '/drip': { + // Headers land immediately, the body trickles: the shape a lazily-streamed response body and an + // orphaned-response cleanup both need (TRANSPORT-9, TRANSPORT-25, SEAM-30). + res.writeHead(200, {'content-type': 'application/octet-stream'}); + let sent = 0; + const timer = setInterval(() => { + sent += 1; + if (sent >= DRIP_CHUNKS) { + clearInterval(timer); + res.end('end'); + return; + } + res.write(`chunk-${String(sent)};`); + }, DRIP_INTERVAL_MS); + res.on('close', () => { + clearInterval(timer); + }); + return; + } + case '/slow': + // Nothing is written at all, so a request against it is still awaiting response headers when + // the timeout or abort under test fires. + setTimeout(() => { + res.writeHead(200); + res.end('done'); + }, SLOW_RESPONSE_MS).unref(); + return; + case '/redirect': + res.writeHead(302, {location: '/echo-headers'}); + res.end(); + return; + default: + res.writeHead(200, {'content-length': '0'}); + res.end(); + } +} + +/** + * Starts a local `node:http` server exposing the fixed set of endpoints every `TRANSPORT-N` assertion + * needs, on an ephemeral port so parallel test files never collide. + * + * @returns the listening server; the caller closes it in its own `afterAll`. + */ +export function startFixtureServer(): Promise { + return new Promise(resolve => { + const server: Server = createServer((req, res) => { + route(new URL(req.url ?? '/', 'http://localhost').pathname, req, res); + }); + server.listen(0, '127.0.0.1', () => { + const address = server.address(); + const port = + typeof address === 'object' && address !== null ? address.port : 0; + resolve({ + url: `http://127.0.0.1:${String(port)}`, + close: () => + new Promise(done => { + // closeAllConnections, not close alone: a keep-alive socket a transport still holds open + // would otherwise stall this for the server's whole idle timeout. + server.closeAllConnections(); + server.close(() => { + done(); + }); + }), + }); + }); + }); +} diff --git a/packages/transport-conformance/src/index.ts b/packages/transport-conformance/src/index.ts new file mode 100644 index 0000000..c5841b7 --- /dev/null +++ b/packages/transport-conformance/src/index.ts @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: MIT +// packages/transport-conformance/src/index.ts +export { + runTransportConformanceSuite, + type TransportCapabilities, +} from './run-suite.js'; +export {startFixtureServer, type TestServer} from './fixtures.js'; diff --git a/packages/transport-conformance/src/run-suite.ts b/packages/transport-conformance/src/run-suite.ts new file mode 100644 index 0000000..57833b6 --- /dev/null +++ b/packages/transport-conformance/src/run-suite.ts @@ -0,0 +1,624 @@ +// SPDX-License-Identifier: MIT +// packages/transport-conformance/src/run-suite.ts +// The single TRANSPORT-N conformance suite, run once per transport package so the two adapters cannot +// drift. Exercises: TRANSPORT-1..9, TRANSPORT-15..17, TRANSPORT-20..27, TRANSPORT-29, SEAM-12, +// SEAM-16, SEAM-30, NFR-15. TRANSPORT-10..14 are asserted at their source in @dexpace/transport-shared; +// TRANSPORT-18/28's collapses are Deviation Ledger rows; TRANSPORT-30's full flow is +// transport-undici's challenge-handler.test.ts. +import {afterAll, beforeAll, describe, expect, test} from 'bun:test'; +import { + getBuildInfo, + getGlobalLogger, + Headers, + Request, + RequestOptions, + setGlobalLogger, + type Body, + type Logger, + type Transport, +} from '@dexpace/core'; +import {startFixtureServer, type TestServer} from './fixtures.js'; + +/** + * The clauses `docs/product-spec/17-transport-adapter-conformance-contract.md` scopes to only one + * reference transport, plus the one drop-set entry that legitimately differs between the two. + */ +export interface TransportCapabilities { + /** TRANSPORT-8: the transport has an internal-cancel path distinct from a caller abort. */ + readonly supportsInternalCancel: boolean; + /** + * TRANSPORT-30: the transport can be configured with a proxy at all. The proxy behaviour itself is + * asserted in `transport-undici`'s own tests, because only that package can construct one. + */ + readonly supportsProxy: boolean; + /** TRANSPORT-11: whether `Connection` is in this transport's outbound drop set. */ + readonly dropsConnectionHeader: boolean; +} + +/** What every row below needs: a transport factory, the live fixture origin, and the capability flags. */ +interface SuiteContext { + readonly makeTransport: () => Transport; + readonly capabilities: TransportCapabilities; + /** Resolves a fixture path against the server started in `beforeAll`; read lazily, at run time. */ + url(path: string): string; +} + +/** + * Awaits `pending` and hands back its rejection reason. + * + * Deliberately not `expect(pending).rejects.…`: that form is typed `void` here, so a row that has to + * assert something *after* the rejection (a `close()` that must not stall, say) would race its own + * assertion. This settles first, then asserts. + */ +/** How long the post-delivery producer stalls before failing; long enough to outlive `send`. */ +const POST_DELIVERY_MS = 150; + +async function rejection(pending: Promise): Promise { + try { + await pending; + } catch (error) { + return error; + } + throw new Error('expected the send to reject, but it resolved'); +} + +/** Creates a transport, runs `body` against it, and closes it on every exit path. */ +async function withTransport( + make: () => Transport, + body: (transport: Transport) => Promise, +): Promise { + const transport = make(); + try { + return await body(transport); + } finally { + await transport.close(); + } +} + +/** + * Runs `body` with a capturing global logger installed and returns every `header` field the + * drop log emitted, lower-cased. Restores the previous logger on every exit path. + */ +async function captureDroppedHeaders( + body: () => Promise, +): Promise { + const dropped: string[] = []; + const previous: Logger = getGlobalLogger(); + const capturing: Logger = { + atLevel: () => { + let name: string | undefined; + const entry = { + field: (key: string, value: unknown) => { + if (key === 'header') name = String(value); + return entry; + }, + event: () => entry, + cause: () => entry, + emit: () => { + if (name !== undefined) dropped.push(name.toLowerCase()); + }, + }; + return entry; + }, + withContext: () => capturing, + }; + setGlobalLogger(capturing); + try { + await body(); + } finally { + setGlobalLogger(previous); + } + return dropped; +} + +async function readEchoedHeaders( + transport: Transport, + request: Request, +): Promise> { + const response = await transport.send(request); + return JSON.parse(await response.text()) as Record; +} + +function registerDispatchRows(ctx: SuiteContext): void { + describe('TRANSPORT-1/2/21/23: dispatch, pipeline authority, null-safety', () => { + test('a 302 is returned raw, never followed by the native client', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder().url(ctx.url('/redirect')).build(); + const response = await transport.send(request); + expect(response.status.code).toBe(302); + expect(response.headers.get('location')).toBe('/echo-headers'); + await response.close(); + }); + }); + + test('a failure is delivered through the promise, never a synchronous throw', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder().url('http://127.0.0.1:1').build(); + // Reaching the next line at all is the assertion: a synchronous throw would abort the test + // here rather than surface through the promise (TRANSPORT-21). + const pending = transport.send(request); + expect(pending).toBeInstanceOf(Promise); + expect(await rejection(pending)).toBeDefined(); + }); + }); + + test('a success never resolves to a null or undefined response', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder() + .url(ctx.url('/echo-headers')) + .build(); + const response = await transport.send(request); + expect(response).toBeDefined(); + expect(response.request.url.href).toBe(ctx.url('/echo-headers')); + await response.close(); + }); + }); + }); +} + +function registerStatusRows(ctx: SuiteContext): void { + describe('TRANSPORT-24/26/27: status fidelity and inbound downgrades', () => { + test('a vendor 520 is surfaced faithfully with a readable body', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder() + .url(ctx.url('/vendor-status')) + .build(); + const response = await transport.send(request); + expect(response.status.code).toBe(520); + expect(await response.text()).toBe('vendor status body'); + }); + }); + + test('a body-less POST dispatches with a zero-length body, not a throw', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder() + .method('POST') + .url(ctx.url('/echo-headers')) + .build(); + const echoed = await readEchoedHeaders(transport, request); + // TRANSPORT-26: the zero-length substitution is observable as the framing the client + // computed, not as a rejected send. + expect(echoed['content-length']).toBe('0'); + }); + }); + + test('an unparseable Content-Type downgrades the response rather than failing it', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder() + .url(ctx.url('/malformed-content-type')) + .build(); + const response = await transport.send(request); + expect(response.status.code).toBe(200); + expect(response.headers.get('content-type')).toBe('not-a-media-type'); + expect(await response.text()).toBe('body'); + }); + }); + }); +} + +function registerBodyRows(ctx: SuiteContext): void { + describe('TRANSPORT-17/19/25: request bodies written once, response bodies streamed lazily', () => { + test('a single-use body is written exactly once and its bytes reach the wire', async () => { + await withTransport(ctx.makeTransport, async transport => { + let writeCount = 0; + const payload = new TextEncoder().encode('payload'); + // Built from scratch rather than monkey-patching stringBody: every core model is frozen + // (HTTP-1), and a replayable body would not exercise the single-use path at all. + const body: Body = { + kind: 'stream', + mediaType: 'text/plain', + contentLength: payload.byteLength, + replayable: false, + async writeTo(sink) { + writeCount += 1; + const writer = sink.getWriter(); + await writer.write(payload); + await writer.close(); + }, + }; + const request = Request.newBuilder() + .method('POST') + .url(ctx.url('/echo-body')) + .body(body) + .build(); + const response = await transport.send(request); + expect(await response.text()).toBe('payload'); + expect(writeCount).toBe(1); + }); + }); + + test('the response body streams on demand rather than arriving pre-buffered', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder().url(ctx.url('/drip')).build(); + const response = await transport.send(request); + const stream = response.body; + if (stream === null) throw new Error('the response carried no body'); + expect(stream).toBeInstanceOf(ReadableStream); + const reader = stream.getReader(); + const first = await reader.read(); + // The fixture drips for ~1s; a first chunk in hand while the stream is still open is the + // observable form of "not pre-buffered" (SEAM-11, TRANSPORT-25). + expect(first.done).toBe(false); + reader.releaseLock(); + await response.close(); + }); + }); + + test('closing without reading releases the connection, idempotently', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder().url(ctx.url('/drip')).build(); + const response = await transport.send(request); + await response.close(); + // Reaching the next line proves close() is idempotent: a second close that threw or hung + // would fail or time out the row (BODY-15). + await response.close(); + }); + }); + }); +} + +function registerProducerRows(ctx: SuiteContext): void { + describe('TRANSPORT-19: an abandoned or failed request-body producer', () => { + test('a producer that fails after delivery does not escape as an unhandled rejection', async () => { + await withTransport(ctx.makeTransport, async transport => { + // The fixture answers 413 without draining, so `send` resolves while `writeTo` is still + // parked. The producer then fails with nobody left awaiting it -- and a transport that does + // not keep a handler on the producer's settlement lets that rejection reach the runtime's + // default `unhandledRejection` policy, which terminates the process (TRANSPORT-19, SEAM-30). + // Both `bun test` and `node --test` fail a test that leaks one, so this row needs no + // process-level listener of its own to be the assertion. + const body: Body = { + kind: 'stream', + mediaType: 'application/octet-stream', + contentLength: -1, + replayable: false, + async writeTo(sink) { + const writer = sink.getWriter(); + await writer.write(new Uint8Array(1024)); + await new Promise(resolve => setTimeout(resolve, POST_DELIVERY_MS)); + throw new Error('producer failed after the response was delivered'); + }, + }; + const request = Request.newBuilder() + .method('POST') + .url(ctx.url('/early-response')) + .body(body) + .build(); + const response = await transport.send(request); + expect(response.status.code).toBe(413); + await response.close(); + // Outlives the producer, so the rejection has actually happened by the time the row ends. + await new Promise(resolve => setTimeout(resolve, POST_DELIVERY_MS * 3)); + }); + }); + }); +} + +function registerFailureRows(ctx: SuiteContext): void { + describe('TRANSPORT-4/5/6/20/22: failure classification and socket release', () => { + test('a dead port surfaces the retryable TransportFailureError', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder().url('http://127.0.0.1:1').build(); + expect(await rejection(transport.send(request))).toMatchObject({ + name: 'TransportFailureError', + }); + }); + }); + + test('a per-call timeout is retryable, not a cancellation', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder().url(ctx.url('/slow')).build(); + const options = RequestOptions.newBuilder().timeoutMs(50).build(); + expect(await rejection(transport.send(request, options))).toMatchObject( + {name: 'TransportFailureError'}, + ); + }); + }); + + test('two concurrent calls are each bounded by their own timeout', async () => { + await withTransport(ctx.makeTransport, async transport => { + const slow = (): Request => + Request.newBuilder().url(ctx.url('/slow')).build(); + const started = Date.now(); + // TRANSPORT-5: the per-call override applies to that call only, and neither call waits on + // the other. Both are awaited, so the transport closes with nothing still in flight. + const brief = rejection( + transport.send( + slow(), + RequestOptions.newBuilder().timeoutMs(60).build(), + ), + ); + const patient = rejection( + transport.send( + slow(), + RequestOptions.newBuilder().timeoutMs(1_200).build(), + ), + ); + expect(await brief).toMatchObject({name: 'TransportFailureError'}); + // The short call cannot have been extended to the long call's deadline. + expect(Date.now() - started).toBeLessThan(1_000); + expect(await patient).toMatchObject({name: 'TransportFailureError'}); + }); + }); + + test('a sub-resolution 1ms timeout still times out rather than hanging', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder().url(ctx.url('/slow')).build(); + const options = RequestOptions.newBuilder().timeoutMs(1).build(); + expect(await rejection(transport.send(request, options))).toMatchObject( + {name: 'TransportFailureError'}, + ); + }); + }); + }); +} + +function registerCancellationRows(ctx: SuiteContext): void { + describe('TRANSPORT-3/7/9: cancellation is terminal, and orphans are released', () => { + test('aborting mid-request yields a terminal CancellationError', async () => { + await withTransport(ctx.makeTransport, async transport => { + const controller = new AbortController(); + const request = Request.newBuilder().url(ctx.url('/slow')).build(); + const pending = transport.send(request, undefined, controller.signal); + setTimeout(() => { + controller.abort(); + }, 20); + expect(await rejection(pending)).toMatchObject({ + name: 'CancellationError', + }); + }); + }); + + test('a cancelled exchange leaves no handle that stalls close()', async () => { + const transport = ctx.makeTransport(); + const controller = new AbortController(); + const request = Request.newBuilder().url(ctx.url('/slow')).build(); + const pending = transport.send(request, undefined, controller.signal); + setTimeout(() => { + controller.abort(); + }, 20); + expect(await rejection(pending)).toBeDefined(); + // A dangling handle would stall this close() until the row times out. + await transport.close(); + }); + + test('an abort after the response was delivered does not close its body (SEAM-16)', async () => { + await withTransport(ctx.makeTransport, async transport => { + const controller = new AbortController(); + const request = Request.newBuilder() + .url(ctx.url('/vendor-status')) + .build(); + const response = await transport.send( + request, + undefined, + controller.signal, + ); + controller.abort(); + // The caller owns the delivered body even when the signal fires afterwards; a transport that + // wired an unconditional abort listener would truncate this read. + expect(await response.text()).toBe('vendor status body'); + }); + }); + + test('a timeout while headers are still pending releases the connection (SEAM-30)', async () => { + const transport = ctx.makeTransport(); + const request = Request.newBuilder().url(ctx.url('/slow')).build(); + const options = RequestOptions.newBuilder().timeoutMs(50).build(); + expect(await rejection(transport.send(request, options))).toMatchObject({ + name: 'TransportFailureError', + }); + await transport.close(); + }); + }); +} + +function registerLifecycleRows(ctx: SuiteContext): void { + describe('TRANSPORT-15/16/29, SEAM-12: lifecycle and concurrency', () => { + test('close is idempotent', async () => { + const transport = ctx.makeTransport(); + await transport.close(); + // A second close that threw or hung would fail or time out the row (TRANSPORT-16). + await transport.close(); + }); + + test('many concurrent sends each resolve to their own response', async () => { + await withTransport(ctx.makeTransport, async transport => { + const responses = await Promise.all( + Array.from({length: 20}, (_unused, index) => + transport.send( + Request.newBuilder() + .url(ctx.url('/echo-headers')) + .headers( + Headers.newBuilder().set('X-Call', String(index)).build(), + ) + .build(), + ), + ), + ); + const seen = await Promise.all( + responses.map(async response => { + const echoed = JSON.parse(await response.text()) as Record< + string, + string + >; + return echoed['x-call']; + }), + ); + // Per-request state confined to the promise graph: 20 distinct values, no interleaving. + expect(new Set(seen).size).toBe(20); + }); + }); + }); +} + +function registerHeaderRows(ctx: SuiteContext): void { + describe('TRANSPORT-10/11, NFR-15: the outbound header pass', () => { + test('a caller-supplied Content-Length never reaches the wire (framing is the client’s)', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder() + .method('POST') + .url(ctx.url('/echo-headers')) + .headers(Headers.newBuilder().set('Content-Length', '999').build()) + .body({ + kind: 'byte-array', + mediaType: 'text/plain', + contentLength: 5, + replayable: true, + async writeTo(sink) { + const writer = sink.getWriter(); + await writer.write(new TextEncoder().encode('hello')); + await writer.close(); + }, + }) + .build(); + const echoed = await readEchoedHeaders(transport, request); + expect(echoed['content-length']).not.toBe('999'); + }); + }); + + test('a body-derived Content-Type is stamped when the caller set none', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder() + .method('POST') + .url(ctx.url('/echo-headers')) + .body({ + kind: 'byte-array', + mediaType: 'application/x-conformance', + contentLength: 2, + replayable: true, + async writeTo(sink) { + const writer = sink.getWriter(); + await writer.write(new Uint8Array([1, 2])); + await writer.close(); + }, + }) + .build(); + const echoed = await readEchoedHeaders(transport, request); + expect(echoed['content-type']).toBe('application/x-conformance'); + }); + }); + + test('a stamped User-Agent survives the drop pass unmangled', async () => { + await withTransport(ctx.makeTransport, async transport => { + const identity = getBuildInfo().identityTokens.join(' '); + const request = Request.newBuilder() + .url(ctx.url('/echo-headers')) + .headers(Headers.newBuilder().set('User-Agent', identity).build()) + .build(); + const echoed = await readEchoedHeaders(transport, request); + expect(echoed['user-agent']).toBe(identity); + }); + }); + }); +} + +function registerDropSetRows(ctx: SuiteContext): void { + describe('TRANSPORT-11/13: the transport-specific drop set', () => { + test('the Connection header follows this transport’s documented drop set', async () => { + // Asserted through the drop log, not the echoed request: both clients set a `Connection` + // header of their own for connection management, so the wire cannot tell a forwarded + // caller header from the client's own. The log is where the decision is observable + // (TRANSPORT-11 with TRANSPORT-13). + const dropped = await captureDroppedHeaders(async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder() + .url(ctx.url('/echo-headers')) + .headers( + Headers.newBuilder().set('Connection', 'keep-alive').build(), + ) + .build(); + const response = await transport.send(request); + await response.close(); + }); + }); + expect(dropped.includes('connection')).toBe( + ctx.capabilities.dropsConnectionHeader, + ); + }); + }); +} + +function registerScopedRows(ctx: SuiteContext): void { + if (ctx.capabilities.supportsInternalCancel) { + describe('TRANSPORT-8: an internal cancel is told apart from a timeout', () => { + test('the same slow endpoint yields a terminal cancel and a retryable timeout', async () => { + await withTransport(ctx.makeTransport, async transport => { + const request = Request.newBuilder().url(ctx.url('/slow')).build(); + const controller = new AbortController(); + const cancelled = transport.send( + request, + undefined, + controller.signal, + ); + controller.abort(); + expect(await rejection(cancelled)).toMatchObject({ + name: 'CancellationError', + }); + const timedOut = transport.send( + Request.newBuilder().url(ctx.url('/slow')).build(), + RequestOptions.newBuilder().timeoutMs(30).build(), + ); + expect(await rejection(timedOut)).toMatchObject({ + name: 'TransportFailureError', + }); + }); + }); + }); + } + + if (ctx.capabilities.supportsProxy) { + describe('TRANSPORT-30: proxy-capable, but only when asked', () => { + test('an unconfigured proxy-capable transport still routes normally', async () => { + // §17's own conformance line for TRANSPORT-30 ("assert normal requests still route"). + // The regression it guards is a transport that installs a proxy dispatcher unconditionally + // and tunnels every request through nothing. + await withTransport(ctx.makeTransport, async transport => { + const response = await transport.send( + Request.newBuilder().url(ctx.url('/echo-headers')).build(), + ); + expect(response.status.code).toBe(200); + await response.close(); + }); + }); + }); + } +} + +/** + * Registers the whole `TRANSPORT-N` conformance suite against one transport factory. + * + * @param name - the transport's name, used as the outer `describe` label. + * @param makeTransport - builds a fresh transport; called once per row and closed by the suite. + * @param capabilities - the clauses §17 scopes to a subset of transports. + */ +export function runTransportConformanceSuite( + name: string, + makeTransport: () => Transport, + capabilities: TransportCapabilities, +): void { + describe(`${name} conformance (TRANSPORT-1..30, SEAM-12/16/30, NFR-15)`, () => { + let server: TestServer; + beforeAll(async () => { + server = await startFixtureServer(); + }); + afterAll(async () => { + await server.close(); + }); + + const ctx: SuiteContext = { + makeTransport, + capabilities, + url: path => `${server.url}${path}`, + }; + registerDispatchRows(ctx); + registerStatusRows(ctx); + registerBodyRows(ctx); + registerProducerRows(ctx); + registerFailureRows(ctx); + registerCancellationRows(ctx); + registerLifecycleRows(ctx); + registerHeaderRows(ctx); + registerDropSetRows(ctx); + registerScopedRows(ctx); + }); +} diff --git a/packages/transport-conformance/tsconfig.json b/packages/transport-conformance/tsconfig.json new file mode 100644 index 0000000..42c3719 --- /dev/null +++ b/packages/transport-conformance/tsconfig.json @@ -0,0 +1,15 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "rootDir": "./src", + "target": "ES2023", + "lib": [ + "ES2023", + "DOM", + "DOM.AsyncIterable" + ] + }, + "include": [ + "src/**/*.ts" + ] +} diff --git a/packages/transport-fetch/README.md b/packages/transport-fetch/README.md new file mode 100644 index 0000000..234b944 --- /dev/null +++ b/packages/transport-fetch/README.md @@ -0,0 +1,57 @@ +# @dexpace/transport-fetch + +The zero-dependency `Transport` for the dexpace SDK, built on the runtime's own global `fetch`. +Nothing beyond a `@dexpace/core` peer is installed. + +```sh +bun add @dexpace/transport-fetch @dexpace/core +``` + +```typescript +import {Request} from '@dexpace/core'; +import {fetchTransport} from '@dexpace/transport-fetch'; + +await using transport = fetchTransport({headerDropLogging: 'first-per-name'}); + +const response = await transport.send( + Request.newBuilder().url('https://example.com/v1/users').build(), +); +try { + console.log(response.status.code, await response.text()); +} finally { + await response.close(); // the caller owns the body, always (BODY-15) +} +``` + +## What this transport deliberately does not do + +- **No proxy support, at all (`TRANSPORT-30`, scoped out).** There is no `proxy` option on + `FetchTransportOptions` — an absent option, not a silently ignored one — so a caller reaching for + proxying is type-directed to `@dexpace/transport-undici` rather than discovering the gap at + runtime. Node's bare global `fetch` exposes no proxy hook that does not route through `undici` + internals, and depending on `undici` would undo this package's entire reason to exist. +- **No native-internal cancel path (`TRANSPORT-8`, scoped out).** `fetch` has no teardown distinct + from an `AbortSignal` abort, so there is no second failure mode to tell apart from a timeout. +- **No connection pool to release.** `close()` is a sanctioned no-op over a runtime global this + package does not own, and `send()` keeps working after it — this transport's documented `SEAM-15` + post-close mode. `@dexpace/transport-undici` is the one with real close semantics. +- **`Response.protocol` is always `HTTP_1_1`.** A documented best-effort default: the WHATWG + `Response` object exposes no negotiated-HTTP-version field to read. Recorded in the Deviation + Ledger, not silently papered over. + +## Behavior worth knowing + +- Redirects are **never** followed (`redirect: 'manual'`). The SDK pipeline is the redirect + authority (`TRANSPORT-1`/`TRANSPORT-2`). +- `Content-Length`, `Host`, `Transfer-Encoding`, and `Connection` are dropped outbound — the client + computes its own framing — and every drop is logged by name (never by value) through the global + logger, deduped per name by default (`TRANSPORT-11`/`TRANSPORT-13`). +- An abort that fires **after** `send()` resolved does not close the delivered body: the caller owns + it (`SEAM-16`). Cancellation stays live for the whole in-flight window. +- A timeout surfaces as the retryable `TransportFailureError`; a caller abort as the terminal + `CancellationError` (`TRANSPORT-3`/`TRANSPORT-4`). A raw `DOMException` is never surfaced. + +## Conformance + +Proven against the shared `TRANSPORT-N` suite in `@dexpace/transport-conformance`, the same one +`@dexpace/transport-undici` runs, so the two adapters cannot drift. diff --git a/packages/transport-fetch/api-extractor.json b/packages/transport-fetch/api-extractor.json new file mode 100644 index 0000000..455423c --- /dev/null +++ b/packages/transport-fetch/api-extractor.json @@ -0,0 +1,22 @@ +{ + "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json", + "mainEntryPointFilePath": "/dist/index.d.ts", + "apiReport": { + "enabled": true, + "reportFolder": "/etc/" + }, + "docModel": { + "enabled": false + }, + "dtsRollup": { + "enabled": false + }, + "messages": { + "extractorMessageReporting": { + "ae-forgotten-export": { + "logLevel": "error", + "addToApiReportFile": false + } + } + } +} diff --git a/packages/transport-fetch/etc/transport-fetch.api.md b/packages/transport-fetch/etc/transport-fetch.api.md new file mode 100644 index 0000000..072dfe8 --- /dev/null +++ b/packages/transport-fetch/etc/transport-fetch.api.md @@ -0,0 +1,27 @@ +## API Report File for "@dexpace/transport-fetch" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts + +import { HeaderDropLogging } from '@dexpace/transport-shared'; +import { Transport } from '@dexpace/core'; + +// @public +export type FetchLike = (input: string, init: RequestInit & { + duplex?: 'half'; +}) => Promise; + +// @public +export function fetchTransport(options?: FetchTransportOptions): Transport & AsyncDisposable; + +// @public +export interface FetchTransportOptions { + readonly defaultTimeoutMs?: number; + readonly fetch?: FetchLike; + readonly headerDropLogging?: HeaderDropLogging; +} + +// (No @packageDocumentation comment for this package) + +``` diff --git a/packages/transport-fetch/package.json b/packages/transport-fetch/package.json new file mode 100644 index 0000000..b5c04ba --- /dev/null +++ b/packages/transport-fetch/package.json @@ -0,0 +1,49 @@ +{ + "name": "@dexpace/transport-fetch", + "version": "0.0.0", + "description": "Fetch-based transport adapter for the dexpace Node.js SDK.", + "license": "MIT", + "type": "module", + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "engines": { + "node": ">=20.3" + }, + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + } + }, + "files": [ + "dist" + ], + "sideEffects": false, + "dependencies": { + "@dexpace/transport-shared": "workspace:*" + }, + "peerDependencies": { + "@dexpace/core": "workspace:*" + }, + "peerDependenciesMeta": { + "@dexpace/core": { + "optional": false + } + }, + "devDependencies": { + "@dexpace/core": "workspace:*", + "@dexpace/transport-conformance": "workspace:*", + "@microsoft/api-extractor": "catalog:", + "expect-type": "catalog:", + "fast-check": "catalog:", + "typescript": "catalog:" + }, + "scripts": { + "build": "tsc -p tsconfig.build.json", + "typecheck": "tsc --noEmit", + "test": "bun test", + "api:local": "api-extractor run --local", + "api:ci": "api-extractor run", + "prepublishOnly": "bun run build && bun run api:ci && publint . && attw --pack . --ignore-rules cjs-resolves-to-esm" + } +} diff --git a/packages/transport-fetch/src/fetch-transport.conformance.test.ts b/packages/transport-fetch/src/fetch-transport.conformance.test.ts new file mode 100644 index 0000000..4ab7aa1 --- /dev/null +++ b/packages/transport-fetch/src/fetch-transport.conformance.test.ts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: MIT +// packages/transport-fetch/src/fetch-transport.conformance.test.ts +// Runs the shared TRANSPORT-N suite (@dexpace/transport-conformance) against fetchTransport(). +import {runTransportConformanceSuite} from '@dexpace/transport-conformance'; +import {fetchTransport} from './fetch-transport.js'; + +runTransportConformanceSuite('fetchTransport', () => fetchTransport(), { + // TRANSPORT-8 scoped out: the global fetch has no internal-cancel path distinct from an abort. + supportsInternalCancel: false, + // TRANSPORT-30 scoped out: proxying would mean depending on undici internals (design doc s6). + supportsProxy: false, + // TRANSPORT-11: `Connection` is a WHATWG forbidden request header, so fetch drops it either way. + dropsConnectionHeader: true, +}); diff --git a/packages/transport-fetch/src/fetch-transport.test.ts b/packages/transport-fetch/src/fetch-transport.test.ts new file mode 100644 index 0000000..97fd065 --- /dev/null +++ b/packages/transport-fetch/src/fetch-transport.test.ts @@ -0,0 +1,243 @@ +// SPDX-License-Identifier: MIT +// packages/transport-fetch/src/fetch-transport.test.ts +// Exercises: TRANSPORT-2 (no retrying/redirecting dispatcher is ever composed), TRANSPORT-15/16 +// (close is a documented no-op), TRANSPORT-17/19 (single-use body written once, abandoned producer +// unblocked), TRANSPORT-22 (an adaptation throw still closes the native response), TRANSPORT-30 +// (no proxy option exists at all) +import {describe, expect, test} from 'bun:test'; +import { + byteArrayBody, + Headers, + Request, + streamBody, + type Body, +} from '@dexpace/core'; +import {fetchTransport} from './fetch-transport.js'; + +/** + * Awaits `pending` and hands back its rejection reason. `expect(p).rejects.…` is typed `void` here, + * so this keeps the assertion ordered with whatever the row checks afterwards. + */ +async function rejection(pending: Promise): Promise { + try { + await pending; + } catch (error) { + return error; + } + throw new Error('expected the promise to reject, but it resolved'); +} + +/** A `fetch` double recording the `RequestInit` it was handed, answering a fixed 200. */ +type RecordedInit = RequestInit & {duplex?: 'half'}; + +function recordingFetch(): { + fetch: (input: string, init: RecordedInit) => Promise; + calls: RecordedInit[]; +} { + const calls: RecordedInit[] = []; + return { + calls, + fetch: (_input, init) => { + calls.push(init); + return Promise.resolve(new globalThis.Response('ok', {status: 200})); + }, + }; +} + +describe('fetchTransport dispatch', () => { + test('TRANSPORT-1/2: redirects are never followed by the native client', async () => { + const recorder = recordingFetch(); + const transport = fetchTransport({fetch: recorder.fetch}); + const request = Request.newBuilder() + .url('http://127.0.0.1:1/anything') + .build(); + await (await transport.send(request)).close(); + expect(recorder.calls[0]?.redirect).toBe('manual'); + }); + + test('TRANSPORT-11: the framing headers the client computes are dropped', async () => { + const recorder = recordingFetch(); + const transport = fetchTransport({fetch: recorder.fetch}); + const request = Request.newBuilder() + .url('http://127.0.0.1:1/anything') + .headers( + Headers.newBuilder() + .set('Content-Length', '999') + .set('Connection', 'keep-alive') + .set('X-Kept', 'yes') + .build(), + ) + .build(); + await (await transport.send(request)).close(); + const sent = recorder.calls[0]?.headers as globalThis.Headers; + expect(sent.get('content-length')).toBeNull(); + expect(sent.get('connection')).toBeNull(); + expect(sent.get('x-kept')).toBe('yes'); + }); + + test('a small replayable body is materialized rather than streamed', async () => { + const recorder = recordingFetch(); + const transport = fetchTransport({fetch: recorder.fetch}); + const request = Request.newBuilder() + .method('POST') + .url('http://127.0.0.1:1/anything') + .body( + byteArrayBody(new Uint8Array([1, 2, 3]), 'application/octet-stream'), + ) + .build(); + await (await transport.send(request)).close(); + expect(recorder.calls[0]?.body).toBeInstanceOf(Uint8Array); + expect(recorder.calls[0]?.duplex).toBeUndefined(); + }); + + test('TRANSPORT-17: a single-use body is streamed with duplex declared', async () => { + const recorder = recordingFetch(); + const transport = fetchTransport({fetch: recorder.fetch}); + const source = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array([7])); + controller.close(); + }, + }); + const request = Request.newBuilder() + .method('POST') + .url('http://127.0.0.1:1/anything') + .body(streamBody(source)) + .build(); + await (await transport.send(request)).close(); + expect(recorder.calls[0]?.body).toBeInstanceOf(ReadableStream); + expect(recorder.calls[0]?.duplex).toBe('half'); + }); +}); + +describe('fetchTransport failure paths', () => { + test('TRANSPORT-22: an adaptation throw cancels the native body before propagating', async () => { + let cancelled = false; + const body = new ReadableStream({ + cancel() { + cancelled = true; + }, + }); + // A deliberately hostile Response: the only way to make adaptation fail, since every value a + // conforming one carries is either total (Status.of) or degraded rather than rejected. + const hostile = { + status: 200, + statusText: 'OK', + body, + headers: { + forEach: () => { + throw new Error('adaptation exploded'); + }, + getSetCookie: () => [], + }, + } as unknown as globalThis.Response; + + const transport = fetchTransport({fetch: () => Promise.resolve(hostile)}); + const request = Request.newBuilder().url('http://127.0.0.1:1/x').build(); + expect(await rejection(transport.send(request))).toMatchObject({ + message: 'adaptation exploded', + }); + expect(cancelled).toBe(true); + }); + + test('TRANSPORT-19/20: a producer failure fails the send and unwinds the producer', async () => { + const failing: Body = { + kind: 'stream', + mediaType: undefined, + contentLength: -1, + replayable: false, + writeTo() { + return Promise.reject(new Error('producer exploded')); + }, + }; + // A fetch that never settles, so the only way this send can finish is the producer's failure + // winning the race -- the regression this guards is sequencing the two instead of racing them. + const transport = fetchTransport({ + fetch: () => new Promise(() => undefined), + }); + const request = Request.newBuilder() + .method('POST') + .url('http://127.0.0.1:1/x') + .body(failing) + .build(); + expect(await rejection(transport.send(request))).toMatchObject({ + name: 'TransportFailureError', + }); + }); +}); + +describe('fetchTransport request-body failures', () => { + test('a buffered body that cannot be written fails the send the same way', async () => { + const recorder = recordingFetch(); + const transport = fetchTransport({fetch: recorder.fetch}); + const request = Request.newBuilder() + .method('POST') + .url('http://127.0.0.1:1/x') + .body({ + kind: 'byte-array', + mediaType: 'text/plain', + contentLength: 3, + replayable: true, + writeTo: () => Promise.reject(new Error('body exploded')), + }) + .build(); + // The materialized branch classifies a body failure exactly as the streaming branch does. + expect(await rejection(transport.send(request))).toMatchObject({ + name: 'TransportFailureError', + cause: {message: 'body exploded'}, + }); + expect(recorder.calls.length).toBe(0); + }); + + test('a network failure is wrapped as TransportFailureError with its cause kept', async () => { + const cause = new Error('connect ECONNREFUSED'); + const transport = fetchTransport({fetch: () => Promise.reject(cause)}); + const request = Request.newBuilder().url('http://127.0.0.1:1/x').build(); + expect(await rejection(transport.send(request))).toMatchObject({ + name: 'TransportFailureError', + cause, + }); + }); +}); + +describe('fetchTransport lifecycle', () => { + test('TRANSPORT-15/16: close is a no-op and send still works afterwards (SEAM-15)', async () => { + const recorder = recordingFetch(); + const transport = fetchTransport({fetch: recorder.fetch}); + await transport.close(); + // Reaching the next line proves the second close neither threw nor hung (TRANSPORT-16). + await transport.close(); + const request = Request.newBuilder().url('http://127.0.0.1:1/x').build(); + await (await transport.send(request)).close(); + expect(recorder.calls.length).toBe(1); + }); + + test('asyncDispose is the same teardown as close', async () => { + const transport = fetchTransport(); + await transport[Symbol.asyncDispose](); + await transport.close(); + }); + + test('an aborted signal fails the send before any fetch call is made', async () => { + const recorder = recordingFetch(); + const transport = fetchTransport({fetch: recorder.fetch}); + const controller = new AbortController(); + controller.abort(); + const request = Request.newBuilder().url('http://127.0.0.1:1/x').build(); + expect( + await rejection(transport.send(request, undefined, controller.signal)), + ).toMatchObject({name: 'CancellationError'}); + expect(recorder.calls.length).toBe(0); + }); + + test('defaultTimeoutMs applies when the call supplies no timeout of its own', async () => { + const recorder = recordingFetch(); + const transport = fetchTransport({ + fetch: recorder.fetch, + defaultTimeoutMs: 5_000, + }); + const request = Request.newBuilder().url('http://127.0.0.1:1/x').build(); + await (await transport.send(request)).close(); + expect(recorder.calls[0]?.signal).toBeInstanceOf(AbortSignal); + }); +}); diff --git a/packages/transport-fetch/src/fetch-transport.ts b/packages/transport-fetch/src/fetch-transport.ts new file mode 100644 index 0000000..1da8a72 --- /dev/null +++ b/packages/transport-fetch/src/fetch-transport.ts @@ -0,0 +1,332 @@ +// SPDX-License-Identifier: MIT +// packages/transport-fetch/src/fetch-transport.ts +import { + composeSignal, + Protocol, + Response, + Status, + TransportFailureError, + type Body, + type Request, + type RequestOptions, + type Transport, +} from '@dexpace/core'; +import { + abortToSdkError, + createDropLogger, + degradeInboundHeaders, + forkSignal, + isMaterializable, + mapOutboundHeaders, + materializeBody, + producerFailure, + pumpBody, + type ForkedSignal, + type HeaderDropLogging, +} from '@dexpace/transport-shared'; + +/** + * TRANSPORT-1's redirect mode. `'manual'` yields the raw 3xx — status, `Location`, body — on every + * runtime this package is tested against (Node and Bun, both `undici`-backed). + * + * On a **browser** the same value yields an *opaque-redirect* filtered response instead: status `0`, + * no headers, a null body. The redirect is still not followed, so TRANSPORT-1 holds, but the + * pipeline above has nothing to redirect *with*. `@dexpace/transport-fetch` is therefore Node/Bun in + * practice even though its dependency list would run anywhere; a browser build needs a redirect + * strategy that does not depend on reading `Location` off the 3xx. + */ +const REDIRECT_MODE = 'manual' as const; + +/** + * TRANSPORT-11's outbound drop set for this transport. `connection` is in it because WHATWG `fetch` + * treats it as a forbidden request header and would strip it silently — dropping it here makes the + * removal observable through the drop log instead. + */ +const FETCH_FORBIDDEN_HEADERS = [ + 'content-length', + 'host', + 'transfer-encoding', + 'connection', +] as const; + +/** + * Bodies at or below this declared length are materialized into one `Uint8Array` instead of streamed, + * which sidesteps the `duplex: 'half'` corner cases some `fetch` implementations still have. An + * explicit named bound, per the styleguide's "every buffer declares its bound" rule. + */ +const MAX_MATERIALIZED_BODY_BYTES = 1_000_000; + +/** + * Options for {@link fetchTransport}. + * + * There is deliberately **no** `proxy` option: Node's bare global `fetch` exposes no proxy hook that + * does not route through `undici` internals, and depending on `undici` would undo this package's + * entire reason to exist. The absence is the contract — reach for `@dexpace/transport-undici` when + * you need proxying (TRANSPORT-30, scoped out; design doc §6). + * + * @public + */ +export interface FetchTransportOptions { + /** How dropped header names are logged (TRANSPORT-13); defaults to `'first-per-name'`. */ + readonly headerDropLogging?: HeaderDropLogging; + /** A timeout applied to every call that supplies no `RequestOptions.timeoutMs` of its own. */ + readonly defaultTimeoutMs?: number; + /** A custom `fetch` implementation; defaults to `globalThis.fetch`. */ + readonly fetch?: FetchLike; +} + +/** + * The narrow slice of `fetch` this transport calls. Deliberately not `typeof globalThis.fetch`: some + * runtimes hang extra statics off that value (Bun's `fetch.preconnect`), and requiring them would + * reject every reasonable test double while adding nothing this transport uses. + * + * @public + */ +export type FetchLike = ( + input: string, + init: RequestInit & {duplex?: 'half'}, +) => Promise; + +/** A request body prepared for one `fetch` call, plus the teardown its producer may still need. */ +interface PreparedBody { + /** What to hand `RequestInit.body`, or `undefined` for a body-less request. */ + readonly init: BodyInit | undefined; + /** `'half'` when `init` is a stream, which `fetch` requires be declared explicitly. */ + readonly duplex: 'half' | undefined; + /** Settles when the streaming producer finishes; `undefined` for the buffered/no-body cases. */ + readonly done: Promise | undefined; + /** Idempotent teardown for an abandoned producer (TRANSPORT-19); resolves once it has unwound. */ + abandon(cause: unknown): Promise; +} + +const NO_BODY: PreparedBody = { + init: undefined, + duplex: undefined, + done: undefined, + abandon: () => Promise.resolve(), +}; + +async function prepareBody(body: Body | undefined): Promise { + if (body === undefined) return NO_BODY; + if (isMaterializable(body, MAX_MATERIALIZED_BODY_BYTES)) { + try { + return {...NO_BODY, init: await materializeBody(body)}; + } catch (error) { + // Same classification the streaming branch gives the same failure: a body that could not be + // produced is a transport failure with its cause intact, not a raw body error on one path and + // a wrapped one on the other (TRANSPORT-18's buffering clause, restated). + throw new TransportFailureError('request body could not be written', { + cause: error, + }); + } + } + const pump = pumpBody(body); + return { + init: pump.readable, + duplex: 'half', + done: pump.done, + abandon: cause => pump.abandon(cause), + }; +} + +/** One entry per VALUE, so a repeated name survives as repeated appends (HTTP-14). */ +function toNativeHeaders( + request: Request, + logDrops: (dropped: readonly string[]) => void, +): globalThis.Headers { + const {sent, dropped} = mapOutboundHeaders( + request.headers, + FETCH_FORBIDDEN_HEADERS, + {bodyDerivedMediaType: request.body?.mediaType}, + ); + logDrops(dropped); + + const native = new globalThis.Headers(); + for (const [name, value] of sent.entries()) { + try { + native.append(name, value); + } catch { + // TRANSPORT-12: a name the WHATWG layer rejects degrades to a drop, never a failed send. + logDrops([name]); + } + } + return native; +} + +function adaptResponse( + request: Request, + fetchResponse: globalThis.Response, + logDrops: (dropped: readonly string[]) => void, +): Response { + const raw: [string, string][] = []; + fetchResponse.headers.forEach((value, name) => { + // Set-Cookie is the one name WHATWG keeps un-joined; every other name arrives comma-joined. + if (name.toLowerCase() !== 'set-cookie') raw.push([name, value]); + }); + for (const cookie of fetchResponse.headers.getSetCookie()) { + raw.push(['set-cookie', cookie]); + } + + const {headers, dropped} = degradeInboundHeaders(raw); + logDrops(dropped); + + return ( + Response.newBuilder() + .request(request) + // A documented best-effort default, not an observed value: the WHATWG `Response` exposes no + // negotiated-HTTP-version field for this transport to read (Deviation Ledger). + .protocol(Protocol.HTTP_1_1) + .status(Status.of(fetchResponse.status)) + .reasonPhrase(fetchResponse.statusText || undefined) + .headers(headers) + .body(fetchResponse.body) + .build() + ); +} + +/** Everything one dispatch needs beyond the request itself; keeps `max-params` at three. */ +interface DispatchPlan { + readonly headers: globalThis.Headers; + readonly prepared: PreparedBody; + /** The forked signal handed to `fetch`; detached by `send` the moment the response is delivered. */ + readonly fork: ForkedSignal; +} + +class FetchTransport implements Transport, AsyncDisposable { + readonly #logDrops: (dropped: readonly string[]) => void; + readonly #fetch: FetchLike; + readonly #defaultTimeoutMs: number | undefined; + + constructor(options: FetchTransportOptions) { + this.#logDrops = createDropLogger( + options.headerDropLogging ?? 'first-per-name', + ); + this.#fetch = options.fetch ?? globalThis.fetch.bind(globalThis); + this.#defaultTimeoutMs = options.defaultTimeoutMs; + } + + async send( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, + ): Promise { + const composed = composeSignal( + signal, + options?.timeoutMs ?? this.#defaultTimeoutMs, + ); + if (composed?.aborted) throw abortToSdkError(composed, composed.reason); + + // Dispatched with a fork the caller cannot reach: cancellation stays live for the whole in-flight + // window and goes inert the moment the response is handed over (SEAM-16). + const plan: DispatchPlan = { + headers: toNativeHeaders(request, this.#logDrops), + prepared: await prepareBody(request.body), + fork: forkSignal(composed), + }; + try { + return await this.#exchange(request, plan, composed); + } finally { + plan.fork.detach(); + } + } + + async #exchange( + request: Request, + plan: DispatchPlan, + composed: AbortSignal | undefined, + ): Promise { + const fetchResponse = await this.#dispatch(request, plan); + + if (composed?.aborted) { + // TRANSPORT-9 / SEAM-30: this response will never reach a caller, so this producer closes it. + await fetchResponse.body?.cancel().catch(() => undefined); + await plan.prepared.abandon(composed.reason); + throw abortToSdkError(composed, composed.reason); + } + + try { + // TRANSPORT-22: a live socket is in hand, so any throw here must release it before propagating. + return adaptResponse(request, fetchResponse, this.#logDrops); + } catch (error) { + await fetchResponse.body?.cancel().catch(() => undefined); + // TRANSPORT-19: nothing is delivered on this path either, so the producer is owed its teardown + // exactly as on the abort branch above. + await plan.prepared.abandon(error); + throw error; + } + } + + async #dispatch( + request: Request, + plan: DispatchPlan, + ): Promise { + const {prepared} = plan; + const {signal} = plan.fork; + const init: RequestInit & {duplex?: 'half'} = { + method: request.method, + headers: plan.headers, + // TRANSPORT-1: the pipeline, not the native client, is the redirect authority. + redirect: REDIRECT_MODE, + }; + if (prepared.init !== undefined) init.body = prepared.init; + if (prepared.duplex !== undefined) init.duplex = prepared.duplex; + if (signal !== undefined) init.signal = signal; + + try { + // Raced, not sequenced: a producer failure must surface even while `fetch` is still pending, + // and a producer that never resolves must not outlive the send (TRANSPORT-19). + return await Promise.race([ + this.#fetch(request.url.href, init), + producerFailure(prepared.done), + ]); + } catch (error) { + await prepared.abandon(error); + if (signal?.aborted) throw abortToSdkError(signal, error); + throw new TransportFailureError( + error instanceof Error ? error.message : 'fetch failed', + {cause: error}, + ); + } + } + + /** + * Resolves immediately: the global `fetch` owns no resource this package created, so there is + * nothing to release (SEAM-14). `send()` therefore keeps working after `close()` — the documented + * post-close mode this transport picks under SEAM-15. + * + * @returns a promise that resolves once teardown is complete, which is immediately. + */ + close(): Promise { + return Promise.resolve(); + } + + /** + * Single teardown path, delegating to {@link FetchTransport.close}. + * + * @returns a promise that resolves once teardown is complete. + */ + [Symbol.asyncDispose](): Promise { + return this.close(); + } +} + +/** + * Creates a `Transport` backed by the standard global `fetch` — the zero-dependency option. + * + * `close()` is a sanctioned no-op and `send()` keeps working after it (SEAM-15). There is no proxy + * support at all; see {@link FetchTransportOptions}. + * + * The returned transport is `AsyncDisposable`, so `await using transport = fetchTransport(...)` + * releases it at scope exit — the single teardown path `docs/knowledge/resource-management.md` asks + * for. + * + * @param options - optional transport settings. + * @returns a transport ready to send, disposable through `await using`. + * + * @public + */ +export function fetchTransport( + options: FetchTransportOptions = {}, +): Transport & AsyncDisposable { + return new FetchTransport(options); +} diff --git a/packages/transport-fetch/src/index.ts b/packages/transport-fetch/src/index.ts new file mode 100644 index 0000000..d8ed16e --- /dev/null +++ b/packages/transport-fetch/src/index.ts @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: MIT +// packages/transport-fetch/src/index.ts +export {fetchTransport} from './fetch-transport.js'; +export type {FetchLike, FetchTransportOptions} from './fetch-transport.js'; diff --git a/packages/transport-fetch/tsconfig.build.json b/packages/transport-fetch/tsconfig.build.json new file mode 100644 index 0000000..d39dc7e --- /dev/null +++ b/packages/transport-fetch/tsconfig.build.json @@ -0,0 +1,11 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "declaration": true, + "declarationMap": true, + "sourceMap": true + }, + "exclude": [ + "src/**/*.test.ts" + ] +} diff --git a/packages/transport-fetch/tsconfig.json b/packages/transport-fetch/tsconfig.json new file mode 100644 index 0000000..5c03021 --- /dev/null +++ b/packages/transport-fetch/tsconfig.json @@ -0,0 +1,16 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "outDir": "./dist", + "rootDir": "./src", + "target": "ES2023", + "lib": [ + "ES2023", + "DOM", + "DOM.AsyncIterable" + ] + }, + "include": [ + "src/**/*.ts" + ] +} diff --git a/packages/transport-shared/README.md b/packages/transport-shared/README.md new file mode 100644 index 0000000..dc04786 --- /dev/null +++ b/packages/transport-shared/README.md @@ -0,0 +1,22 @@ +# @dexpace/transport-shared + +Internal plumbing shared by `@dexpace/transport-fetch` and `@dexpace/transport-undici`. **Not a +package you install directly** — every export is `@internal`, and both transports depend on it so +that the one algorithm they both need exists once rather than twice. + +It is published anyway because `NFR-4` snapshots every published unit regardless of how its exports +are marked, and because a transport's own `dependencies` must resolve for consumers. + +## What lives here, and why it is not in a transport + +Putting any of this in one transport would make the other depend on a sibling transport, which the +Phase 8 segmentation design deliberately avoids — the two adapters must stay independent of each +other, not merely of the rest of the tree. + +| Module | Concern | +|---|---| +| `header-mapping.ts` | `TRANSPORT-10`/`TRANSPORT-12`'s outbound drop-and-degrade pass and `TRANSPORT-14`'s lenient inbound copy, which preserves obs-text values rather than rejecting them | +| `drop-log.ts` | `TRANSPORT-13`'s bounded, case-insensitive, drain-to-cap dedup of already-logged drop names. Names only — never values | +| `abort-mapping.ts` | The single mapping from an aborted signal to a canonical SDK error: `TransportFailureError` on timeout, `CancellationError` otherwise. A raw `DOMException` is never surfaced | +| `body-pump.ts` | Turning a `Body` into a request stream the transport owns the closing of, plus `TRANSPORT-19`'s idempotent teardown for an abandoned producer | +| `signal-fork.ts` | `SEAM-16`'s abort-after-delivery rule: both native clients tie a response body's lifetime to the signal they were given, so the transport dispatches over a fork it detaches at delivery | diff --git a/packages/transport-shared/api-extractor.json b/packages/transport-shared/api-extractor.json new file mode 100644 index 0000000..455423c --- /dev/null +++ b/packages/transport-shared/api-extractor.json @@ -0,0 +1,22 @@ +{ + "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json", + "mainEntryPointFilePath": "/dist/index.d.ts", + "apiReport": { + "enabled": true, + "reportFolder": "/etc/" + }, + "docModel": { + "enabled": false + }, + "dtsRollup": { + "enabled": false + }, + "messages": { + "extractorMessageReporting": { + "ae-forgotten-export": { + "logLevel": "error", + "addToApiReportFile": false + } + } + } +} diff --git a/packages/transport-shared/etc/transport-shared.api.md b/packages/transport-shared/etc/transport-shared.api.md new file mode 100644 index 0000000..2ee005e --- /dev/null +++ b/packages/transport-shared/etc/transport-shared.api.md @@ -0,0 +1,93 @@ +## API Report File for "@dexpace/transport-shared" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts + +import type { Body as Body_2 } from '@dexpace/core'; +import { DexpaceError } from '@dexpace/core'; +import { Headers as Headers_2 } from '@dexpace/core'; + +// Warning: (ae-internal-missing-underscore) The name "abortToSdkError" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function abortToSdkError(signal: AbortSignal, cause: unknown): DexpaceError; + +// Warning: (ae-internal-missing-underscore) The name "BodyPump" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export interface BodyPump { + abandon(cause: unknown): Promise; + readonly done: Promise; + readonly readable: ReadableStream; +} + +// Warning: (ae-internal-missing-underscore) The name "createDropLogger" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function createDropLogger(mode: HeaderDropLogging): (dropped: readonly string[]) => void; + +// Warning: (ae-internal-missing-underscore) The name "degradeInboundHeaders" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function degradeInboundHeaders(raw: Iterable): { + headers: Headers_2; + dropped: readonly string[]; +}; + +// Warning: (ae-internal-missing-underscore) The name "ForkedSignal" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export interface ForkedSignal { + detach(): void; + readonly signal: AbortSignal | undefined; +} + +// Warning: (ae-internal-missing-underscore) The name "forkSignal" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function forkSignal(source: AbortSignal | undefined): ForkedSignal; + +// Warning: (ae-internal-missing-underscore) The name "HeaderDropLogging" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export type HeaderDropLogging = 'all' | 'first-per-name' | 'quiet'; + +// Warning: (ae-internal-missing-underscore) The name "isMaterializable" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function isMaterializable(body: Body_2, maxBytes: number): boolean; + +// Warning: (ae-internal-missing-underscore) The name "mapOutboundHeaders" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function mapOutboundHeaders(headers: Headers_2, forbidden: readonly string[], opts?: MapOutboundHeadersOptions): { + sent: Headers_2; + dropped: readonly string[]; +}; + +// Warning: (ae-internal-missing-underscore) The name "MapOutboundHeadersOptions" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export interface MapOutboundHeadersOptions { + readonly bodyDerivedMediaType?: string | undefined; +} + +// Warning: (ae-internal-missing-underscore) The name "materializeBody" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function materializeBody(body: Body_2): Promise>; + +// Warning: (ae-internal-missing-underscore) The name "producerFailure" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function producerFailure(done: Promise | undefined): Promise; + +// Warning: (ae-internal-missing-underscore) The name "pumpBody" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function pumpBody(body: Body_2): BodyPump; + +// (No @packageDocumentation comment for this package) + +``` diff --git a/packages/transport-shared/package.json b/packages/transport-shared/package.json new file mode 100644 index 0000000..3f5f0ee --- /dev/null +++ b/packages/transport-shared/package.json @@ -0,0 +1,46 @@ +{ + "name": "@dexpace/transport-shared", + "version": "0.0.0", + "description": "Shared transport adaptation and mapping helpers for the dexpace Node.js SDK.", + "license": "MIT", + "type": "module", + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "engines": { + "node": ">=20.3" + }, + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + } + }, + "files": [ + "dist" + ], + "sideEffects": false, + "dependencies": {}, + "peerDependencies": { + "@dexpace/core": "workspace:*" + }, + "peerDependenciesMeta": { + "@dexpace/core": { + "optional": false + } + }, + "devDependencies": { + "@dexpace/core": "workspace:*", + "@microsoft/api-extractor": "catalog:", + "expect-type": "catalog:", + "fast-check": "catalog:", + "typescript": "catalog:" + }, + "scripts": { + "build": "tsc -p tsconfig.build.json", + "typecheck": "tsc --noEmit", + "test": "bun test", + "api:local": "api-extractor run --local", + "api:ci": "api-extractor run", + "prepublishOnly": "bun run build && bun run api:ci && publint . && attw --pack . --ignore-rules cjs-resolves-to-esm" + } +} diff --git a/packages/transport-shared/src/abort-mapping.test.ts b/packages/transport-shared/src/abort-mapping.test.ts new file mode 100644 index 0000000..533b964 --- /dev/null +++ b/packages/transport-shared/src/abort-mapping.test.ts @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/abort-mapping.test.ts +// Exercises: TRANSPORT-3 (cancellation -> CancellationError), TRANSPORT-4 (timeout -> TransportFailureError) +import {describe, expect, test} from 'bun:test'; +import {CancellationError, TransportFailureError} from '@dexpace/core'; +import {abortToSdkError} from './abort-mapping.js'; + +describe('abortToSdkError', () => { + test('maps AbortController abort to CancellationError', () => { + const controller = new AbortController(); + controller.abort(new Error('user abort')); + const err = abortToSdkError(controller.signal, controller.signal.reason); + expect(err).toBeInstanceOf(CancellationError); + expect(err.message).toBe('request cancelled'); + }); + + test('maps AbortSignal.timeout to TransportFailureError', async () => { + const signal = AbortSignal.timeout(5); + await new Promise(r => setTimeout(r, 20)); + const err = abortToSdkError(signal, signal.reason); + expect(err).toBeInstanceOf(TransportFailureError); + expect(err.message).toBe('request timed out'); + }); +}); diff --git a/packages/transport-shared/src/abort-mapping.ts b/packages/transport-shared/src/abort-mapping.ts new file mode 100644 index 0000000..7c06590 --- /dev/null +++ b/packages/transport-shared/src/abort-mapping.ts @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/abort-mapping.ts +import { + CancellationError, + TransportFailureError, + isTimeoutSignal, + type DexpaceError, +} from '@dexpace/core'; + +/** + * Maps an aborted signal to the canonical SDK error type. + * + * @param signal - the aborted AbortSignal + * @param cause - the original reason or error + * @returns a TransportFailureError if the signal was aborted by timeout, or CancellationError otherwise. + * + * @internal + */ +export function abortToSdkError( + signal: AbortSignal, + cause: unknown, +): DexpaceError { + return isTimeoutSignal(signal) + ? new TransportFailureError('request timed out', {cause}) + : new CancellationError('request cancelled', {cause}); +} diff --git a/packages/transport-shared/src/body-pump.test.ts b/packages/transport-shared/src/body-pump.test.ts new file mode 100644 index 0000000..860af4d --- /dev/null +++ b/packages/transport-shared/src/body-pump.test.ts @@ -0,0 +1,183 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/body-pump.test.ts +// Exercises: TRANSPORT-17 (a body is written exactly once), TRANSPORT-19 (an abandoned streaming +// producer is unblocked, teardown idempotent), BODY-8 (the sink's creator owns closing it) +import {describe, expect, test} from 'bun:test'; +import {byteArrayBody, type Body} from '@dexpace/core'; +import { + isMaterializable, + materializeBody, + producerFailure, + pumpBody, +} from './body-pump.js'; + +function countingBody(closesSink: boolean): Body & {readonly writes: number[]} { + const writes: number[] = []; + return { + kind: 'stream', + mediaType: 'text/plain', + contentLength: -1, + replayable: false, + writes, + async writeTo(sink) { + writes.push(1); + const writer = sink.getWriter(); + await writer.write(new TextEncoder().encode('ab')); + if (closesSink) await writer.close(); + else writer.releaseLock(); + }, + }; +} + +/** Awaits `pending` and hands back its rejection reason, so the assertion stays ordered. */ +async function rejection(pending: Promise): Promise { + try { + await pending; + } catch (error) { + return error; + } + throw new Error('expected the promise to reject, but it resolved'); +} + +async function drain(stream: ReadableStream): Promise { + const reader = stream.getReader(); + const parts: string[] = []; + for (;;) { + const {done, value} = await reader.read(); + if (done) break; + parts.push(new TextDecoder().decode(value)); + } + return parts.join(''); +} + +describe('pumpBody', () => { + test('terminates the stream for a body that closes the sink it was given', async () => { + const body = countingBody(true); + const pump = pumpBody(body); + expect(await drain(pump.readable)).toBe('ab'); + await pump.done; + expect(body.writes.length).toBe(1); + }); + + test('terminates the stream for a body that leaves the sink open (BODY-8)', async () => { + // @dexpace/body-file's writeTo releases its lock without closing; the pump must still end the + // stream, or the native client waits forever on a request body that never finishes. + const pump = pumpBody(countingBody(false)); + expect(await drain(pump.readable)).toBe('ab'); + await pump.done; + }); + + test('a producer failure rejects `done` rather than hanging the stream', async () => { + const body: Body = { + kind: 'stream', + mediaType: undefined, + contentLength: -1, + replayable: false, + writeTo() { + return Promise.reject(new Error('producer exploded')); + }, + }; + const pump = pumpBody(body); + expect(await rejection(pump.done)).toMatchObject({ + message: 'producer exploded', + }); + expect(await rejection(drain(pump.readable))).toBeDefined(); + }); + + test('abandon unblocks a producer that would otherwise never finish, idempotently', async () => { + let unblocked = false; + const body: Body = { + kind: 'stream', + mediaType: undefined, + contentLength: -1, + replayable: false, + async writeTo(sink) { + const writer = sink.getWriter(); + try { + // No reader ever drains this, so the second write parks on backpressure forever unless + // abandon() aborts the writer underneath it (TRANSPORT-19). + for (;;) await writer.write(new Uint8Array(64 * 1024)); + } finally { + unblocked = true; + } + }, + }; + const pump = pumpBody(body); + await pump.abandon(new Error('send failed')); + await pump.abandon(new Error('send failed')); + expect(unblocked).toBe(true); + }); +}); + +describe('producerFailure', () => { + /** Settles `pending` against a marker, so "never settles" is observable without hanging the row. */ + async function raceWithTimeout(pending: Promise): Promise { + return Promise.race([ + pending.then( + () => 'resolved', + (error: unknown) => `rejected: ${(error as Error).message}`, + ), + new Promise(resolve => + setTimeout(() => { + resolve('pending'); + }, 50), + ), + ]); + } + + test('never settles when there is no streamed producer', async () => { + expect(await raceWithTimeout(producerFailure(undefined))).toBe('pending'); + }); + + test('never settles when the producer succeeds', async () => { + // A producer finishing says nothing about the response, so this must not win a `Promise.race` + // against a dispatch that is still in flight. + expect(await raceWithTimeout(producerFailure(Promise.resolve()))).toBe( + 'pending', + ); + }); + + test('carries the producer failure onward', async () => { + const done = Promise.reject(new Error('producer exploded')); + expect(await raceWithTimeout(producerFailure(done))).toBe( + 'rejected: producer exploded', + ); + }); + + test('keeps a handler on a rejection that lands after the race settled', async () => { + // The delivery-path guarantee, at its source: once `Promise.race` has attached to this promise, + // a producer that fails later is an observed rejection rather than one that reaches the + // runtime's default `unhandledRejection` policy. A leak here fails the row on both runners. + let fail!: (error: Error) => void; + const done = new Promise((_resolve, reject) => { + fail = reject; + }); + const raced = await Promise.race([ + producerFailure(done), + new Promise(resolve => + setTimeout(() => { + resolve('delivered'); + }, 10), + ), + ]); + expect(raced).toBe('delivered'); + fail(new Error('late producer failure')); + await new Promise(resolve => setTimeout(resolve, 50)); + }); +}); + +describe('materializeBody / isMaterializable', () => { + test('collects every chunk in order', async () => { + const bytes = await materializeBody( + byteArrayBody(new Uint8Array([1, 2, 3])), + ); + expect([...bytes]).toEqual([1, 2, 3]); + }); + + test('classifies by replayability and declared length', () => { + const small = byteArrayBody(new Uint8Array([1])); + expect(isMaterializable(small, 10)).toBe(true); + expect(isMaterializable(small, 0)).toBe(false); + expect(isMaterializable(countingBody(true), 10)).toBe(false); + }); +}); diff --git a/packages/transport-shared/src/body-pump.ts b/packages/transport-shared/src/body-pump.ts new file mode 100644 index 0000000..a7e7f6d --- /dev/null +++ b/packages/transport-shared/src/body-pump.ts @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/body-pump.ts +import type {Body} from '@dexpace/core'; + +/** + * A streaming request body in flight: the stream to hand the native client, the producer's own + * settlement, and the teardown an abandoned send owes it (TRANSPORT-19). + * + * @internal + */ +export interface BodyPump { + /** The bytes `writeTo` produces, ready to hand to the native client. */ + readonly readable: ReadableStream; + /** Settles when the producer finishes; rejects with whatever `writeTo` raised. */ + readonly done: Promise; + /** Idempotent teardown: aborts the producer and resolves once it has actually unwound. */ + abandon(cause: unknown): Promise; +} + +/** + * The sink handed to `writeTo`, interposed rather than passing the `TransformStream`'s own writable + * straight through. Closing belongs to whoever created the stream (BODY-8), and the two conventions + * in this tree disagree: every `@dexpace/core` body closes the sink it was given, while + * `@dexpace/body-file`'s deliberately does not. Owning `close` here terminates the request body + * exactly once for both shapes — handing over the raw writable would either double-close (a + * `TypeError` that surfaces as a failed send) or never close at all (the native client waiting + * forever on a stream that never ends). + */ +function interposedSink( + writer: WritableStreamDefaultWriter, +): WritableStream { + return new WritableStream({ + write: chunk => writer.write(chunk), + close: () => undefined, + abort: () => undefined, + }); +} + +/** + * Starts `body`'s producer against a fresh `TransformStream` and returns the read end. + * + * The returned `done` is retained, never floating: a `writeTo` rejection must fail the send rather + * than leave the native client waiting on a stream that never closes. + * + * @param body - the body to stream; written exactly once (TRANSPORT-17). + * @returns the read end, the producer's settlement, and its teardown. + * + * @internal + */ +export function pumpBody(body: Body): BodyPump { + const {readable, writable} = new TransformStream(); + const writer = writable.getWriter(); + const done = (async () => { + try { + await body.writeTo(interposedSink(writer)); + } catch (error) { + await writer.abort(error).catch(() => undefined); + throw error; + } + await writer.close(); + })(); + return { + readable, + done, + abandon: async (cause: unknown) => { + // `abort` is idempotent, satisfying TRANSPORT-19's idempotent-teardown clause; awaiting the + // producer with its rejection swallowed guarantees it has unwound before `send()` returns. + await writer.abort(cause).catch(() => undefined); + await done.catch(() => undefined); + }, + }; +} + +/** + * A promise that rejects when `done` rejects and otherwise never settles, for racing a pending + * dispatch against its own request-body producer. + * + * Racing is not the only reason to call this, and on the delivery path it is not even the main one: + * `Promise.race` attaches a handler to `done` that outlives the race, so a producer that fails + * *after* the native client already delivered a response is an observed rejection rather than an + * unhandled one. Without it that late rejection reaches Node's default `unhandledRejection` policy + * and takes the process down — the exact hazard SEAM-30 names, arriving from the request side. + * + * @param done - the producer settlement from {@link pumpBody}, or `undefined` when the body was not + * streamed. + * @returns a promise that rejects with the producer's failure and never resolves. + * + * @internal + */ +export function producerFailure( + done: Promise | undefined, +): Promise { + if (done === undefined) return new Promise(() => undefined); + // `then` with no rejection handler: a producer *success* says nothing about the response, so the + // derived promise only ever carries the failure onward. + return done.then(() => new Promise(() => undefined)); +} + +/** + * Collects `body` into one contiguous buffer, for the small-and-replayable case both transports + * prefer over a streamed request body. + * + * The `Uint8Array` return type is load-bearing, not decoration: `BodyInit` accepts + * `ArrayBufferView` but not the `ArrayBufferLike`-backed default, which may be a + * `SharedArrayBuffer`. This always allocates a fresh, non-shared buffer, so it says so. + * + * @param body - the body to write. + * @returns every byte the body produced, in order. + * + * @internal + */ +export async function materializeBody( + body: Body, +): Promise> { + // Chunks are retained by reference until the merge below, which relies on the Web Streams + // convention that a chunk passed to `write()` belongs to the sink. Every `Body` in this tree + // allocates per chunk (`node:fs` read streams included); a producer that wrote views over one + // reused scratch buffer would need a copy here instead. + const chunks: Uint8Array[] = []; + let total = 0; + await body.writeTo( + new WritableStream({ + write(chunk) { + chunks.push(chunk); + total += chunk.byteLength; + }, + }), + ); + const merged = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + merged.set(chunk, offset); + offset += chunk.byteLength; + } + return merged; +} + +/** + * Whether a body is small enough and replayable enough to materialize rather than stream. Streaming + * request bodies still carry `duplex: 'half'` corner cases in some `fetch` implementations, so the + * buffered path is the default wherever it is available. + * + * @param body - the body to classify. + * @param maxBytes - the inclusive upper bound on a materializable body's declared length. + * @returns `true` when {@link materializeBody} should be used instead of {@link pumpBody}. + * + * @internal + */ +export function isMaterializable(body: Body, maxBytes: number): boolean { + return ( + body.replayable && body.contentLength >= 0 && body.contentLength <= maxBytes + ); +} diff --git a/packages/transport-shared/src/drop-log.test.ts b/packages/transport-shared/src/drop-log.test.ts new file mode 100644 index 0000000..41bce17 --- /dev/null +++ b/packages/transport-shared/src/drop-log.test.ts @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/drop-log.test.ts +// Exercises: TRANSPORT-13 (HeaderDropLogging: all, first-per-name, quiet; bounded case-insensitive dedup) +import {afterEach, beforeEach, describe, expect, test} from 'bun:test'; +import {getGlobalLogger, setGlobalLogger, type Logger} from '@dexpace/core'; +import {createDropLogger} from './drop-log.js'; + +let logged: {event?: string; fields: Record}[] = []; +let originalLogger: Logger; + +beforeEach(() => { + logged = []; + originalLogger = getGlobalLogger(); + setGlobalLogger({ + atLevel: () => { + const entry: {event?: string; fields: Record} = { + fields: {}, + }; + const mockEvent = { + event: (name: string) => { + entry.event = name; + return mockEvent; + }, + field: (key: string, value: unknown) => { + entry.fields[key] = value; + return mockEvent; + }, + cause: () => mockEvent, + emit: () => { + logged.push(entry); + }, + }; + return mockEvent; + }, + withContext: () => originalLogger, + }); +}); + +afterEach(() => { + setGlobalLogger(originalLogger); +}); + +describe('createDropLogger (TRANSPORT-13)', () => { + test('mode quiet logs nothing', () => { + const logger = createDropLogger('quiet'); + logger(['Content-Length', 'Host']); + expect(logged.length).toBe(0); + }); + + test('mode all logs every occurrence', () => { + const logger = createDropLogger('all'); + logger(['Content-Length']); + logger(['content-length']); + expect(logged.length).toBe(2); + }); + + test('mode first-per-name dedups case-insensitively', () => { + const logger = createDropLogger('first-per-name'); + logger(['Content-Length']); + logger(['content-length']); + logger(['X-Custom']); + expect(logged.length).toBe(2); + expect(logged[0]?.fields).toEqual({header: 'content-length'}); + expect(logged[1]?.fields).toEqual({header: 'x-custom'}); + }); + + test('bounded dedup drains to MAX_LOGGED_DROP_NAMES', () => { + const logger = createDropLogger('first-per-name'); + const names = Array.from({length: 150}, (_, i) => `x-header-${String(i)}`); + logger(names); + expect(logged.length).toBe(150); + }); +}); diff --git a/packages/transport-shared/src/drop-log.ts b/packages/transport-shared/src/drop-log.ts new file mode 100644 index 0000000..0994318 --- /dev/null +++ b/packages/transport-shared/src/drop-log.ts @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/drop-log.ts +import {getGlobalLogger} from '@dexpace/core'; + +/** + * Logging mode for dropped headers (TRANSPORT-13). + * + * @internal + */ +export type HeaderDropLogging = 'all' | 'first-per-name' | 'quiet'; + +/** Bound on the dedup set so an attacker synthesising distinct names cannot grow it (TRANSPORT-13, XCUT-14). */ +const MAX_LOGGED_DROP_NAMES = 128; + +function emitDropLog(key: string): void { + try { + getGlobalLogger() + .atLevel('verbose') + .event('http.header.dropped') + .field('header', key) + .emit(); + } catch { + // OBS-20: logger failure must never fail the request + } +} + +/** + * Evicts the oldest name once the set has outgrown its bound. One eviction per insert is enough + * precisely because this runs on every insert -- the set can only ever be one over the cap. + */ +function trimSeen(seen: Set): void { + if (seen.size > MAX_LOGGED_DROP_NAMES) { + const first = seen.values().next().value; + if (first !== undefined) { + seen.delete(first); + } + } +} + +/** + * Creates a drop logger function adhering to the requested logging mode and bounded dedup policy. + * + * @internal + */ +export function createDropLogger( + mode: HeaderDropLogging, +): (dropped: readonly string[]) => void { + if (mode === 'quiet') { + return () => undefined; + } + const seen = new Set(); + return (dropped: readonly string[]) => { + for (const name of dropped) { + const key = name.toLowerCase(); + if (mode === 'first-per-name' && seen.has(key)) { + continue; + } + if (mode === 'first-per-name') { + seen.add(key); + trimSeen(seen); + } + emitDropLog(key); + } + }; +} diff --git a/packages/transport-shared/src/header-mapping.test.ts b/packages/transport-shared/src/header-mapping.test.ts new file mode 100644 index 0000000..4ee6219 --- /dev/null +++ b/packages/transport-shared/src/header-mapping.test.ts @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/header-mapping.test.ts +// Exercises: TRANSPORT-10 (Content-Type authority), TRANSPORT-11 (framing-header drop set, verbose log), +// TRANSPORT-12 (per-header graceful degradation), TRANSPORT-14 (lenient inbound copy, obs-text preserved, control-byte header dropped) +import {describe, expect, test} from 'bun:test'; +import {Headers} from '@dexpace/core'; +import {degradeInboundHeaders, mapOutboundHeaders} from './header-mapping.js'; + +describe('mapOutboundHeaders', () => { + test('drops framing headers the native client computes', () => { + const {sent, dropped} = mapOutboundHeaders( + Headers.newBuilder() + .set('Content-Length', '999') + .set('X-Custom', 'v') + .build(), + ['content-length', 'host', 'transfer-encoding'], + ); + expect(sent.get('content-length')).toBeUndefined(); + expect(sent.get('x-custom')).toBe('v'); + expect(dropped).toContain('content-length'); + }); + + test('an explicit Content-Type is never overwritten by a body-derived one', () => { + const {sent} = mapOutboundHeaders( + Headers.newBuilder().set('Content-Type', 'text/plain').build(), + [], + {bodyDerivedMediaType: 'application/json'}, + ); + expect(sent.get('content-type')).toBe('text/plain'); + }); + + test('sets body-derived Content-Type when none is provided', () => { + const {sent} = mapOutboundHeaders( + Headers.newBuilder().set('X-Custom', 'v').build(), + [], + {bodyDerivedMediaType: 'application/json'}, + ); + expect(sent.get('content-type')).toBe('application/json'); + expect(sent.get('x-custom')).toBe('v'); + }); +}); + +describe('mapOutboundHeaders graceful degradation (TRANSPORT-12)', () => { + test('a value the outbound grammar rejects drops that header only', () => { + // `addInbound` is the lenient path (HTTP-19) and admits obs-text; the strict outbound `add` + // does not. A Headers built from a server response and re-sent is the realistic way a + // model-valid, wire-invalid value reaches this function. + const inbound = Headers.newBuilder() + .addInbound('X-Obs-Text', 'caf\u00e9') + .add('X-Kept', 'value') + .build(); + const {sent, dropped} = mapOutboundHeaders(inbound, []); + expect(sent.get('x-obs-text')).toBeUndefined(); + expect(sent.get('x-kept')).toBe('value'); + expect(dropped).toEqual(['x-obs-text']); + }); + + test('an unusable body-derived media type is dropped rather than failing the mapping', () => { + const {sent, dropped} = mapOutboundHeaders( + Headers.newBuilder().set('X-Kept', 'value').build(), + [], + {bodyDerivedMediaType: 'text/plain\u0000'}, + ); + expect(sent.get('content-type')).toBeUndefined(); + expect(sent.get('x-kept')).toBe('value'); + expect(dropped).toEqual(['content-type']); + }); +}); + +describe('degradeInboundHeaders', () => { + test('drops a header whose value carries a control byte, keeps the rest', () => { + const {headers, dropped} = degradeInboundHeaders([ + ['x-bad', 'v\x01alue'], + ['x-good', 'value'], + ]); + expect(headers.get('x-bad')).toBeUndefined(); + expect(headers.get('x-good')).toBe('value'); + expect(dropped).toEqual(['x-bad']); + }); + + test('drops a header whose name carries non-ASCII or control characters', () => { + const {headers, dropped} = degradeInboundHeaders([ + ['x-bad\x02name', 'value'], + ['x-bad-café', 'value'], + ['x-good', 'value'], + ]); + expect(headers.get('x-bad\x02name')).toBeUndefined(); + expect(headers.get('x-bad-café')).toBeUndefined(); + expect(headers.get('x-good')).toBe('value'); + expect(dropped).toContain('x-bad\x02name'); + expect(dropped).toContain('x-bad-café'); + }); + + test('preserves an obs-text (non-ASCII) byte in a value rather than stripping it', () => { + const {headers} = degradeInboundHeaders([['x-name', 'café']]); + expect(headers.get('x-name')).toBe('café'); + }); +}); diff --git a/packages/transport-shared/src/header-mapping.ts b/packages/transport-shared/src/header-mapping.ts new file mode 100644 index 0000000..fbf72db --- /dev/null +++ b/packages/transport-shared/src/header-mapping.ts @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/header-mapping.ts +import {Headers} from '@dexpace/core'; + +/* eslint-disable no-control-regex -- RFC 9110 requires testing for ASCII control characters */ +const CONTROL_BYTE = /[\x00-\x08\x0B-\x1F\x7F]/u; +const NON_ASCII_OR_CONTROL = /[\x00-\x1F\x7F-\uFFFF]/u; +/* eslint-enable no-control-regex -- re-enable */ + +/** + * Options for outbound header mapping. + * + * @internal + */ +export interface MapOutboundHeadersOptions { + /** A media type derived from the request body to use if Content-Type is absent. */ + readonly bodyDerivedMediaType?: string | undefined; +} + +/** + * Filters forbidden framing headers and applies per-header degradation for outbound requests (TRANSPORT-10-12). + * + * @internal + */ +export function mapOutboundHeaders( + headers: Headers, + forbidden: readonly string[], + opts: MapOutboundHeadersOptions = {}, +): {sent: Headers; dropped: readonly string[]} { + const forbiddenSet = new Set(forbidden.map(h => h.toLowerCase())); + const dropped: string[] = []; + const builder = Headers.newBuilder(); + for (const [name, value] of headers.entries()) { + if (forbiddenSet.has(name.toLowerCase())) { + dropped.push(name.toLowerCase()); + continue; + } + try { + builder.add(name, value); + } catch { + dropped.push(name.toLowerCase()); + } + } + if ( + opts.bodyDerivedMediaType !== undefined && + headers.get('content-type') === undefined + ) { + try { + builder.set('Content-Type', opts.bodyDerivedMediaType); + } catch { + dropped.push('content-type'); + } + } + return {sent: builder.build(), dropped}; +} + +/** + * Leniently copies inbound response headers, dropping malformed entries while preserving obs-text (TRANSPORT-14). + * + * @internal + */ +export function degradeInboundHeaders( + raw: Iterable, +): {headers: Headers; dropped: readonly string[]} { + const dropped: string[] = []; + const builder = Headers.newBuilder(); + for (const [name, value] of raw) { + if (NON_ASCII_OR_CONTROL.test(name) || CONTROL_BYTE.test(value)) { + dropped.push(name); + continue; + } + try { + builder.addInbound(name, value); + } catch { + dropped.push(name); + } + } + return {headers: builder.build(), dropped}; +} diff --git a/packages/transport-shared/src/index.ts b/packages/transport-shared/src/index.ts new file mode 100644 index 0000000..9c88c43 --- /dev/null +++ b/packages/transport-shared/src/index.ts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/index.ts +export {abortToSdkError} from './abort-mapping.js'; +export { + isMaterializable, + materializeBody, + producerFailure, + pumpBody, + type BodyPump, +} from './body-pump.js'; +export {createDropLogger, type HeaderDropLogging} from './drop-log.js'; +export { + degradeInboundHeaders, + mapOutboundHeaders, + type MapOutboundHeadersOptions, +} from './header-mapping.js'; +export {forkSignal, type ForkedSignal} from './signal-fork.js'; diff --git a/packages/transport-shared/src/signal-fork.test.ts b/packages/transport-shared/src/signal-fork.test.ts new file mode 100644 index 0000000..6fa63e8 --- /dev/null +++ b/packages/transport-shared/src/signal-fork.test.ts @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/signal-fork.test.ts +// Exercises: SEAM-16 (an abort after delivery must not reach the native client), SEAM-13/TRANSPORT-7 +// (an abort before delivery must) +import {describe, expect, test} from 'bun:test'; +import {forkSignal} from './signal-fork.js'; + +describe('forkSignal', () => { + test('returns no signal when the caller supplied none', () => { + const fork = forkSignal(undefined); + expect(fork.signal).toBeUndefined(); + expect(() => { + fork.detach(); + }).not.toThrow(); + }); + + test('forwards an abort that fires while still attached, reason and all', () => { + const controller = new AbortController(); + const fork = forkSignal(controller.signal); + const reason = new Error('caller changed their mind'); + controller.abort(reason); + expect(fork.signal?.aborted).toBe(true); + expect(fork.signal?.reason).toBe(reason); + }); + + test('an already-aborted source forks as already aborted', () => { + const controller = new AbortController(); + controller.abort(new Error('too late')); + const fork = forkSignal(controller.signal); + expect(fork.signal?.aborted).toBe(true); + }); + + test('an abort after detach never reaches the fork (SEAM-16)', () => { + const controller = new AbortController(); + const fork = forkSignal(controller.signal); + fork.detach(); + fork.detach(); // idempotent + controller.abort(new Error('after delivery')); + expect(fork.signal?.aborted).toBe(false); + }); +}); diff --git a/packages/transport-shared/src/signal-fork.ts b/packages/transport-shared/src/signal-fork.ts new file mode 100644 index 0000000..5561906 --- /dev/null +++ b/packages/transport-shared/src/signal-fork.ts @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: MIT +// packages/transport-shared/src/signal-fork.ts + +/** + * A caller signal, forwarded to the native client only for as long as the transport wants it. + * + * @internal + */ +export interface ForkedSignal { + /** Hand this to the native client instead of the caller's own signal. */ + readonly signal: AbortSignal | undefined; + /** Stops forwarding. Idempotent; later aborts of the source no longer reach the native client. */ + detach(): void; +} + +/** + * Forks `source` into a signal the transport controls. + * + * SEAM-16 forbids a signal abort that fires *after* the send resolved from closing the + * already-delivered response body — the caller still owns it, even when discarding the value. Both + * WHATWG `fetch` and undici tie the response body's lifetime to whatever signal they were handed, so + * passing the caller's signal straight through violates that clause: a later `controller.abort()` + * truncates a body the caller was reading. Forwarding through a fork the transport detaches at + * delivery keeps cancellation live for the whole in-flight window (SEAM-13, TRANSPORT-7) and inert + * afterwards. + * + * @param source - the composed caller/timeout signal, if any. + * @returns the signal to dispatch with, plus the detach the transport calls on delivery. + * + * @internal + */ +export function forkSignal(source: AbortSignal | undefined): ForkedSignal { + if (source === undefined) { + return {signal: undefined, detach: () => undefined}; + } + const controller = new AbortController(); + if (source.aborted) { + controller.abort(source.reason); + return {signal: controller.signal, detach: () => undefined}; + } + const forward = (): void => { + controller.abort(source.reason); + }; + source.addEventListener('abort', forward, {once: true}); + return { + signal: controller.signal, + // removeEventListener is idempotent, so a detach on both the success and failure path is safe. + detach: () => { + source.removeEventListener('abort', forward); + }, + }; +} diff --git a/packages/transport-shared/tsconfig.build.json b/packages/transport-shared/tsconfig.build.json new file mode 100644 index 0000000..d39dc7e --- /dev/null +++ b/packages/transport-shared/tsconfig.build.json @@ -0,0 +1,11 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "declaration": true, + "declarationMap": true, + "sourceMap": true + }, + "exclude": [ + "src/**/*.test.ts" + ] +} diff --git a/packages/transport-shared/tsconfig.json b/packages/transport-shared/tsconfig.json new file mode 100644 index 0000000..6a85a2a --- /dev/null +++ b/packages/transport-shared/tsconfig.json @@ -0,0 +1,17 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "outDir": "./dist", + "rootDir": "./src", + "target": "ES2023", + "stripInternal": false, + "lib": [ + "ES2023", + "DOM", + "DOM.AsyncIterable" + ] + }, + "include": [ + "src/**/*.ts" + ] +} diff --git a/packages/transport-undici/README.md b/packages/transport-undici/README.md new file mode 100644 index 0000000..dbb338a --- /dev/null +++ b/packages/transport-undici/README.md @@ -0,0 +1,88 @@ +# @dexpace/transport-undici + +The full-featured `Transport` for the dexpace SDK, built on `undici` — connection-pool control, +proxy routing, and real ownership-aware `close()` semantics. Exactly one external dependency. + +```sh +bun add @dexpace/transport-undici @dexpace/core +``` + +```typescript +import {Request} from '@dexpace/core'; +import {undiciTransport} from '@dexpace/transport-undici'; + +await using transport = undiciTransport({ + agentOptions: {connections: 32}, + defaultTimeoutMs: 30_000, +}); + +const response = await transport.send( + Request.newBuilder().url('https://example.com/v1/users').build(), +); +await response.close(); +``` + +## Dispatcher ownership + +Exactly one decision, made once at construction, fixing both the dispatcher and who closes it: + +| Option supplied | Dispatcher used | Closed by `close()` | +|---|---|---| +| `dispatcher` | yours, as-is | **no** — a caller-supplied client is never touched (`SEAM-14`) | +| `proxy` | a `ProxyAgent` this package constructs, plus an `Agent` for `NO_PROXY` hosts | yes, both | +| neither | an `Agent` this package constructs | yes | + +Supplying **both** `dispatcher` and `proxy` is a construction-time `TypeError`, not a silent win for +one: a bring-your-own dispatcher may already be a `ProxyAgent`, and ignoring either option would +hide which is in force. `close()` is idempotent and concurrent calls share one teardown +(`TRANSPORT-15`/`TRANSPORT-16`). + +`close()` **destroys** the dispatchers it owns rather than draining them: `TRANSPORT-16` requires a +non-blocking shutdown with no unbounded await, and a graceful close would stall teardown for as long +as one in-flight send against a slow peer takes. Sends still in flight therefore reject with the +terminal `CancellationError`, and so does a `send()` issued after `close()` — this transport's +documented `SEAM-15` post-close mode. It cannot succeed over a dispatcher that no longer exists, so +it is not reported as a retryable failure. + +## Proxy support and its one real limit + +`ProxyOptions` routes here in full: address, Basic credentials, and `NO_PROXY`/`nonProxyHosts` +bypass globs, which route over a separate direct `Agent` rather than being tunnelled anyway. + +**A custom `challengeHandler` cannot be dispatched**, and the limitation is surfaced rather than +silently misbehaving (`TRANSPORT-30`): + +- undici's `ProxyAgent` takes its credential **only** from its own constructor and rejects any + per-request `Proxy-Authorization` header with `InvalidArgumentError` — a deliberate security fix + on their side, not an oversight. The constructor runs before any challenge has been seen, so + there is no point at which a handler-minted credential could be applied to the exchange that + provoked it. +- Configuring one therefore emits a WARN at construction, and a second WARN the first time a proxy + actually answers `407`. The `407` is surfaced to the caller unchanged, for its own auth layer. +- Proxy auth falls back to **Basic**: `ProxyOptions.credentials`, which is passed to the + `ProxyAgent` constructor as a token. Credentials are never logged, and are never sent in answer to + an origin-server `401`. +- A per-request `Proxy-Authorization` header is dropped from the outbound pass whenever a proxy is + configured — forwarding one would turn every proxied send into a hard failure. The drop is logged + by name like any other. + +## Behavior worth knowing + +- File bodies (`body.kind === 'file'`, e.g. `@dexpace/body-file`'s `fileBody()`) dispatch straight + off the file honoring `start`/`count`, one fewer userspace copy than the `fetch` transport + (`TRANSPORT-28`; a literal kernel zero-copy path does not exist on Node — see the Deviation + Ledger). Recognition is structural, on `kind` alone: this package does not depend on + `@dexpace/body-file`. +- Redirects are pinned off (`maxRedirections: 0`) even behind a bring-your-own dispatcher that may + carry a redirect interceptor. The pipeline is the single redirect authority. +- `Connection` is **not** dropped outbound — §17's own note is that an undici-class transport + forwards it. `Content-Length`, `Host`, and `Transfer-Encoding` are. +- Destroying the dispatcher mid-flight surfaces as the terminal `CancellationError`, while a timeout + on the same path stays the retryable `TransportFailureError` (`TRANSPORT-8`). +- `Response.protocol` is always `HTTP_1_1`: undici's `ResponseData` does not surface the negotiated + version. A Deviation Ledger row, not a silent gap. + +## Conformance + +Proven against the shared `TRANSPORT-N` suite in `@dexpace/transport-conformance`, the same one +`@dexpace/transport-fetch` runs, so the two adapters cannot drift. diff --git a/packages/transport-undici/api-extractor.json b/packages/transport-undici/api-extractor.json new file mode 100644 index 0000000..455423c --- /dev/null +++ b/packages/transport-undici/api-extractor.json @@ -0,0 +1,22 @@ +{ + "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json", + "mainEntryPointFilePath": "/dist/index.d.ts", + "apiReport": { + "enabled": true, + "reportFolder": "/etc/" + }, + "docModel": { + "enabled": false + }, + "dtsRollup": { + "enabled": false + }, + "messages": { + "extractorMessageReporting": { + "ae-forgotten-export": { + "logLevel": "error", + "addToApiReportFile": false + } + } + } +} diff --git a/packages/transport-undici/etc/transport-undici.api.md b/packages/transport-undici/etc/transport-undici.api.md new file mode 100644 index 0000000..752c8eb --- /dev/null +++ b/packages/transport-undici/etc/transport-undici.api.md @@ -0,0 +1,27 @@ +## API Report File for "@dexpace/transport-undici" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts + +import type { Agent } from 'undici'; +import type { Dispatcher } from 'undici'; +import { HeaderDropLogging } from '@dexpace/transport-shared'; +import { ProxyOptions } from '@dexpace/core'; +import { Transport } from '@dexpace/core'; + +// @public +export function undiciTransport(options?: UndiciTransportOptions): Transport & AsyncDisposable; + +// @public +export interface UndiciTransportOptions { + readonly agentOptions?: Agent.Options; + readonly defaultTimeoutMs?: number; + readonly dispatcher?: Dispatcher; + readonly headerDropLogging?: HeaderDropLogging; + readonly proxy?: ProxyOptions; +} + +// (No @packageDocumentation comment for this package) + +``` diff --git a/packages/transport-undici/package.json b/packages/transport-undici/package.json new file mode 100644 index 0000000..7cd57b7 --- /dev/null +++ b/packages/transport-undici/package.json @@ -0,0 +1,50 @@ +{ + "name": "@dexpace/transport-undici", + "version": "0.0.0", + "description": "Undici-based transport adapter with proxy and connection pooling support for the dexpace Node.js SDK.", + "license": "MIT", + "type": "module", + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "engines": { + "node": ">=20.3" + }, + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + } + }, + "files": [ + "dist" + ], + "sideEffects": false, + "dependencies": { + "@dexpace/transport-shared": "workspace:*", + "undici": "^6.21.1" + }, + "peerDependencies": { + "@dexpace/core": "workspace:*" + }, + "peerDependenciesMeta": { + "@dexpace/core": { + "optional": false + } + }, + "devDependencies": { + "@dexpace/core": "workspace:*", + "@dexpace/transport-conformance": "workspace:*", + "@microsoft/api-extractor": "catalog:", + "expect-type": "catalog:", + "fast-check": "catalog:", + "typescript": "catalog:" + }, + "scripts": { + "build": "tsc -p tsconfig.build.json", + "typecheck": "tsc --noEmit", + "test": "bun test", + "api:local": "api-extractor run --local", + "api:ci": "api-extractor run", + "prepublishOnly": "bun run build && bun run api:ci && publint . && attw --pack . --ignore-rules cjs-resolves-to-esm" + } +} diff --git a/packages/transport-undici/src/challenge-handler.test.ts b/packages/transport-undici/src/challenge-handler.test.ts new file mode 100644 index 0000000..942c5db --- /dev/null +++ b/packages/transport-undici/src/challenge-handler.test.ts @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: MIT +// packages/transport-undici/src/challenge-handler.test.ts +// Exercises: TRANSPORT-30 -- an undispatchable custom proxy challenge handler is surfaced with a +// WARN at construction and again the first time a 407 actually arrives, proxy auth falls back to +// Basic, an origin-server 401 is never treated as a proxy challenge, and no credential is ever logged +import {afterEach, beforeEach, describe, expect, test} from 'bun:test'; +import { + createProxyOptions, + getGlobalLogger, + Headers, + Protocol, + Request, + Response, + setGlobalLogger, + Status, + type Logger, + type ProxyOptions, +} from '@dexpace/core'; +import { + createProxyChallengeReporter, + warnIfCustomChallengeHandler, +} from './challenge-handler.js'; + +/** Every field value the global logger saw, so "credentials are never logged" is checkable. */ +let logged: string[] = []; +let previousLogger: Logger; + +beforeEach(() => { + logged = []; + previousLogger = getGlobalLogger(); + const capturing: Logger = { + atLevel: level => { + const entry = { + field: (key: string, value: unknown) => { + logged.push(`${key}=${String(value)}`); + return entry; + }, + event: (name: string) => { + logged.push(`event=${name}@${level}`); + return entry; + }, + cause: (error: unknown) => { + logged.push(`cause=${String(error)}`); + return entry; + }, + emit: () => undefined, + }; + return entry; + }, + withContext: () => capturing, + }; + setGlobalLogger(capturing); +}); + +afterEach(() => { + setGlobalLogger(previousLogger); +}); + +const SECRET = 'hunter2'; + +function proxyWithHandler(): ProxyOptions { + return createProxyOptions({ + type: 'http', + host: 'proxy.internal', + port: 8080, + credentials: {username: 'user', password: SECRET}, + challengeHandler: () => 'Bearer minted-token', + }); +} + +function makeResponse(status: number): Response { + return Response.newBuilder() + .request(Request.newBuilder().url('http://localhost').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(status)) + .headers(Headers.newBuilder().build()) + .build(); +} + +describe('warnIfCustomChallengeHandler', () => { + test('says nothing without a proxy, or with a proxy carrying no custom handler', () => { + warnIfCustomChallengeHandler(undefined); + warnIfCustomChallengeHandler( + createProxyOptions({type: 'http', host: 'proxy.internal', port: 8080}), + ); + expect(logged).toEqual([]); + }); + + test('warns at construction, naming the proxy address but never its credentials', () => { + warnIfCustomChallengeHandler(proxyWithHandler()); + const rendered = logged.join('|'); + expect(rendered).toContain( + 'event=proxy.challengeHandler.unsupported@warning', + ); + expect(rendered).toContain('proxy.host=proxy.internal'); + expect(rendered).toContain('proxy.port=8080'); + expect(rendered).not.toContain(SECRET); + }); +}); + +describe('createProxyChallengeReporter', () => { + test('is inert when no custom handler is configured', () => { + const report = createProxyChallengeReporter( + createProxyOptions({type: 'http', host: 'proxy.internal', port: 8080}), + ); + report(makeResponse(407)); + expect(logged).toEqual([]); + }); + + test('never treats an origin-server 401 as a proxy challenge', () => { + const report = createProxyChallengeReporter(proxyWithHandler()); + report(makeResponse(401)); + report(makeResponse(200)); + expect(logged).toEqual([]); + }); + + test('warns on the first 407 and stays quiet on every one after it', () => { + const report = createProxyChallengeReporter(proxyWithHandler()); + report(makeResponse(407)); + const afterFirst = logged.length; + report(makeResponse(407)); + report(makeResponse(407)); + expect(logged.length).toBe(afterFirst); + const rendered = logged.join('|'); + expect(rendered).toContain('event=proxy.challenge.unanswered@warning'); + expect(rendered).not.toContain(SECRET); + expect(rendered).not.toContain('minted-token'); + }); + + test('a logger that throws never fails the request it was describing (OBS-20)', () => { + setGlobalLogger({ + atLevel: () => { + throw new Error('logger exploded'); + }, + withContext: () => getGlobalLogger(), + }); + const report = createProxyChallengeReporter(proxyWithHandler()); + expect(() => { + report(makeResponse(407)); + }).not.toThrow(); + expect(() => { + warnIfCustomChallengeHandler(proxyWithHandler()); + }).not.toThrow(); + }); +}); diff --git a/packages/transport-undici/src/challenge-handler.ts b/packages/transport-undici/src/challenge-handler.ts new file mode 100644 index 0000000..778aaa2 --- /dev/null +++ b/packages/transport-undici/src/challenge-handler.ts @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: MIT +// packages/transport-undici/src/challenge-handler.ts +import { + getGlobalLogger, + type LogEvent, + type ProxyOptions, + type Response, +} from '@dexpace/core'; + +/** OBS-20: a logger failure must never fail the request it was describing. */ +function safeWarn(event: string, decorate: (entry: LogEvent) => void): void { + try { + const entry = getGlobalLogger().atLevel('warning').event(event); + decorate(entry); + entry.emit(); + } catch { + // Deliberately swallowed -- see OBS-20. + } +} + +/** Names the proxy without ever rendering its credentials (TRANSPORT-30, redaction rules). */ +function describeProxy(entry: LogEvent, proxy: ProxyOptions): LogEvent { + return entry.field('proxy.host', proxy.host).field('proxy.port', proxy.port); +} + +function hasCustomChallengeHandler(proxy: ProxyOptions | undefined): boolean { + return proxy !== undefined && typeof proxy.challengeHandler === 'function'; +} + +/** + * TRANSPORT-30's discoverability clause, at construction: undici cannot dispatch a custom + * (non-Basic) proxy challenge handler at all, so a configured one is surfaced with a WARN rather + * than silently ignored. + * + * The reason is a hard constraint of the native client, not a gap in this package: `ProxyAgent` + * rejects a per-request `Proxy-Authorization` header with `InvalidArgumentError` — it was removed + * deliberately as a security fix — and takes its credential only from its own constructor, which + * runs before any challenge has been seen. There is therefore no point at which a handler-minted + * credential could be applied to the exchange that provoked it. Proxy auth falls back to Basic: + * `ProxyOptions.credentials`, which this transport does pass to the `ProxyAgent` constructor. + * + * @param proxy - the configured proxy, if any. + * + * @internal + */ +export function warnIfCustomChallengeHandler( + proxy: ProxyOptions | undefined, +): void { + if (proxy === undefined || !hasCustomChallengeHandler(proxy)) return; + safeWarn('proxy.challengeHandler.unsupported', entry => { + describeProxy(entry, proxy).field( + 'detail', + 'undici takes proxy credentials only from the ProxyAgent constructor and rejects a ' + + 'per-request Proxy-Authorization header, so a custom challenge handler cannot be ' + + 'dispatched; proxy auth falls back to Basic (ProxyOptions.credentials)', + ); + }); +} + +/** + * Builds the per-transport reporter for TRANSPORT-30's second discoverability moment: the first time + * a proxy actually answers 407 while an undispatchable challenge handler is configured. + * + * Only a 407 is reported. A 401 is an *origin-server* challenge, and nothing about proxy credentials + * belongs anywhere near it — the spec makes that an explicit MUST NOT, so it is a guard here rather + * than an accident of control flow. The credential itself is never logged on any path; the 407 is + * returned to the caller untouched, for its own auth layer to act on. + * + * @param proxy - the configured proxy, if any. + * @returns a reporter to call with each adapted response; warns at most once per transport. + * + * @internal + */ +export function createProxyChallengeReporter( + proxy: ProxyOptions | undefined, +): (response: Response) => void { + if (proxy === undefined || !hasCustomChallengeHandler(proxy)) { + return () => undefined; + } + let reported = false; + return (response: Response) => { + if (response.status.code !== 407 || reported) return; + reported = true; + safeWarn('proxy.challenge.unanswered', entry => { + describeProxy(entry, proxy).field( + 'detail', + 'the proxy issued a 407 and the configured challenge handler cannot be dispatched; ' + + 'the response is surfaced unchanged for the caller’s own auth layer', + ); + }); + }; +} diff --git a/packages/transport-undici/src/index.ts b/packages/transport-undici/src/index.ts new file mode 100644 index 0000000..6a6f233 --- /dev/null +++ b/packages/transport-undici/src/index.ts @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: MIT +// packages/transport-undici/src/index.ts +export {undiciTransport} from './undici-transport.js'; +export type {UndiciTransportOptions} from './undici-transport.js'; diff --git a/packages/transport-undici/src/undici-transport.conformance.test.ts b/packages/transport-undici/src/undici-transport.conformance.test.ts new file mode 100644 index 0000000..866b18f --- /dev/null +++ b/packages/transport-undici/src/undici-transport.conformance.test.ts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +// packages/transport-undici/src/undici-transport.conformance.test.ts +// Runs the shared TRANSPORT-N suite (@dexpace/transport-conformance) against undiciTransport(). +import {runTransportConformanceSuite} from '@dexpace/transport-conformance'; +import {undiciTransport} from './undici-transport.js'; + +runTransportConformanceSuite('undiciTransport', () => undiciTransport(), { + supportsInternalCancel: true, + supportsProxy: true, + // TRANSPORT-11's own note: an undici-class transport forwards `Connection` rather than dropping it. + dropsConnectionHeader: false, +}); diff --git a/packages/transport-undici/src/undici-transport.test.ts b/packages/transport-undici/src/undici-transport.test.ts new file mode 100644 index 0000000..ea8f166 --- /dev/null +++ b/packages/transport-undici/src/undici-transport.test.ts @@ -0,0 +1,467 @@ +// SPDX-License-Identifier: MIT +// packages/transport-undici/src/undici-transport.test.ts +// Exercises: TRANSPORT-2 (no redirect interceptor is composed), TRANSPORT-8 (a native-internal cancel +// is terminal while a timeout stays retryable), TRANSPORT-11 (undici keeps `Connection`), +// TRANSPORT-15/16 (ownership-aware, idempotent close), TRANSPORT-22 (an adaptation throw destroys the +// native body), TRANSPORT-20 (a permanent argument error is terminal, a no-response failure is +// retryable), TRANSPORT-28 (a file body dispatches its declared byte range), SEAM-14 +import {createRequire} from 'node:module'; +import {mkdtemp, rm, writeFile} from 'node:fs/promises'; +import {createServer, type Server} from 'node:http'; +import {tmpdir} from 'node:os'; +import {join} from 'node:path'; +import {afterAll, beforeAll, describe, expect, test} from 'bun:test'; +import { + createProxyOptions, + getGlobalLogger, + Headers, + Request, + RequestOptions, + IoError, + setGlobalLogger, + TransportFailureError, + type FileBodyDescriptor, + type Logger, +} from '@dexpace/core'; +import type {Dispatcher} from 'undici'; +import {undiciTransport} from './undici-transport.js'; + +const require = createRequire(import.meta.url); +const undici = require('undici/index.js') as typeof import('undici'); + +/** + * Awaits `pending` and hands back its rejection reason. `expect(p).rejects.…` is typed `void` here, + * so this keeps the assertion ordered with whatever the row checks afterwards. + */ +async function rejection(pending: Promise): Promise { + try { + await pending; + } catch (error) { + return error; + } + throw new Error('expected the promise to reject, but it resolved'); +} + +/** Installs a logger that records every dropped header name, and returns the restore function. */ +function captureDroppedHeaders(): { + dropped: string[]; + restore: () => void; +} { + const dropped: string[] = []; + const previous = getGlobalLogger(); + const capturing: Logger = { + atLevel: () => { + let name: string | undefined; + const entry = { + field: (key: string, value: unknown) => { + if (key === 'header') name = String(value); + return entry; + }, + event: () => entry, + cause: () => entry, + emit: () => { + if (name !== undefined) dropped.push(name); + }, + }; + return entry; + }, + withContext: () => capturing, + }; + setGlobalLogger(capturing); + return { + dropped, + restore: () => { + setGlobalLogger(previous); + }, + }; +} + +/** Records every request body the server received, so a file body's byte range is checkable. */ +let server: Server; +let origin: string; +const received: string[] = []; + +beforeAll(async () => { + server = createServer((req, res) => { + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer) => chunks.push(chunk)); + req.on('end', () => { + received.push(Buffer.concat(chunks).toString('utf8')); + if (req.url === '/slow') return; // never answers -- the in-flight fixture + res.writeHead(200, {'content-type': 'text/plain'}); + res.end('ok'); + }); + }); + await new Promise(done => { + server.listen(0, '127.0.0.1', done); + }); + const address = server.address(); + const port = + typeof address === 'object' && address !== null ? address.port : 0; + origin = `http://127.0.0.1:${String(port)}`; +}); + +afterAll(async () => { + server.closeAllConnections(); + await new Promise(done => { + server.close(() => { + done(); + }); + }); +}); + +describe('undiciTransport construction and ownership', () => { + test('SEAM-14: a bring-your-own dispatcher is never closed by the transport', async () => { + let closed = false; + const byo = { + request: () => Promise.reject(new Error('not dispatched in this test')), + close: () => { + closed = true; + return Promise.resolve(); + }, + destroy: () => Promise.resolve(), + } as unknown as Dispatcher; + + const transport = undiciTransport({dispatcher: byo}); + await transport.close(); + await transport.close(); + expect(closed).toBe(false); + }); + + test('TRANSPORT-15/16: an owned agent is closed, idempotently', async () => { + const transport = undiciTransport({agentOptions: {connections: 1}}); + await transport.close(); + // Reaching the next line proves the second close neither threw nor hung (TRANSPORT-16). + await transport.close(); + }); + + test('a transport-constructed ProxyAgent is owned and released too', async () => { + const transport = undiciTransport({ + proxy: createProxyOptions({type: 'http', host: '127.0.0.1', port: 3128}), + }); + // The ProxyAgent is SDK-created, so close() must release it -- the bug this guards is closing + // only the separately-constructed direct Agent and leaking the ProxyAgent actually in use. + await transport.close(); + await transport.close(); + }); + + test('supplying both a dispatcher and a proxy fails loudly at construction', () => { + const agent = new undici.Agent(); + expect(() => + undiciTransport({ + dispatcher: agent, + proxy: createProxyOptions({type: 'http', host: 'proxy', port: 8080}), + }), + ).toThrow(TypeError); + void agent.close(); + }); +}); + +describe('undiciTransport dispatch', () => { + test('TRANSPORT-2/11: redirects are pinned off and Connection is forwarded, not dropped', async () => { + const dispatched: Dispatcher.RequestOptions[] = []; + const recorder = { + request: (options: Dispatcher.RequestOptions) => { + dispatched.push(options); + return Promise.resolve({ + statusCode: 200, + headers: {}, + body: { + destroy: () => undefined, + [Symbol.asyncIterator]: () => ({ + next: () => Promise.resolve({done: true, value: undefined}), + }), + }, + } as unknown as Dispatcher.ResponseData); + }, + close: () => Promise.resolve(), + } as unknown as Dispatcher; + + const transport = undiciTransport({dispatcher: recorder}); + const request = Request.newBuilder() + .url(`${origin}/anything?q=1`) + .headers( + Headers.newBuilder() + .set('Connection', 'keep-alive') + .set('Content-Length', '999') + .build(), + ) + .build(); + await (await transport.send(request)).close(); + + const sent = dispatched[0]; + expect(sent?.maxRedirections).toBe(0); + expect(sent?.path).toBe('/anything?q=1'); + const headers = sent?.headers as string[]; + expect(headers).toContain('Connection'); + expect(headers).not.toContain('Content-Length'); + }); +}); + +describe('undiciTransport body and adaptation paths', () => { + test('TRANSPORT-28: a file body dispatches exactly its declared byte range', async () => { + const dir = await mkdtemp(join(tmpdir(), 'undici-file-body-')); + try { + const path = join(dir, 'payload.bin'); + await writeFile(path, 'ABCDEFGH'); + // The structural recognition contract, built by hand: this package must narrow on + // `kind === 'file'` alone, never on an instanceof against @dexpace/body-file, which it + // deliberately does not depend on. + const descriptor: FileBodyDescriptor = { + kind: 'file', + mediaType: 'application/octet-stream', + contentLength: 4, + replayable: true, + path, + start: 2, + count: 4, + writeTo: () => + Promise.reject(new Error('the transport must not call writeTo here')), + }; + const transport = undiciTransport(); + const request = Request.newBuilder() + .method('POST') + .url(`${origin}/upload`) + .body(descriptor) + .build(); + received.length = 0; + await (await transport.send(request)).close(); + await transport.close(); + expect(received[0]).toBe('CDEF'); + } finally { + await rm(dir, {recursive: true, force: true}); + } + }); + + test('a zero-count file body dispatches as an empty body, not a stream error', async () => { + const dir = await mkdtemp(join(tmpdir(), 'undici-empty-file-body-')); + try { + const path = join(dir, 'payload.bin'); + await writeFile(path, 'ABCDEFGH'); + // createReadStream throws ERR_OUT_OF_RANGE the moment `end` falls below `start`, which is what + // `start + count - 1` computes for count 0 -- the empty range needs its own branch. + const descriptor: FileBodyDescriptor = { + kind: 'file', + mediaType: 'application/octet-stream', + contentLength: 0, + replayable: true, + path, + start: 4, + count: 0, + writeTo: () => Promise.resolve(), + }; + const transport = undiciTransport(); + received.length = 0; + const response = await transport.send( + Request.newBuilder() + .method('POST') + .url(`${origin}/upload`) + .body(descriptor) + .build(), + ); + await response.close(); + await transport.close(); + expect(received[0]).toBe(''); + } finally { + await rm(dir, {recursive: true, force: true}); + } + }); +}); + +describe('undiciTransport request-body failures', () => { + test('a body that cannot be written fails the send as a transport failure', async () => { + const transport = undiciTransport(); + const request = Request.newBuilder() + .method('POST') + .url(`${origin}/upload`) + .body({ + kind: 'byte-array', + mediaType: 'text/plain', + contentLength: 3, + replayable: true, + writeTo: () => Promise.reject(new Error('body exploded')), + }) + .build(); + // Classified the same way the streaming branch classifies the same failure, cause intact. + expect(await rejection(transport.send(request))).toMatchObject({ + name: 'TransportFailureError', + cause: {message: 'body exploded'}, + }); + await transport.close(); + }); + + test('TRANSPORT-22: an adaptation throw destroys the native body before propagating', async () => { + let destroyed = false; + const hostile = { + get statusCode(): number { + throw new Error('adaptation exploded'); + }, + headers: {}, + body: { + destroy: () => { + destroyed = true; + }, + }, + } as unknown as Dispatcher.ResponseData; + + const transport = undiciTransport({ + dispatcher: { + request: () => Promise.resolve(hostile), + close: () => Promise.resolve(), + } as unknown as Dispatcher, + }); + const request = Request.newBuilder().url(`${origin}/anything`).build(); + expect(await rejection(transport.send(request))).toMatchObject({ + message: 'adaptation exploded', + }); + expect(destroyed).toBe(true); + }); +}); + +describe('undiciTransport failure classification (TRANSPORT-20)', () => { + test('an argument undici can never accept is terminal, not a retryable failure', async () => { + // The drop set that removes Proxy-Authorization is chosen from `options.proxy`, so a BYO + // ProxyAgent leaves the header in place and ProxyAgent.dispatch rejects it outright. That is a + // permanent misconfiguration: classifying it as TransportFailureError would make it an IoError, + // and classify.ts returns true for every IoError -- a caller's whole retry budget spent + // re-proving the same rejection. It is reported outside the IoError tree instead. + const agent = new undici.ProxyAgent({uri: 'http://127.0.0.1:1/'}); + const transport = undiciTransport({dispatcher: agent}); + try { + const request = Request.newBuilder() + .url('http://example.invalid/') + .headers( + Headers.newBuilder() + .set('Proxy-Authorization', 'Basic Zm9vOmJhcg==') + .build(), + ) + .build(); + const error = await rejection(transport.send(request)); + expect(error).toBeInstanceOf(TypeError); + // Outside the IoError tree is the whole point: classify.ts's allow-list returns true for + // every IoError and false for anything it was never opted into (RETRY-2). + expect(error).not.toBeInstanceOf(IoError); + expect((error as {cause?: {code?: string}}).cause?.code).toBe( + 'UND_ERR_INVALID_ARG', + ); + } finally { + await transport.close(); + await agent.close(); + } + }); + + test('a genuine network failure stays the retryable TransportFailureError', async () => { + // The twin of the row above: the catch-all branch must keep classifying a no-response failure + // as retryable, so narrowing it did not turn every dispatch error terminal. + const transport = undiciTransport(); + try { + const request = Request.newBuilder().url('http://127.0.0.1:1/').build(); + const error = await rejection(transport.send(request)); + expect(error).toBeInstanceOf(TransportFailureError); + expect(error).toBeInstanceOf(IoError); + } finally { + await transport.close(); + } + }); +}); + +describe('undiciTransport proxy dispatch (TRANSPORT-30)', () => { + test('a per-request Proxy-Authorization is dropped when a proxy is configured', async () => { + // ProxyAgent.dispatch throws InvalidArgumentError on ANY per-request Proxy-Authorization -- a + // deliberate undici security fix -- so forwarding one would turn every proxied send into a hard + // failure. It is dropped instead, and the drop log is what keeps that discoverable + // (TRANSPORT-11/12/30). + const {dropped, restore} = captureDroppedHeaders(); + const transport = undiciTransport({ + headerDropLogging: 'all', + proxy: createProxyOptions({ + type: 'http', + host: '127.0.0.1', + port: 1, + nonProxyHosts: ['127.0.0.1'], + }), + }); + try { + const response = await transport.send( + Request.newBuilder() + .url(`${origin}/anything`) + .headers( + Headers.newBuilder() + .set('Proxy-Authorization', 'Basic stale') + .build(), + ) + .build(), + ); + await response.close(); + expect(dropped).toContain('proxy-authorization'); + } finally { + restore(); + await transport.close(); + } + }); + + test('a proxied transport routes a NO_PROXY host over its direct agent', async () => { + const transport = undiciTransport({ + proxy: createProxyOptions({ + type: 'http', + host: '127.0.0.1', + port: 1, + nonProxyHosts: ['127.0.0.1'], + }), + }); + // Port 1 is a dead proxy: reaching the fixture at all proves the bypass routed direct. + const response = await transport.send( + Request.newBuilder().url(`${origin}/anything`).build(), + ); + expect(response.status.code).toBe(200); + await response.close(); + await transport.close(); + }); + + test('asyncDispose is the same teardown as close', async () => { + const transport = undiciTransport(); + await transport[Symbol.asyncDispose](); + await transport.close(); + }); +}); + +describe('undiciTransport cancellation (TRANSPORT-8)', () => { + test('TRANSPORT-16: close does not wait out an in-flight request', async () => { + const transport = undiciTransport(); + const pending = rejection( + transport.send(Request.newBuilder().url(`${origin}/slow`).build()), + ); + await new Promise(resolve => setTimeout(resolve, 25)); + const startedClosing = Date.now(); + await transport.close(); + // The fixture holds /slow open forever; a graceful close would block here until it gave up. + expect(Date.now() - startedClosing).toBeLessThan(1_000); + expect(await pending).toMatchObject({name: 'CancellationError'}); + }); + + test('destroying the dispatcher mid-flight is terminal, not a retryable failure', async () => { + const agent = new undici.Agent(); + const transport = undiciTransport({dispatcher: agent}); + const pending = transport.send( + Request.newBuilder().url(`${origin}/slow`).build(), + ); + // Give the request time to actually reach the socket before tearing the client down. + await new Promise(resolve => setTimeout(resolve, 25)); + await agent.destroy(); + expect(await rejection(pending)).toMatchObject({ + name: 'CancellationError', + }); + }); + + test('a timeout on the same path stays retryable', async () => { + const transport = undiciTransport(); + const pending = transport.send( + Request.newBuilder().url(`${origin}/slow`).build(), + RequestOptions.newBuilder().timeoutMs(40).build(), + ); + expect(await rejection(pending)).toMatchObject({ + name: 'TransportFailureError', + }); + await transport.close(); + }); +}); diff --git a/packages/transport-undici/src/undici-transport.ts b/packages/transport-undici/src/undici-transport.ts new file mode 100644 index 0000000..9d0a815 --- /dev/null +++ b/packages/transport-undici/src/undici-transport.ts @@ -0,0 +1,533 @@ +// SPDX-License-Identifier: MIT +// packages/transport-undici/src/undici-transport.ts +import {createReadStream} from 'node:fs'; +import {createRequire} from 'node:module'; +import {Readable} from 'node:stream'; +import type {ReadableStream as NodeReadableStream} from 'node:stream/web'; +import { + CancellationError, + composeSignal, + Protocol, + Response, + shouldBypassProxy, + Status, + TransportFailureError, + type Body, + type FileBodyDescriptor, + type ProxyOptions, + type Request, + type RequestOptions, + type Transport, +} from '@dexpace/core'; +import { + abortToSdkError, + createDropLogger, + degradeInboundHeaders, + forkSignal, + isMaterializable, + mapOutboundHeaders, + materializeBody, + producerFailure, + pumpBody, + type BodyPump, + type ForkedSignal, + type HeaderDropLogging, +} from '@dexpace/transport-shared'; +import type {Agent, Dispatcher, ProxyAgent} from 'undici'; +import { + createProxyChallengeReporter, + warnIfCustomChallengeHandler, +} from './challenge-handler.js'; + +/** + * `undici` is loaded through `createRequire`, not a static `import`, because Bun resolves the bare + * specifier `undici` to its own built-in shim: the shim's `Agent` constructs but has no `request` + * method, so every dispatch under `bun test` would fail with a `TypeError` instead of reaching the + * wire. Requiring the real package's entry file by path bypasses that alias and resolves identically + * under plain Node. The types still come from the static `import type` above, so this stays fully + * checked. Revisit when Bun's shim implements `Dispatcher.request`, or if undici ever adds an + * `exports` map that hides `index.js` (this package pins `^6`, which has neither). + */ +const require = createRequire(import.meta.url); +const undici = require('undici/index.js') as typeof import('undici'); + +/** + * TRANSPORT-11's outbound drop set for this transport. `connection` is deliberately absent — §17's + * own note is that an undici-class transport forwards it rather than dropping it. + */ +const UNDICI_FORBIDDEN_HEADERS: readonly string[] = [ + 'content-length', + 'host', + 'transfer-encoding', +]; + +/** + * The drop set when this transport owns a `ProxyAgent`. `ProxyAgent.dispatch` throws + * `InvalidArgumentError` on *any* per-request `Proxy-Authorization` — a deliberate undici security + * fix, not an oversight — so forwarding one turns every proxied send into a hard failure. Dropping + * it degrades one header instead (TRANSPORT-12) and, because every drop is logged by name, keeps the + * limitation discoverable rather than silent (TRANSPORT-11/13, TRANSPORT-30). + */ +const UNDICI_PROXIED_FORBIDDEN_HEADERS: readonly string[] = [ + ...UNDICI_FORBIDDEN_HEADERS, + 'proxy-authorization', +]; + +/** Bodies at or below this declared length are buffered rather than streamed; see the fetch twin. */ +const MAX_MATERIALIZED_BODY_BYTES = 1_000_000; + +/** + * Options for {@link undiciTransport}. + * + * @public + */ +export interface UndiciTransportOptions { + /** + * A bring-your-own `Dispatcher`. It is used as-is and **never** closed by this transport + * (SEAM-14); supplying it together with `proxy` is a construction-time error. + */ + readonly dispatcher?: Dispatcher; + /** Proxy configuration; the transport constructs and owns the resulting `ProxyAgent`. */ + readonly proxy?: ProxyOptions; + /** How dropped header names are logged (TRANSPORT-13); defaults to `'first-per-name'`. */ + readonly headerDropLogging?: HeaderDropLogging; + /** A timeout applied to every call that supplies no `RequestOptions.timeoutMs` of its own. */ + readonly defaultTimeoutMs?: number; + /** `Agent` options, used only when no `dispatcher` is supplied. */ + readonly agentOptions?: Agent.Options; +} + +/** The dispatcher pair one transport routes over, plus the subset it owns and must close. */ +interface DispatcherSet { + /** Where a non-bypassed request goes; identical to `direct` when no proxy is configured. */ + readonly proxied: Dispatcher; + /** Where a `shouldBypassProxy` host goes, so `NO_PROXY` is honored rather than tunnelled. */ + readonly direct: Dispatcher; + /** Dispatchers this transport constructed; empty for a caller-supplied one (SEAM-14). */ + readonly owned: readonly Dispatcher[]; +} + +/** + * The proxy URI plus its Basic credential, kept apart. `formatProxyOptions` is deliberately *not* + * used here: it masks credentials as `***:***` for logging, and feeding that to `ProxyAgent` would + * authenticate with the literal mask (TRANSPORT-30 — credentials must not leak, and must still work). + */ +function toProxyAgentOptions(proxy: ProxyOptions): ProxyAgent.Options { + // `host` is stored bare, so an IPv6 literal needs its brackets back before it can be a URL authority. + const host = proxy.host.includes(':') ? `[${proxy.host}]` : proxy.host; + const uri = `${proxy.type}://${host}:${String(proxy.port)}`; + if (proxy.credentials === undefined) return {uri}; + const raw = `${proxy.credentials.username}:${proxy.credentials.password}`; + return {uri, token: `Basic ${Buffer.from(raw).toString('base64')}`}; +} + +/** + * One exclusive decision, made once, fixing both the dispatcher pair and its ownership. Supplying + * both `dispatcher` and `proxy` fails loudly rather than silently picking one: a BYO dispatcher may + * already be a `ProxyAgent`, and ignoring either option hides which is in force. + */ +function selectDispatchers(options: UndiciTransportOptions): DispatcherSet { + if (options.dispatcher !== undefined && options.proxy !== undefined) { + throw new TypeError( + 'supply either `dispatcher` or `proxy`, not both: a bring-your-own dispatcher may already be ' + + 'a ProxyAgent, and silently ignoring one of the two hides which is in force', + ); + } + if (options.dispatcher !== undefined) { + const byo = options.dispatcher; + return {proxied: byo, direct: byo, owned: []}; + } + // Agent, not Pool: a Pool is bound to one origin at construction, but a general-purpose Transport + // must reach whatever origin each Request names. + const direct = new undici.Agent(options.agentOptions); + if (options.proxy === undefined) + return {proxied: direct, direct, owned: [direct]}; + const proxied = new undici.ProxyAgent(toProxyAgentOptions(options.proxy)); + return {proxied, direct, owned: [proxied, direct]}; +} + +/** undici's flat `[name, value, name, value, ...]` form -- the only shape that keeps a repeated name repeated (HTTP-14). */ +function toUndiciHeaders( + request: Request, + forbidden: readonly string[], + logDrops: (dropped: readonly string[]) => void, +): string[] { + const {sent, dropped} = mapOutboundHeaders(request.headers, forbidden, { + bodyDerivedMediaType: request.body?.mediaType, + }); + logDrops(dropped); + return [...sent.entries()].flat(); +} + +/** + * undici's own codes for "this exchange was torn down from inside the client", as opposed to a + * network failure. TRANSPORT-8 requires the two be told apart: a destroyed dispatcher is terminal + * (nothing about retrying it can succeed — the client is gone), while a timeout on the same code + * path stays retryable. Reached only after the caller-signal branch, so a caller abort and a + * per-call timeout are already classified by then. + */ +const NATIVE_CANCEL_CODES: ReadonlySet = new Set([ + 'UND_ERR_DESTROYED', + 'UND_ERR_ABORTED', + 'UND_ERR_CLOSED', +]); + +/** + * undici's codes for "these arguments can never work", as opposed to "this exchange failed". Both are + * raised by argument validation and are perfectly reproducible, so classifying them as + * `TransportFailureError` would hand `classify.ts` an always-retryable verdict (it returns `true` for + * every `IoError`) and spend a caller's whole retry budget re-proving a permanent misconfiguration. + * The commonest way to reach one is a bring-your-own `ProxyAgent` plus a per-request + * `Proxy-Authorization`: `UNDICI_PROXIED_FORBIDDEN_HEADERS` only drops that header when this + * transport constructed the proxy itself, so with a BYO dispatcher it reaches `dispatch` and is + * rejected outright. + */ +const TERMINAL_ARGUMENT_CODES: ReadonlySet = new Set([ + 'UND_ERR_INVALID_ARG', + 'UND_ERR_NOT_SUPPORTED', +]); + +function errorCode(error: unknown): string | undefined { + const code = (error as {code?: unknown} | null | undefined)?.code; + return typeof code === 'string' ? code : undefined; +} + +function isNativeCancel(error: unknown): boolean { + const code = errorCode(error); + return code !== undefined && NATIVE_CANCEL_CODES.has(code); +} + +/** + * Maps one dispatch failure onto the SDK's error vocabulary. Extracted from `#dispatch` so the four + * branches read as one classification table rather than as control flow wrapped around a call. + * + * @param error - whatever the dispatch rejected with. + * @param signal - the forked signal the dispatch was given, if any. + * @returns the error to throw; never returns normally without one. + */ +function toDispatchError( + error: unknown, + signal: AbortSignal | undefined, +): Error { + if (signal?.aborted) return abortToSdkError(signal, error); + if (isNativeCancel(error)) { + // TRANSPORT-8: terminal, never retryable -- the dispatcher this send was routed over no longer + // exists, so a retry over it cannot succeed. + return new CancellationError('undici dispatcher was destroyed', { + cause: error, + }); + } + const code = errorCode(error); + if (code !== undefined && TERMINAL_ARGUMENT_CODES.has(code)) { + // Deliberately outside the IoError tree: `classify.ts` is an allow-list, so anything that is not + // an IoError, a timeout, or a retryable status is non-retryable for free (RETRY-2). `TypeError` + // matches `selectDispatchers`, which already reports a caller misconfiguration that way. + return new TypeError( + error instanceof Error + ? error.message + : 'undici rejected the request arguments', + {cause: error}, + ); + } + return new TransportFailureError( + error instanceof Error ? error.message : 'undici dispatch failed', + {cause: error}, + ); +} + +/** What undici accepts as a request body; `undefined` is not one of them, `null` is. */ +type UndiciBody = Exclude; + +/** A request body prepared for one dispatch, plus the teardown an abandoned producer is owed. */ +interface PreparedBody { + readonly init: UndiciBody; + readonly pump: BodyPump | undefined; +} + +/** + * TRANSPORT-28's recognition contract, in one named place: a plain string-literal check, never a + * cross-package `instanceof` against `@dexpace/body-file` (which this package does not depend on). + * `Body.kind` is a union on one interface rather than a discriminated union of interfaces, so the + * narrowing has to be spelled out as a predicate. + */ +function isFileBody(body: Body): body is FileBodyDescriptor { + return body.kind === 'file'; +} + +async function prepareBody(body: Body | undefined): Promise { + if (body === undefined) return {init: null, pump: undefined}; + if (isFileBody(body)) { + // An empty range is not a degenerate read stream: `createReadStream` throws ERR_OUT_OF_RANGE the + // moment `end` (start + count - 1) falls below `start`, so a zero-count file body has to become + // an explicit empty body rather than a stream nobody can open. + if (body.count === 0) return {init: new Uint8Array(0), pump: undefined}; + // TRANSPORT-28: dispatch straight off the file, honoring start/count, rather than routing the + // bytes through a userspace TransformStream first. The closest available approximation of the + // reference's zero-copy path -- see the Deviation Ledger for why a literal one does not exist. + return { + init: createReadStream(body.path, { + start: body.start, + end: body.start + body.count - 1, + }), + pump: undefined, + }; + } + if (isMaterializable(body, MAX_MATERIALIZED_BODY_BYTES)) { + try { + return {init: await materializeBody(body), pump: undefined}; + } catch (error) { + // Same classification the streaming branch gives the same failure -- see the fetch twin. + throw new TransportFailureError('request body could not be written', { + cause: error, + }); + } + } + const pump = pumpBody(body); + return { + init: Readable.fromWeb(pump.readable as unknown as NodeReadableStream), + pump, + }; +} + +/** + * Wraps undici's body in a web stream that reads only when pulled. + * + * Deliberately not `Readable.toWeb`: Bun's adapter keeps enqueuing after the controller closes and + * throws `ERR_INVALID_STATE` the moment a response is closed without being fully read — which is + * exactly TRANSPORT-25's close-without-reading path. Deliberately not a `start()` that attaches a + * `'data'` listener either: that switches the Node stream into flowing mode and buffers the whole + * body eagerly, defeating the same requirement from the other side. Async iteration is pull-based, + * so a chunk is read only when the consumer asks, and `cancel` destroys the underlying body, which + * is what returns the connection to the pool. + */ +function toDemandDrivenStream(body: Readable): ReadableStream { + // `undefined` as the return type, not the default `any`: the done-result's `value` would + // otherwise destructure as `any` and defeat the type-aware lint rules. + const chunks = body[Symbol.asyncIterator]() as AsyncIterator< + Uint8Array, + undefined + >; + return new ReadableStream({ + async pull(controller) { + try { + const {done, value} = await chunks.next(); + if (done) controller.close(); + else controller.enqueue(value); + } catch (error) { + controller.error(error); + } + }, + cancel(reason) { + body.destroy(reason instanceof Error ? reason : undefined); + }, + }); +} + +function adaptResponse( + request: Request, + result: Dispatcher.ResponseData, + logDrops: (dropped: readonly string[]) => void, +): Response { + const raw: [string, string][] = []; + for (const [name, value] of Object.entries(result.headers)) { + if (value === undefined) continue; + // An array means a genuinely repeated header (Set-Cookie); keep each value its own entry. + if (Array.isArray(value)) for (const each of value) raw.push([name, each]); + else raw.push([name, value]); + } + const {headers, dropped} = degradeInboundHeaders(raw); + logDrops(dropped); + + return ( + Response.newBuilder() + .request(request) + // A documented best-effort default: undici's ResponseData does not surface the negotiated HTTP + // version any more than the WHATWG Response does (Deviation Ledger). + .protocol(Protocol.HTTP_1_1) + .status(Status.of(result.statusCode)) + .headers(headers) + .body(toDemandDrivenStream(result.body)) + .build() + ); +} + +/** Everything one dispatch needs that is not the request itself; keeps `max-params` at three. */ +interface DispatchContext { + readonly headers: string[]; + readonly body: UndiciBody; + /** The forked signal handed to undici; detached by `send` the moment the response is delivered. */ + readonly fork: ForkedSignal; +} + +class UndiciTransport implements Transport, AsyncDisposable { + readonly #dispatchers: DispatcherSet; + readonly #proxy: ProxyOptions | undefined; + readonly #logDrops: (dropped: readonly string[]) => void; + readonly #defaultTimeoutMs: number | undefined; + readonly #forbiddenHeaders: readonly string[]; + readonly #reportProxyChallenge: (response: Response) => void; + #closing: Promise | undefined; + + constructor(options: UndiciTransportOptions) { + this.#dispatchers = selectDispatchers(options); + this.#proxy = options.proxy; + this.#logDrops = createDropLogger( + options.headerDropLogging ?? 'first-per-name', + ); + this.#defaultTimeoutMs = options.defaultTimeoutMs; + this.#forbiddenHeaders = + options.proxy === undefined + ? UNDICI_FORBIDDEN_HEADERS + : UNDICI_PROXIED_FORBIDDEN_HEADERS; + this.#reportProxyChallenge = createProxyChallengeReporter(options.proxy); + // TRANSPORT-30: undici cannot dispatch a custom challenge handler at all, so the limitation is + // surfaced up front rather than discovered on a 407. + warnIfCustomChallengeHandler(options.proxy); + } + + async send( + request: Request, + options?: RequestOptions, + signal?: AbortSignal, + ): Promise { + const composed = composeSignal( + signal, + options?.timeoutMs ?? this.#defaultTimeoutMs, + ); + if (composed?.aborted) throw abortToSdkError(composed, composed.reason); + + const prepared = await prepareBody(request.body); + // Dispatched with a fork the caller cannot reach: cancellation stays live for the whole in-flight + // window and goes inert the moment the response is handed over (SEAM-16). + const context: DispatchContext = { + headers: toUndiciHeaders(request, this.#forbiddenHeaders, this.#logDrops), + body: prepared.init, + fork: forkSignal(composed), + }; + try { + return await this.#exchange(request, context, prepared.pump); + } finally { + context.fork.detach(); + } + } + + async #exchange( + request: Request, + context: DispatchContext, + pump: BodyPump | undefined, + ): Promise { + const result = await this.#dispatch(request, context, pump); + // The fork, not the caller's signal: it mirrors the source for as long as it stays attached, + // which is exactly the in-flight window this check is about. + const dispatched = context.fork.signal; + + if (dispatched?.aborted) { + // TRANSPORT-9 / SEAM-30: this response will never reach a caller, so this producer closes it. + await result.body.dump().catch(() => undefined); + await pump?.abandon(dispatched.reason); + throw abortToSdkError(dispatched, dispatched.reason); + } + + try { + // TRANSPORT-22: a live socket is in hand, so any throw here must release it before propagating. + const response = adaptResponse(request, result, this.#logDrops); + this.#reportProxyChallenge(response); + return response; + } catch (error) { + result.body.destroy(); + // TRANSPORT-19: nothing is delivered on this path either, so the producer is owed its teardown + // exactly as on the abort branch above. + await pump?.abandon(error); + throw error; + } + } + + async #dispatch( + request: Request, + context: DispatchContext, + pump: BodyPump | undefined, + ): Promise { + const dispatcher = + this.#proxy !== undefined && + shouldBypassProxy(this.#proxy, request.url.hostname) + ? this.#dispatchers.direct + : this.#dispatchers.proxied; + try { + // Raced, not sequenced, for the same two reasons as the fetch twin: a producer failure must + // surface even while undici is still pending, and -- because the race keeps a handler on + // `done` after it settles -- a producer that fails *after* delivery is an observed rejection + // rather than one that reaches Node's default `unhandledRejection` policy (TRANSPORT-19). + return await Promise.race([ + dispatcher.request({ + origin: request.url.origin, + path: `${request.url.pathname}${request.url.search}`, + method: request.method, + headers: context.headers, + body: context.body, + // `?? null` rather than an omitted key: `exactOptionalPropertyTypes` makes an explicit + // `undefined` a distinct, rejected value here, and undici reads `null` as "no signal". + signal: context.fork.signal ?? null, + // TRANSPORT-1: pinned explicitly rather than inherited -- a BYO dispatcher may carry a + // redirect interceptor, and the pipeline is the single redirect authority. + maxRedirections: 0, + }), + producerFailure(pump?.done), + ]); + } catch (error) { + await pump?.abandon(error); + throw toDispatchError(error, context.fork.signal); + } + } + + /** + * Releases every dispatcher this transport constructed, in reverse acquisition order, and never a + * caller-supplied one (SEAM-14, TRANSPORT-15). Idempotent, and concurrent calls share one + * teardown (TRANSPORT-16). + * + * `destroy()`, not undici's graceful `close()`: TRANSPORT-16 requires a non-blocking shutdown with + * no unbounded await, and `close()` waits for every enqueued request to finish — one in-flight send + * against a slow peer would stall teardown for that peer's whole timeout. Sends still in flight + * therefore reject with the terminal `CancellationError`, which is also this transport's documented + * SEAM-15 post-close mode: a send issued after `close()` cannot succeed over a dispatcher that no + * longer exists, so it is not reported as a retryable failure. + * + * @returns a promise that resolves once the owned dispatchers are released. + */ + close(): Promise { + this.#closing ??= (async () => { + for (const dispatcher of [...this.#dispatchers.owned].reverse()) { + await dispatcher.destroy(); + } + })(); + return this.#closing; + } + + /** + * Single teardown path, delegating to {@link UndiciTransport.close}. + * + * @returns a promise that resolves once teardown is complete. + */ + [Symbol.asyncDispose](): Promise { + return this.close(); + } +} + +/** + * Creates a `Transport` backed by `undici` — the full-featured option, with connection-pool control, + * proxy support, and real `close()` semantics over the dispatchers it owns. + * + * The returned transport is `AsyncDisposable`, so `await using transport = undiciTransport(...)` + * releases it at scope exit — the single teardown path `docs/knowledge/resource-management.md` asks + * for. + * + * @param options - optional transport settings. + * @returns a transport ready to send, disposable through `await using`. + * @throws `TypeError` when both `dispatcher` and `proxy` are supplied. + * + * @public + */ +export function undiciTransport( + options: UndiciTransportOptions = {}, +): Transport & AsyncDisposable { + return new UndiciTransport(options); +} diff --git a/packages/transport-undici/tsconfig.build.json b/packages/transport-undici/tsconfig.build.json new file mode 100644 index 0000000..d39dc7e --- /dev/null +++ b/packages/transport-undici/tsconfig.build.json @@ -0,0 +1,11 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "declaration": true, + "declarationMap": true, + "sourceMap": true + }, + "exclude": [ + "src/**/*.test.ts" + ] +} diff --git a/packages/transport-undici/tsconfig.json b/packages/transport-undici/tsconfig.json new file mode 100644 index 0000000..5c03021 --- /dev/null +++ b/packages/transport-undici/tsconfig.json @@ -0,0 +1,16 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "outDir": "./dist", + "rootDir": "./src", + "target": "ES2023", + "lib": [ + "ES2023", + "DOM", + "DOM.AsyncIterable" + ] + }, + "include": [ + "src/**/*.ts" + ] +} diff --git a/scripts/verify-consumer-types.mjs b/scripts/verify-consumer-types.mjs index 4383879..7f2af15 100644 --- a/scripts/verify-consumer-types.mjs +++ b/scripts/verify-consumer-types.mjs @@ -60,6 +60,34 @@ const builtLoggingDebug = join( 'dist', 'index.js', ); +const builtBodyFile = join( + repoRoot, + 'packages', + 'body-file', + 'dist', + 'index.js', +); +const builtTransportShared = join( + repoRoot, + 'packages', + 'transport-shared', + 'dist', + 'index.js', +); +const builtTransportFetch = join( + repoRoot, + 'packages', + 'transport-fetch', + 'dist', + 'index.js', +); +const builtTransportUndici = join( + repoRoot, + 'packages', + 'transport-undici', + 'dist', + 'index.js', +); const tsc = join(repoRoot, 'node_modules', '.bin', 'tsc'); // Checked up front, not left to the catch below. A missing prerequisite reported through the @@ -74,6 +102,10 @@ for (const artifact of [ builtCodecJson, builtLoggingPino, builtLoggingDebug, + builtBodyFile, + builtTransportShared, + builtTransportFetch, + builtTransportUndici, ]) { assert.ok( existsSync(artifact), @@ -206,6 +238,9 @@ import { type LoggingStepSettings, LOGGING_STEP_TYPE, loggingStep, + IoError, + TransportFailureError, + type FileBodyDescriptor, } from ${JSON.stringify(built)}; import { jsonSerde, @@ -223,6 +258,18 @@ import { type DebugLike, type DebugFactory, } from ${JSON.stringify(builtLoggingDebug)}; +import { + fileBody, + type FileBodyOptions, +} from ${JSON.stringify(builtBodyFile)}; +import { + fetchTransport, + type FetchTransportOptions, +} from ${JSON.stringify(builtTransportFetch)}; +import { + undiciTransport, + type UndiciTransportOptions, +} from ${JSON.stringify(builtTransportUndici)}; export function readBody(response: Response): Promise { @@ -469,6 +516,25 @@ export function loggingSeam(logger: Logger, meter: Meter, tracer: Tracer): void export function bridgeAdapters(pino: PinoLike, debug: DebugLike, debugFactory: DebugFactory): [Logger, Logger] { return [createPinoLogger(pino), createDebugLogger(debugFactory, 'custom')]; } + +// Every symbol Phase 8a promotes, referenced from a consumer's own .d.ts on the declared lib with +// types: []. @dexpace/transport-shared is deliberately absent: its exports are @internal and no +// consumer is meant to import them, so only its build artifact's existence is asserted above. +export function transportErrors(failure: TransportFailureError, io: IoError): string[] { + return [failure.name, failure.message, io.name]; +} + +export function transportAdapters( + descriptor: FileBodyDescriptor, + fileOptions: FileBodyOptions, + fetchOptions: FetchTransportOptions, +): [Transport, FileBodyDescriptor] { + return [fetchTransport(fetchOptions), fileBody(descriptor.path, fileOptions)]; +} + +export function undiciAdapter(options: UndiciTransportOptions): Transport { + return undiciTransport(options); +} `; const tsconfig = { diff --git a/scripts/verify-dual-consumption.mjs b/scripts/verify-dual-consumption.mjs index cf9244d..875cd44 100644 --- a/scripts/verify-dual-consumption.mjs +++ b/scripts/verify-dual-consumption.mjs @@ -6,11 +6,18 @@ // Phase 6a, when `@dexpace/codec-json` became the workspace's second package -- a check hard-coded // to one package silently stops covering the workspace the moment it grows. import assert from 'node:assert/strict'; -import {absent, present, serdeBody, Status} from '@dexpace/core'; +import {absent, Headers, present, serdeBody, Status} from '@dexpace/core'; import {jsonSerde} from '@dexpace/codec-json'; import {createPinoLogger} from '@dexpace/logging-pino'; import {createDebugLogger} from '@dexpace/logging-debug'; +import {fileBody} from '@dexpace/body-file'; +import { + mapOutboundHeaders, + degradeInboundHeaders, +} from '@dexpace/transport-shared'; +import {fetchTransport} from '@dexpace/transport-fetch'; +import {undiciTransport} from '@dexpace/transport-undici'; assert.equal(Status.of(200).code, 200); assert.equal(Status.of(200).name, 'OK'); @@ -67,6 +74,29 @@ debugLogger.atLevel('info').event('dual.debug').field('k', 'v').emit(); assert.equal(debugEvents.length, 1); assert.ok(debugEvents[0].includes('event=dual.debug')); +// Exercise body-file +const fb = fileBody('package.json'); +assert.equal(fb.kind, 'file'); +assert.equal(fb.replayable, true); + +// Exercise transport-shared: the outbound drop pass and the lenient inbound copy. +const outbound = mapOutboundHeaders( + Headers.newBuilder().set('Content-Length', '10').set('X-Kept', 'v').build(), + ['content-length'], +); +assert.ok(outbound.dropped.includes('content-length')); +assert.equal(outbound.sent.get('x-kept'), 'v'); +const inbound = degradeInboundHeaders([['Content-Type', 'text/plain']]); +assert.equal(inbound.headers.get('content-type'), 'text/plain'); + +// Exercise both transports far enough to prove the module graph resolved and construction runs -- +// not far enough to need a network. `close()` is the one lifecycle call that is safe with no peer. +for (const transport of [fetchTransport(), undiciTransport()]) { + assert.equal(typeof transport.send, 'function'); + assert.equal(typeof transport[Symbol.asyncDispose], 'function'); + await transport.close(); +} + console.log( 'dual-consumption check passed: plain Node import resolved and executed all packages in workspace', ); diff --git a/scripts/verify-seam-1.mjs b/scripts/verify-seam-1.mjs index cabbd97..4a69328 100644 --- a/scripts/verify-seam-1.mjs +++ b/scripts/verify-seam-1.mjs @@ -1,9 +1,14 @@ // SPDX-License-Identifier: MIT // scripts/verify-seam-1.mjs // -// SEAM-1 / NFR-1: no shipped package carries a runtime dependency. Generalized from a core-only -// check in Phase 6a, when `@dexpace/codec-json` became the workspace's second package — a check -// hard-coded to one package silently stops covering the workspace the moment it grows. +// SEAM-1 / NFR-1: no shipped package carries a runtime dependency it was not explicitly granted. +// Generalized from a core-only check in Phase 6a, when `@dexpace/codec-json` became the workspace's +// second package — a check hard-coded to one package silently stops covering the workspace the moment +// it grows. Phase 8a turned the blanket ban into an allow-list, because NFR-2 grants each optional +// capability core plus at most one external library: `ALLOWED_RUNTIME_DEPENDENCIES` below is that +// grant, written out per package. Every package absent from it is still held to a hard-committed +// empty `dependencies` object — an omitted field is a violation too, so the manifest states the +// invariant rather than merely failing to contradict it. // // It also asserts the peer-dependency pairing `sdk-design-nodejs/02` §2 prescribes for every adapter // package. That is not a style rule: without it npm's nested resolution can install two @@ -24,14 +29,39 @@ const packageDirs = readdirSync(packagesDir, {withFileTypes: true}) assert.ok(packageDirs.length > 0, 'no packages found under packages/'); +const ALLOWED_RUNTIME_DEPENDENCIES = { + '@dexpace/transport-fetch': ['@dexpace/transport-shared'], + '@dexpace/transport-undici': ['@dexpace/transport-shared', 'undici'], +}; + +let checkedCount = 0; + for (const dir of packageDirs) { const manifest = JSON.parse(readFileSync(join(dir, 'package.json'), 'utf8')); - assert.deepEqual( - manifest.dependencies, - {}, - `SEAM-1 violation: ${manifest.name} must declare zero runtime dependencies (a hard-committed empty object)`, - ); + // A private package is never published, so neither the dependency budget nor the dual-package + // hazard below can reach a consumer through it. + if (manifest.private === true) continue; + checkedCount++; + + const allowedDeps = ALLOWED_RUNTIME_DEPENDENCIES[manifest.name]; + + if (allowedDeps === undefined) { + assert.deepEqual( + manifest.dependencies, + {}, + `SEAM-1 violation: ${manifest.name} must declare zero runtime dependencies (a hard-committed empty object)`, + ); + } else { + const unexpected = Object.keys(manifest.dependencies ?? {}).filter( + dep => !allowedDeps.includes(dep), + ); + assert.equal( + unexpected.length, + 0, + `SEAM-1 / NFR-2 violation: ${manifest.name} declared unexpected runtime dependencies: ${unexpected.join(', ')}`, + ); + } if (manifest.name === '@dexpace/core') continue; @@ -46,5 +76,5 @@ for (const dir of packageDirs) { } console.log( - `SEAM-1 check passed: ${String(packageDirs.length)} package(s) have zero runtime dependencies`, + `SEAM-1 check passed: ${String(checkedCount)} package(s) verified against dependency boundaries`, ); diff --git a/scripts/verify-seam-1.test.mjs b/scripts/verify-seam-1.test.mjs index f4206f4..8d3d6f7 100644 --- a/scripts/verify-seam-1.test.mjs +++ b/scripts/verify-seam-1.test.mjs @@ -15,8 +15,10 @@ import assert from 'node:assert/strict'; import {execFileSync} from 'node:child_process'; import { copyFileSync, + existsSync, mkdirSync, mkdtempSync, + readFileSync, readdirSync, rmSync, writeFileSync, @@ -34,6 +36,13 @@ const packagesDir = join(repoRoot, 'packages'); const PACKAGES = readdirSync(packagesDir, {withFileTypes: true}) .filter(entry => entry.isDirectory()) + .filter(entry => { + const pkgJson = join(packagesDir, entry.name, 'package.json'); + return ( + existsSync(pkgJson) && + JSON.parse(readFileSync(pkgJson, 'utf8')).private !== true + ); + }) .map(entry => entry.name); test('the workspace has more than one package, so these checks are not vacuous', () => { @@ -58,7 +67,7 @@ test('verify-seam-1.mjs exits 0 and reports covering every package', () => { assert.match( output, new RegExp( - `SEAM-1 check passed: ${String(PACKAGES.length)} package\\(s\\) have zero runtime dependencies`, + `SEAM-1 check passed: ${String(PACKAGES.length)} package\\(s\\) verified against dependency boundaries`, ), `unexpected output from verify-seam-1.mjs:\n${output}`, ); @@ -116,11 +125,50 @@ test('verify-seam-1.mjs fails when any package declares a runtime dependency', ( ); }); +test('verify-seam-1.mjs fails when a package omits `dependencies` instead of committing to {}', () => { + // An omitted field is not the same as a declared empty one: the manifest has to state the + // invariant, not merely fail to contradict it. Phase 8a's allow-list rewrite briefly accepted + // `dependencies: undefined`, which is exactly how the blanket ban would erode in practice. + const omitted = {...CLEAN_ADAPTER}; + delete omitted.dependencies; + assert.throws( + () => runAgainstFixture({core: CLEAN_CORE, 'codec-fake': omitted}), + /SEAM-1 violation: @dexpace\/codec-fake/, + 'an omitted dependencies field did not fail the gate', + ); +}); + +test('verify-seam-1.mjs allows only the dependencies NFR-2 grants a package by name', () => { + // The allow-listed transports may take their sanctioned dependency and nothing else. Keyed by + // package name, so the grant cannot be inherited by a package that merely looks similar. + const granted = { + ...CLEAN_ADAPTER, + name: '@dexpace/transport-undici', + dependencies: {'@dexpace/transport-shared': 'workspace:*', undici: '^6'}, + }; + assert.match( + runAgainstFixture({core: CLEAN_CORE, 'transport-undici': granted}), + /SEAM-1 check passed: 2 package\(s\)/, + ); + assert.throws( + () => + runAgainstFixture({ + core: CLEAN_CORE, + 'transport-undici': { + ...granted, + dependencies: {...granted.dependencies, lodash: '^4'}, + }, + }), + /NFR-2 violation: @dexpace\/transport-undici declared unexpected runtime dependencies: lodash/, + 'a dependency outside the grant did not fail the gate', + ); +}); + test('verify-seam-1.mjs fails when core itself grows a runtime dependency', () => { assert.throws( () => runAgainstFixture({ - core: {...CLEAN_CORE, dependencies: {undici: '^6'}}, + core: {...CLEAN_CORE, dependencies: {lodash: '^4'}}, 'codec-fake': CLEAN_ADAPTER, }), /SEAM-1 violation: @dexpace\/core/, diff --git a/test/node-conformance/README.md b/test/node-conformance/README.md index b234ffd..d22de30 100644 --- a/test/node-conformance/README.md +++ b/test/node-conformance/README.md @@ -47,4 +47,5 @@ means Phase 4 (pipelines, where `NFR-11`'s async-framework-leak check lands) and | `seams.test.mjs` | `AbortSignal.any()` composition — folded in from the retired `scripts/verify-node-floor.mjs`, whose two assertions were the only Node coverage that existed before this suite — plus the `globalThis.crypto` floor assertion, made from ESM on purpose (Node 18 exposed `crypto` to CommonJS while leaving it undefined in ES modules) | | `io-byte-stream.test.mjs` | Phase 3a's `ByteQueue`, `BufferedSource` + views, `BufferedSink`, `TeeSink`, `writeAll` | | `body-lifecycle.test.mjs` | Phase 3b's public body surface over real Node Web Streams — reader-lock discipline, `pipeTo` ownership, multipart framing, error-body buffering | +| `transport.test.mjs` | Phase 8a's two concrete transports against a real `node:http` server on Node's own `fetch`/`undici`, `AbortSignal`, and Web Streams — redirect passthrough, timeout and no-response classification, a single-use streaming request body, lazy response bodies, `SEAM-16`'s abort-after-delivery rule, and concurrency | | `redirect.test.mjs` | Phase 5b's Location resolution on Node's own WHATWG `URL` parser (relative resolution, percent-encoding preservation, userinfo clearing, bracketed IPv6, which malformed forms throw versus resolve as a relative reference) plus `PIPE-40`'s per-hop close discipline over real Node Web Streams | diff --git a/test/node-conformance/transport.test.mjs b/test/node-conformance/transport.test.mjs new file mode 100644 index 0000000..136d8e9 --- /dev/null +++ b/test/node-conformance/transport.test.mjs @@ -0,0 +1,340 @@ +// SPDX-License-Identifier: MIT +// test/node-conformance/transport.test.mjs +// +// Phase 8a's Node layer. This is the file the suite's membership rule was written for: `bun test` runs both +// transports against *Bun's* `fetch`, `AbortSignal`, and Web Streams, and the shipping runtime is Node's — +// two independent implementations of exactly the surfaces a transport is made of. Bun's `undici` shim alone +// already diverges enough that `undici-transport.ts` has to bypass it by module path. +// +// It is also the only layer that can join BODY-11 to TRANSPORT-28: `@dexpace/body-file` is a Node-only +// package and neither transport depends on it (they narrow structurally on `body.kind === 'file'`), so a real +// `fileBody()` crossing a real transport has no home inside either package's own suite. +// +// Exercises: TRANSPORT-1 (redirects not followed), TRANSPORT-4/20 (timeout and no-response classification), +// TRANSPORT-17 (a single-use body written once, its bytes on the wire), TRANSPORT-24 (vendor status codes), +// TRANSPORT-28/BODY-11 (a real fileBody() over the wire, whole and ranged), +// TRANSPORT-25 (the response body is a lazily-read stream and close releases it), TRANSPORT-29/SEAM-12 +// (concurrent sends), SEAM-16 (an abort after delivery must not close the delivered body). +import assert from 'node:assert/strict'; +import {createServer} from 'node:http'; +import {after, before, describe, it} from 'node:test'; +import {mkdtemp, rm, writeFile} from 'node:fs/promises'; +import {tmpdir} from 'node:os'; +import {join} from 'node:path'; +import {createHash} from 'node:crypto'; +import {Headers, Request, RequestOptions} from '@dexpace/core'; +import {fileBody} from '@dexpace/body-file'; +import {fetchTransport} from '@dexpace/transport-fetch'; +import {undiciTransport} from '@dexpace/transport-undici'; + +/** Long enough that no timeout under test wins the race by luck. */ +const SLOW_RESPONSE_MS = 5_000; + +let server; +let origin; + +before(async () => { + server = createServer((req, res) => { + const {pathname} = new URL(req.url ?? '/', 'http://localhost'); + if (pathname === '/slow') { + setTimeout(() => { + res.writeHead(200); + res.end('done'); + }, SLOW_RESPONSE_MS).unref(); + return; + } + if (pathname === '/redirect') { + res.writeHead(302, {location: '/echo'}); + res.end(); + return; + } + if (pathname === '/vendor') { + res.writeHead(520, {'content-type': 'text/plain'}); + res.end('vendor status body'); + return; + } + const chunks = []; + req.on('data', chunk => chunks.push(chunk)); + req.on('end', () => { + res.writeHead(200, {'content-type': 'application/json'}); + res.end( + JSON.stringify({ + headers: req.headers, + body: Buffer.concat(chunks).toString('utf8'), + }), + ); + }); + }); + await new Promise(resolve => { + server.listen(0, '127.0.0.1', resolve); + }); + origin = `http://127.0.0.1:${server.address().port}`; +}); + +after(async () => { + server.closeAllConnections(); + await new Promise(resolve => { + server.close(resolve); + }); +}); + +/** A genuinely single-use body: `replayable: false` forces the streaming request-body path on both transports. */ +function countingBody(counter) { + const payload = new TextEncoder().encode('payload'); + return { + kind: 'stream', + mediaType: 'text/plain', + contentLength: payload.byteLength, + replayable: false, + async writeTo(sink) { + counter.writes += 1; + const writer = sink.getWriter(); + await writer.write(payload); + await writer.close(); + }, + }; +} + +/** + * Distinguishable bytes, so a truncated or misaligned send fails the digest and not merely the + * length. Printable ASCII rather than the full byte range: the shared `/echo` fixture echoes the + * request body back as a UTF-8 string, which would mangle arbitrary bytes before any assertion here + * could see them. + */ +function fixtureBytes(size) { + const buf = Buffer.alloc(size); + for (let index = 0; index < size; index += 1) { + buf[index] = 33 + ((index * 7) % 94); + } + return buf; +} + +const sha = bytes => createHash('sha256').update(bytes).digest('hex'); + +for (const [name, makeTransport] of [ + ['transport-fetch', () => fetchTransport()], + ['transport-undici', () => undiciTransport()], +]) { + describe(`${name} on the Node runtime`, () => { + it('returns a 302 raw and never follows it (TRANSPORT-1)', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder().url(`${origin}/redirect`).build(), + ); + assert.equal(response.status.code, 302); + assert.equal(response.headers.get('location'), '/echo'); + await response.close(); + } finally { + await transport.close(); + } + }); + + it('surfaces a vendor status with a readable body (TRANSPORT-24)', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder().url(`${origin}/vendor`).build(), + ); + assert.equal(response.status.code, 520); + assert.equal(await response.text(), 'vendor status body'); + } finally { + await transport.close(); + } + }); + + it('writes a single-use streaming body exactly once, bytes intact (TRANSPORT-17)', async () => { + // Node streams a request body through `duplex: 'half'` (fetch) or a `Readable` (undici); Bun's + // handling of both is its own implementation, which is the whole reason this case is here. + const transport = makeTransport(); + const counter = {writes: 0}; + try { + const response = await transport.send( + Request.newBuilder() + .method('POST') + .url(`${origin}/echo`) + .body(countingBody(counter)) + .build(), + ); + const echoed = JSON.parse(await response.text()); + assert.equal(echoed.body, 'payload'); + assert.equal(counter.writes, 1); + } finally { + await transport.close(); + } + }); + + it('exposes the response body as a stream that close() releases (TRANSPORT-25)', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder().url(`${origin}/echo`).build(), + ); + assert.ok(response.body instanceof ReadableStream); + await response.close(); + await response.close(); // idempotent (BODY-15) + } finally { + await transport.close(); + } + }); + + it('classifies a per-call timeout as retryable, not cancellation (TRANSPORT-4)', async () => { + const transport = makeTransport(); + try { + await assert.rejects( + transport.send( + Request.newBuilder().url(`${origin}/slow`).build(), + RequestOptions.newBuilder().timeoutMs(50).build(), + ), + error => { + assert.equal(error.name, 'TransportFailureError'); + return true; + }, + ); + } finally { + await transport.close(); + } + }); + + it('classifies a dead port as a retryable transport failure (TRANSPORT-20)', async () => { + const transport = makeTransport(); + try { + await assert.rejects( + transport.send( + Request.newBuilder().url('http://127.0.0.1:1').build(), + ), + error => { + assert.equal(error.name, 'TransportFailureError'); + return true; + }, + ); + } finally { + await transport.close(); + } + }); + + it('maps a caller abort to a terminal cancellation (TRANSPORT-3)', async () => { + const transport = makeTransport(); + const controller = new AbortController(); + try { + const pending = transport.send( + Request.newBuilder().url(`${origin}/slow`).build(), + undefined, + controller.signal, + ); + setTimeout(() => { + controller.abort(); + }, 20).unref(); + await assert.rejects(pending, error => { + assert.equal(error.name, 'CancellationError'); + return true; + }); + } finally { + await transport.close(); + } + }); + + it('does not close a delivered body when the signal fires afterwards (SEAM-16)', async () => { + // Both native clients tie the response body's lifetime to the signal they were handed, so this + // only holds because the transport dispatches over a fork it detaches at delivery. + const transport = makeTransport(); + const controller = new AbortController(); + try { + const response = await transport.send( + Request.newBuilder().url(`${origin}/vendor`).build(), + undefined, + controller.signal, + ); + controller.abort(); + assert.equal(await response.text(), 'vendor status body'); + } finally { + await transport.close(); + } + }); + + it('keeps concurrent sends independent of one another (TRANSPORT-29, SEAM-12)', async () => { + const transport = makeTransport(); + try { + const responses = await Promise.all( + Array.from({length: 10}, (_unused, index) => + transport.send( + Request.newBuilder() + .url(`${origin}/echo`) + .headers( + Headers.newBuilder().set('X-Call', String(index)).build(), + ) + .build(), + ), + ), + ); + const seen = await Promise.all( + responses.map(async response => { + const echoed = JSON.parse(await response.text()); + return echoed.headers['x-call']; + }), + ); + assert.equal(new Set(seen).size, 10); + } finally { + await transport.close(); + } + }); + + // The two halves of TRANSPORT-28 are tested apart everywhere else: body-file drives `writeTo` + // against a local sink, and transport-undici narrows on a hand-built `{kind: 'file'}` literal. + // Only here do a real factory and a real transport meet -- which matters most for undici, whose + // file path bypasses `writeTo` entirely for its own `createReadStream`. + describe('a real fileBody() over the wire (TRANSPORT-28, BODY-11)', () => { + let dir; + let path; + const source = fixtureBytes(300 * 1024); + + before(async () => { + dir = await mkdtemp(join(tmpdir(), 'dexpace-filebody-')); + path = join(dir, 'payload.bin'); + await writeFile(path, source); + }); + + after(async () => { + await rm(dir, {recursive: true, force: true}); + }); + + it('sends the whole file byte-exactly', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder() + .method('POST') + .url(`${origin}/echo`) + .body(fileBody(path)) + .build(), + ); + const echoed = JSON.parse(await response.text()); + assert.equal(echoed.body.length, source.byteLength); + assert.equal(sha(Buffer.from(echoed.body, 'utf8')), sha(source)); + } finally { + await transport.close(); + } + }); + + it('honors start and count', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder() + .method('POST') + .url(`${origin}/echo`) + .body(fileBody(path, {start: 10, count: 20})) + .build(), + ); + const echoed = JSON.parse(await response.text()); + assert.equal( + sha(Buffer.from(echoed.body, 'utf8')), + sha(source.subarray(10, 30)), + ); + } finally { + await transport.close(); + } + }); + }); + }); +} From dce9194049374bef9c235e0d3eb135978b368ecc Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Sat, 29 Aug 2026 09:40:13 +0300 Subject: [PATCH 2/4] chore: resolve failing ci checks, add a new skill to run CI checks locally. --- .claude/skills/ci-preflight/SKILL.md | 130 ++++++ .claude/skills/ci-preflight/run-ci.mjs | 335 ++++++++++++++++ eslint.config.js | 6 +- test/node-conformance/transport.test.mjs | 481 ++++++++++++----------- 4 files changed, 715 insertions(+), 237 deletions(-) create mode 100644 .claude/skills/ci-preflight/SKILL.md create mode 100644 .claude/skills/ci-preflight/run-ci.mjs diff --git a/.claude/skills/ci-preflight/SKILL.md b/.claude/skills/ci-preflight/SKILL.md new file mode 100644 index 0000000..57ba510 --- /dev/null +++ b/.claude/skills/ci-preflight/SKILL.md @@ -0,0 +1,130 @@ +--- +name: ci-preflight +description: Use before pushing a branch, opening or updating a PR, or whenever asked whether CI will pass, to "run the CI checks", "check CI locally", or to verify a phase is done. Runs every blocking step of .github/workflows/ci.yml against the working tree, reports all failures at once, then resolves them. +--- + +# CI Preflight + +## Overview + +`.github/workflows/ci.yml` is 14 blocking steps across two jobs. Every one of them can run +locally, so a red CI run is always avoidable — `bun test` passing is not evidence, and it is +the single most common reason work gets handed over broken. + +One command runs all of them, in CI's order: + +```bash +node .claude/skills/ci-preflight/run-ci.mjs +``` + +~2.5 minutes warm on a green tree. Full output per step goes to +`node_modules/.cache/ci-preflight/.log`; only a summary and a tail of each failure +reach stdout, so a red run costs a few hundred tokens rather than the ~40k that thirteen +raw `bun run` calls would. + +Do not hand-run the thirteen commands instead. Two things go wrong when you do: + +- **Order is load-bearing.** `test`, `api`, `lint:publish` and every `verify:*` gate resolve + `@dexpace/core` by package name, which lands in `packages/core/dist/`. Run any of them + before `build` and they either fail with unresolved-module noise or — worse — pass green + against yesterday's artifact. +- **You will stop at the first failure.** The point is to hand the user the whole list. + +## The workflow + +1. **Run it.** Add `--skip-install` only if you have not touched `package.json` since the + last install. +2. **All green** → say so plainly: CI is all good, naming the count (`all 14 steps passed`). + Nothing else to do. +3. **Anything red** → report the findings to the user *first*: which gates failed, what each + one means, and the fix you intend. One line per finding, not a transcript dump. +4. **Then resolve them**, using the playbook below. +5. **Re-verify.** Re-run the affected gates while iterating + (`--only lint,api --skip-install`), then **one full clean run before reporting done**. A + subset pass is not a green CI — fixes cross gate boundaries constantly (a lint fix edits + an export, which moves the API report, which fails `api`). + +Report honestly at every step: if a gate still fails, say so with its output. Never describe +a subset run as a full one. + +**Resolve means fix the defect, not silence the gate.** Lowering `coverageThreshold`, +deleting a failing test, adding an `eslint-disable`, or regenerating an `.api.md` to bless an +unintended export are all ways to make the runner green while shipping the bug. Where the +real fix is a judgment call — a deliberate spec deviation, a moved runtime floor, an +intentional public-API change — stop and ask. This repo is structured specifically to +prevent silent gaps (`CLAUDE.md`, "Requirement-ID conventions"); a suppression needs a stated +reason and an owner. + +## Two failure modes that read as success + +Both of these will make you report a passing gate that CI rejects. + +- **A compile error in core masks every lint finding.** `typecheck`, `lint` and `build` all + run `build:core` first, so one bad type in `packages/core/src/` makes all three fail with + the *same* `tsc` error and `gts lint .` never executes. Fix the compile error, then re-run + `lint` — the formatting and rule findings are still there, unseen. +- **The coverage floor fails silently.** `bun test` enforces `bunfig.toml`'s + `coverageThreshold` (0.8) by **exit code alone**. It prints no threshold message, and the + summary still reads `0 fail`. The runner prints a `note:` when it detects this; without + that note you would read the tail and conclude the step passed. (The + `--coverage-threshold` CLI flag is ignored — bunfig is what gates.) + +## Resolution playbook + +`fix:` lines the runner prints come from here. Steps are listed in run order. + +| Step | A failure means | First move | +|---|---|---| +| `install` | `bun.lock` disagrees with a `package.json`. The tree CI installs is not yours, so nothing after it is measuring the right thing — the runner stops here. | `bun install`, then commit `bun.lock`. | +| `typecheck` | `tsc --noEmit` over all 9 projects. | Real fix. Usual suspects: a missing `.js` extension on a relative import (NodeNext), a type import without `import type` (`verbatimModuleSyntax`), an enum/namespace/parameter property (`erasableSyntaxOnly`). | +| `lint` | Formatting **and** type-aware rules; formatting is an error, not a warning. | `bun run fix` first — it clears every prettier finding. Hand-fix what survives: 70-line function cap, `max-depth` 3, `max-params` 3, explicit return types on exported members. Every `eslint-disable` needs a `-- reason`. | +| `build` | Emit failed. **Blocks the ten gates below it**, which the runner reports `SKIP`. | Fix this before reading anything else; the skipped gates are unknown, not passing. | +| `test` | A failing test, *or* the silent coverage floor (see above). | If the tail says `0 fail`, it is coverage — find the file that dropped below 0.8 in the printed table and test it. Otherwise fix the test or the code. | +| `api` | The committed `etc/.api.md` no longer matches the built surface, or an export lacks TSDoc. | Intended export change: `cd packages/ && bun run api:local`, then commit the regenerated report. `(undocumented)` in the diff means the export needs a `@public` block, plus `@throws` naming each catchable error class. **Unintended** change: revert the export, don't bless the report. | +| `lint:publish` | `publint` + `attw` on every built package's `exports` map, `types`/`main` fields, and declaration resolution. | Fix the manifest. `cjs-resolves-to-esm` is already ignored by design (ESM-only); every other rule is real. | +| `verify:dual-consumption` | A built package is no longer importable and runnable by plain `node` through its package name. | Usually a broken `exports` map or a subpath that ships no JS. | +| `verify:consumer-types` | The built `.d.ts` does not compile on the declared `lib` with `types: []` — i.e. a dev-only global (`@types/bun`) leaked into the public surface. | Remove the dependency on the dev global, or declare it. This gate exists because exactly that defect passed all four gates above it. | +| `verify:seam-1` | A package gained a runtime dependency outside the allow-list, or dropped its committed empty `dependencies` object (an omitted field is a violation too). | Remove the dependency — SEAM-1 is the constraint, not the gate. `@dexpace/core` is a **peer** of the satellites, never a dependency. | +| `verify:sse-37` | Core's SSE code reached for serde or a codec package. | Remove the import; SSE-37/38 forbid the coupling. | +| `verify:runtime-floor` | `engines.node` and the `target`/`lib` a package compiles to have drifted apart. | Move both together, deliberately — never raise one to silence this. | +| `audit` | A high-severity advisory in production dependencies. | `bun audit --prod` for detail. Note the tree is tiny (zero runtime deps by design), so a hit here is usually a transitive dev-dep misclassification worth reading carefully. | +| `test:node` | Bun-vs-Node runtime divergence, almost always in `packages/core/src/io/` — Web Streams, `AbortSignal`, `Uint8Array` chunking. | Fix against Node's semantics. A phase touching a runtime-divergent surface should be *adding* cases here; see `test/node-conformance/README.md`. | + +## Local-vs-CI divergences worth stating + +The runner reproduces CI's steps, not CI's machine. Two gaps survive, and both belong in +your report when they matter: + +- **Node version.** `test:node` runs on whatever `node` is active; CI runs it twice, on the + `engines.node` floor (**20.3.0**) and on `lts/*`. A green local run on a newer Node does + not prove the floor. `--node-floor` runs the floor leg via `mise`/`fnm`/`nvm` (downloading + the toolchain once); the runner prints a note when the active major is not 20. + + **Run it whenever the change adds or edits a file under `test/node-conformance/`**, touches + `io/`, reaches for a new built-in, or moves the floor. This gap is not theoretical: Phase + 8a's `transport.test.mjs` passed on Node 26 and failed 20 of 22 cases on 20.3.0, because an + async *root-level* `before` hook does not complete before subtests inside a `describe` when + a file's only root children are suites — fixed in Node 22, and invisible to every other + gate. Own hooks from an enclosing `describe`, never the file root. +- **Bun version.** CI pins `.bun-version`; your local `bun` may be newer. Rarely matters, + but it is the first thing to check if a gate fails in CI and passes locally. + +CI also runs `node-conformance` only after the `ci` job succeeds — so locally, a `test:node` +failure alongside other failures is the same signal, just surfaced earlier. + +Not in CI at all, so the runner does not include them: `bun run test:scripts` (tests the +gates themselves — run it by hand after touching `scripts/`), and changesets (a +consumer-facing change still needs `bun run changeset`). + +## Runner flags + +| Flag | Effect | +|---|---| +| `--only a,b` | Run just these step ids. The iteration loop; still respects order and the build-gates-everything rule. | +| `--skip-install` | Skip the frozen-lockfile install. Safe when `package.json` is untouched. | +| `--node-floor` | Also run `test:node` under Node 20.3.0 via mise/fnm/nvm. | +| `--tail N` | Lines of a failing log to print (default 30). Raise for a wall of tsc errors. | +| — | Each step is capped at 10 minutes and reported `timeout` if it hangs. A gate *can* hang rather than fail — a conformance test holding the event loop open on an unclosed server does exactly that. | +| `--list` | Step ids and the command each runs. | + +Exit code is 0 only when every selected step ran and passed. `SKIP` is never a pass. diff --git a/.claude/skills/ci-preflight/run-ci.mjs b/.claude/skills/ci-preflight/run-ci.mjs new file mode 100644 index 0000000..9825215 --- /dev/null +++ b/.claude/skills/ci-preflight/run-ci.mjs @@ -0,0 +1,335 @@ +// SPDX-License-Identifier: MIT +// .claude/skills/ci-preflight/run-ci.mjs +// +// Runs every blocking step of `.github/workflows/ci.yml` against the working tree, in CI's own +// order, and reports all failures at once rather than stopping at the first. +// +// Two things make this more than a shell alias for thirteen `bun run` calls: +// +// * Ordering is load-bearing. `bun test`, `api`, `lint:publish` and every `verify:*` gate resolve +// `@dexpace/core` by package name, which lands in `packages/core/dist/`. Run them before +// `build` and they either fail with unresolved-module noise or, worse, pass green against +// yesterday's artifact. CI is safe because its Build step precedes its Test step; a human +// running gates ad hoc is not. +// * A failed `build` invalidates the ten gates downstream of it. Running them anyway produces ten +// spurious findings that all say "cannot resolve @dexpace/core". They are reported SKIP here, +// so the summary names the one real defect. +// +// Logs go to node_modules/.cache/ci-preflight/.log — full output stays on disk, only the +// summary and a tail of each failure reach stdout. + +import {spawnSync} from 'node:child_process'; +import {mkdirSync, writeFileSync} from 'node:fs'; +import {argv, cwd, exit, stdout, version} from 'node:process'; + +// Mirrors ci.yml step for step. `ci` is the workflow's own step name, so a failure here can be +// matched to the job that would have caught it. `fix` is the mechanical remedy where one exists. +const STEPS = [ + { + id: 'install', + ci: 'Install (frozen lockfile)', + cmd: 'bun install --frozen-lockfile', + tier: 'install', + fix: 'bun install (then commit the updated bun.lock)', + }, + {id: 'typecheck', ci: 'Typecheck', cmd: 'bun run typecheck', tier: 'build'}, + { + id: 'lint', + ci: 'Lint', + cmd: 'bun run lint', + tier: 'build', + fix: 'bun run fix', + }, + {id: 'build', ci: 'Build', cmd: 'bun run build', tier: 'build'}, + { + id: 'test', + ci: 'Test (with coverage)', + cmd: 'bun test --coverage', + tier: 'gate', + // `bun test` fails the bunfig coverage floor by exit code ALONE -- it prints no threshold + // message, and the summary above it still reads "0 fail". Read the tail without this note and + // the obvious conclusion is that the step passed. (The `--coverage-threshold` CLI flag is + // ignored; bunfig.toml's `coverageThreshold` is the one that gates.) + diagnose: output => + /^\s*0 fail\s*$/m.test(output) + ? 'every test passed, so this is the coverage floor in bunfig.toml (0.8), not a failing' + + ' test. Find the file that dropped below it in the table above.' + : null, + }, + { + id: 'api', + ci: 'API surface check', + cmd: 'bun run api', + tier: 'gate', + fix: 'cd packages/ && bun run api:local, then commit etc/.api.md', + }, + { + id: 'lint:publish', + ci: 'Package health (publint + attw)', + cmd: 'bun run lint:publish', + tier: 'gate', + }, + { + id: 'verify:dual-consumption', + ci: 'Dual JS/TS consumption check', + cmd: 'bun run verify:dual-consumption', + tier: 'gate', + }, + { + id: 'verify:consumer-types', + ci: 'Consumer typecheck against the published .d.ts', + cmd: 'bun run verify:consumer-types', + tier: 'gate', + }, + { + id: 'verify:seam-1', + ci: 'SEAM-1 zero-dependency check', + cmd: 'bun run verify:seam-1', + tier: 'gate', + }, + { + id: 'verify:sse-37', + ci: 'Verify SSE-37/SSE-38', + cmd: 'bun run verify:sse-37', + tier: 'gate', + }, + { + id: 'verify:runtime-floor', + ci: 'Runtime-floor consistency check', + cmd: 'bun run verify:runtime-floor', + tier: 'gate', + }, + {id: 'audit', ci: 'Dependency audit', cmd: 'bun run audit', tier: 'gate'}, + { + id: 'test:node', + ci: 'node-conformance (matrix)', + cmd: 'bun run test:node', + tier: 'gate', + }, +]; + +// engines.node across every publishable package, and the floor leg of ci.yml's node-conformance +// matrix. The other leg is `lts/*`, which resolves at run time and so cannot be pinned here. +const NODE_FLOOR = '20.3.0'; +// Comfortably past the slowest gate (`api`, ~50s) without letting a hung one stall the run. +const STEP_TIMEOUT_MS = 10 * 60 * 1000; +const LOG_DIR = 'node_modules/.cache/ci-preflight'; + +function parseArgs(args) { + const opts = {only: null, skipInstall: false, tail: 30, nodeFloor: false}; + for (let i = 0; i < args.length; i++) { + const arg = args[i]; + if (arg === '--list') opts.list = true; + else if (arg === '--skip-install') opts.skipInstall = true; + else if (arg === '--node-floor') opts.nodeFloor = true; + else if (arg === '--only') + opts.only = (args[++i] ?? '').split(',').filter(Boolean); + else if (arg.startsWith('--only=')) + opts.only = arg.slice(7).split(',').filter(Boolean); + else if (arg === '--tail') opts.tail = Number(args[++i]); + else if (arg.startsWith('--tail=')) opts.tail = Number(arg.slice(7)); + else if (arg === '--help' || arg === '-h') opts.help = true; + else { + console.error(`unknown argument: ${arg}\nRun with --help.`); + exit(2); + } + } + return opts; +} + +const HELP = `Usage: node .claude/skills/ci-preflight/run-ci.mjs [options] + +Runs every blocking step of .github/workflows/ci.yml against the working tree. + + --only a,b Run only these step ids (see --list). Ordering and the + build-gates-everything rule still apply. + --skip-install Skip the frozen-lockfile install. + --node-floor Additionally run test:node under Node ${NODE_FLOOR}, CI's floor + leg. Needs mise, fnm, or nvm; downloads the toolchain once. + --tail N Lines of a failing step's log to print (default 30). + --list List step ids and exit. + +Exit code is 0 only when every step selected ran and passed.`; + +function selectSteps(opts) { + let steps = STEPS; + if (opts.only) { + const known = new Set(STEPS.map(s => s.id)); + const unknown = opts.only.filter(id => !known.has(id)); + if (unknown.length > 0) { + console.error( + `unknown step id(s): ${unknown.join(', ')}\nKnown: ${[...known].join(', ')}`, + ); + exit(2); + } + steps = STEPS.filter(s => opts.only.includes(s.id)); + } + if (opts.skipInstall) steps = steps.filter(s => s.id !== 'install'); + return steps; +} + +function run(step, tail) { + const started = Date.now(); + // `2>&1` inside the shell rather than two piped streams: spawnSync hands back stdout and stderr + // as separate buffers, and concatenating them puts bun's own `$ script` echo *after* the compiler + // error it preceded. The tail is the part that gets read, so it has to be in real order. + const result = spawnSync(`${step.cmd} 2>&1`, { + shell: true, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + // A gate CAN hang rather than fail: a `node --test` file whose teardown hook never runs holds + // the event loop open on an unclosed server and waits forever. Without a cap the whole preflight + // stalls behind it, which reads as "still running" and is the one outcome worse than a red run. + timeout: STEP_TIMEOUT_MS, + killSignal: 'SIGKILL', + }); + const seconds = Math.round((Date.now() - started) / 1000); + const output = `$ ${step.cmd}\n\n${result.stdout ?? ''}${result.stderr ?? ''}`; + const log = `${LOG_DIR}/${step.id.replace(/[:/]/g, '-')}.log`; + writeFileSync(log, output); + const lines = output.trimEnd().split('\n'); + const timedOut = + result.error?.code === 'ETIMEDOUT' || result.signal === 'SIGKILL'; + const ok = result.status === 0 && !timedOut; + return { + ...step, + seconds, + log, + ok, + timedOut, + status: timedOut ? 'timeout' : result.status, + note: timedOut + ? `no output for ${STEP_TIMEOUT_MS / 60000} minutes — killed. A hang here is usually a test` + + ' holding the event loop open (an unclosed server, a teardown hook that never ran), not a' + + ' slow gate.' + : (step.diagnose?.(output) ?? null), + tail: lines.slice(-tail).join('\n'), + }; +} + +function report(results, skipped, opts) { + stdout.write('\n'); + for (const r of results) { + const mark = r.ok ? 'PASS' : 'FAIL'; + const where = r.ok ? '' : ` ${r.log}`; + stdout.write( + ` ${mark} ${r.id.padEnd(24)} ${String(r.seconds).padStart(3)}s${where}\n`, + ); + } + for (const s of skipped) { + stdout.write( + ` SKIP ${s.id.padEnd(24)} build failed — gate not meaningful\n`, + ); + } + + const failed = results.filter(r => !r.ok); + stdout.write('\n'); + if (failed.length === 0 && skipped.length === 0) { + stdout.write(`CI preflight: all ${results.length} steps passed.\n`); + return 0; + } + + stdout.write( + `CI preflight: ${failed.length} FAILED — ${failed.map(f => f.id).join(', ')}\n`, + ); + for (const f of failed) { + stdout.write( + `\n${'='.repeat(72)}\n${f.id} (ci.yml step: "${f.ci}", exit ${f.status})\n`, + ); + if (f.note) stdout.write(`note: ${f.note}\n`); + if (f.fix) stdout.write(`fix: ${f.fix}\n`); + stdout.write(`${'='.repeat(72)}\n${f.tail}\n`); + stdout.write(`[last ${opts.tail} lines; full log: ${f.log}]\n`); + } + return 1; +} + +function runNodeFloor(opts) { + const managers = [ + [ + 'mise', + `mise x node@${NODE_FLOOR} -- node --test test/node-conformance/*.test.mjs`, + ], + [ + 'fnm', + `fnm exec --using=${NODE_FLOOR} node --test test/node-conformance/*.test.mjs`, + ], + [ + 'nvm', + `bash -lc 'nvm exec ${NODE_FLOOR} node --test test/node-conformance/*.test.mjs'`, + ], + ]; + const found = managers.find( + ([bin]) => spawnSync('command', ['-v', bin], {shell: true}).status === 0, + ); + if (!found) { + stdout.write( + `\nnode-floor: no mise/fnm/nvm on PATH — Node ${NODE_FLOOR} leg not exercised.\n`, + ); + return null; + } + stdout.write( + `\nnode-floor: running test:node under Node ${NODE_FLOOR} via ${found[0]}...\n`, + ); + return run( + { + id: 'test:node@floor', + ci: `node-conformance (${NODE_FLOOR})`, + cmd: found[1], + }, + opts.tail, + ); +} + +const opts = parseArgs(argv.slice(2)); +if (opts.help) { + stdout.write(`${HELP}\n`); + exit(0); +} +if (opts.list) { + for (const s of STEPS) stdout.write(`${s.id.padEnd(24)} ${s.cmd}\n`); + exit(0); +} + +mkdirSync(LOG_DIR, {recursive: true}); +const steps = selectSteps(opts); +stdout.write( + `CI preflight — ${steps.length} step(s) from .github/workflows/ci.yml, in ${cwd()}\n`, +); + +const results = []; +const skipped = []; +let buildFailed = false; +for (const step of steps) { + if (buildFailed && step.tier === 'gate') { + skipped.push(step); + continue; + } + stdout.write(` ... ${step.id}\n`); + const result = run(step, opts.tail); + results.push(result); + if (!result.ok && step.id === 'build') buildFailed = true; + // A frozen-lockfile failure means the dependency tree on disk is not the one CI installs. + // Everything after it would be measuring the wrong tree. + if (!result.ok && step.id === 'install') { + stdout.write( + '\ninstall failed — the tree on disk is not the tree CI builds. Stopping.\n', + ); + break; + } +} + +if (opts.nodeFloor && !buildFailed) { + const floor = runNodeFloor(opts); + if (floor) results.push(floor); +} else if (!opts.nodeFloor && results.some(r => r.id === 'test:node')) { + const major = Number(version.slice(1).split('.')[0]); + if (major !== 20) { + stdout.write( + `\nnote: test:node ran on Node ${version}; CI also runs it on ${NODE_FLOOR} (the` + + ' engines.node floor). Re-run with --node-floor to exercise that leg.\n', + ); + } +} + +exit(report(results, skipped, opts)); diff --git a/eslint.config.js b/eslint.config.js index 4ae4e5e..9dacc22 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -22,8 +22,9 @@ export default tseslint.config( rules: {'prettier/prettier': ['error', gtsPrettierOptions]}, }, { - // The root config, the `.mjs` verification scripts, and the Node-runtime - // conformance suite belong to no TypeScript project; they get the + // The root config, the `.mjs` verification scripts, the Node-runtime + // conformance suite, and the `.claude/skills` runners belong to no + // TypeScript project; they get the // gts/format baseline only, never the type-aware tiers below. gts scopes // its own Node globals to a fixed list of filenames that includes none of // these, so declare them here or `console`/`URL` trip `no-undef` — and, in @@ -34,6 +35,7 @@ export default tseslint.config( 'scripts/*.mjs', 'packages/*/scripts/*.mjs', 'test/node-conformance/*.mjs', + '.claude/skills/*/*.mjs', ], languageOptions: {sourceType: 'module', globals: globals.node}, }, diff --git a/test/node-conformance/transport.test.mjs b/test/node-conformance/transport.test.mjs index 136d8e9..fdd8bd9 100644 --- a/test/node-conformance/transport.test.mjs +++ b/test/node-conformance/transport.test.mjs @@ -33,51 +33,6 @@ const SLOW_RESPONSE_MS = 5_000; let server; let origin; -before(async () => { - server = createServer((req, res) => { - const {pathname} = new URL(req.url ?? '/', 'http://localhost'); - if (pathname === '/slow') { - setTimeout(() => { - res.writeHead(200); - res.end('done'); - }, SLOW_RESPONSE_MS).unref(); - return; - } - if (pathname === '/redirect') { - res.writeHead(302, {location: '/echo'}); - res.end(); - return; - } - if (pathname === '/vendor') { - res.writeHead(520, {'content-type': 'text/plain'}); - res.end('vendor status body'); - return; - } - const chunks = []; - req.on('data', chunk => chunks.push(chunk)); - req.on('end', () => { - res.writeHead(200, {'content-type': 'application/json'}); - res.end( - JSON.stringify({ - headers: req.headers, - body: Buffer.concat(chunks).toString('utf8'), - }), - ); - }); - }); - await new Promise(resolve => { - server.listen(0, '127.0.0.1', resolve); - }); - origin = `http://127.0.0.1:${server.address().port}`; -}); - -after(async () => { - server.closeAllConnections(); - await new Promise(resolve => { - server.close(resolve); - }); -}); - /** A genuinely single-use body: `replayable: false` forces the streaming request-body path on both transports. */ function countingBody(counter) { const payload = new TextEncoder().encode('payload'); @@ -111,230 +66,286 @@ function fixtureBytes(size) { const sha = bytes => createHash('sha256').update(bytes).digest('hex'); -for (const [name, makeTransport] of [ - ['transport-fetch', () => fetchTransport()], - ['transport-undici', () => undiciTransport()], -]) { - describe(`${name} on the Node runtime`, () => { - it('returns a 302 raw and never follows it (TRANSPORT-1)', async () => { - const transport = makeTransport(); - try { - const response = await transport.send( - Request.newBuilder().url(`${origin}/redirect`).build(), - ); - assert.equal(response.status.code, 302); - assert.equal(response.headers.get('location'), '/echo'); - await response.close(); - } finally { - await transport.close(); - } - }); - - it('surfaces a vendor status with a readable body (TRANSPORT-24)', async () => { - const transport = makeTransport(); - try { - const response = await transport.send( - Request.newBuilder().url(`${origin}/vendor`).build(), - ); - assert.equal(response.status.code, 520); - assert.equal(await response.text(), 'vendor status body'); - } finally { - await transport.close(); - } - }); - - it('writes a single-use streaming body exactly once, bytes intact (TRANSPORT-17)', async () => { - // Node streams a request body through `duplex: 'half'` (fetch) or a `Readable` (undici); Bun's - // handling of both is its own implementation, which is the whole reason this case is here. - const transport = makeTransport(); - const counter = {writes: 0}; - try { - const response = await transport.send( - Request.newBuilder() - .method('POST') - .url(`${origin}/echo`) - .body(countingBody(counter)) - .build(), - ); - const echoed = JSON.parse(await response.text()); - assert.equal(echoed.body, 'payload'); - assert.equal(counter.writes, 1); - } finally { - await transport.close(); - } - }); - - it('exposes the response body as a stream that close() releases (TRANSPORT-25)', async () => { - const transport = makeTransport(); - try { - const response = await transport.send( - Request.newBuilder().url(`${origin}/echo`).build(), - ); - assert.ok(response.body instanceof ReadableStream); - await response.close(); - await response.close(); // idempotent (BODY-15) - } finally { - await transport.close(); +// Every hook and test lives inside this suite rather than at the file root, and that is +// load-bearing on the declared floor. Under Node 20.3.0 -- `engines.node`, and the floor leg of +// CI's node-conformance matrix -- an async ROOT-level `before` does not finish before subtests +// inside a `describe` start, in a file whose only root children are suites. This file is exactly +// that shape: the loop below contributes two `describe`s and no top-level `it`, so every test read +// `origin` as `undefined` and failed with `malformed or non-absolute URL: undefined/redirect`, +// while the matching root `after` never closed the server and the run hung. Node 22 fixed the +// ordering. Owning the hooks from a suite is correct on every version, and neither `bun test` nor +// a newer local Node can see the difference -- only the matrix floor leg can. +describe('the transport adapters on the Node runtime', () => { + before(async () => { + server = createServer((req, res) => { + const {pathname} = new URL(req.url ?? '/', 'http://localhost'); + if (pathname === '/slow') { + setTimeout(() => { + res.writeHead(200); + res.end('done'); + }, SLOW_RESPONSE_MS).unref(); + return; } - }); - - it('classifies a per-call timeout as retryable, not cancellation (TRANSPORT-4)', async () => { - const transport = makeTransport(); - try { - await assert.rejects( - transport.send( - Request.newBuilder().url(`${origin}/slow`).build(), - RequestOptions.newBuilder().timeoutMs(50).build(), - ), - error => { - assert.equal(error.name, 'TransportFailureError'); - return true; - }, - ); - } finally { - await transport.close(); + if (pathname === '/redirect') { + res.writeHead(302, {location: '/echo'}); + res.end(); + return; } - }); - - it('classifies a dead port as a retryable transport failure (TRANSPORT-20)', async () => { - const transport = makeTransport(); - try { - await assert.rejects( - transport.send( - Request.newBuilder().url('http://127.0.0.1:1').build(), - ), - error => { - assert.equal(error.name, 'TransportFailureError'); - return true; - }, - ); - } finally { - await transport.close(); + if (pathname === '/vendor') { + res.writeHead(520, {'content-type': 'text/plain'}); + res.end('vendor status body'); + return; } - }); - - it('maps a caller abort to a terminal cancellation (TRANSPORT-3)', async () => { - const transport = makeTransport(); - const controller = new AbortController(); - try { - const pending = transport.send( - Request.newBuilder().url(`${origin}/slow`).build(), - undefined, - controller.signal, + const chunks = []; + req.on('data', chunk => chunks.push(chunk)); + req.on('end', () => { + res.writeHead(200, {'content-type': 'application/json'}); + res.end( + JSON.stringify({ + headers: req.headers, + body: Buffer.concat(chunks).toString('utf8'), + }), ); - setTimeout(() => { - controller.abort(); - }, 20).unref(); - await assert.rejects(pending, error => { - assert.equal(error.name, 'CancellationError'); - return true; - }); - } finally { - await transport.close(); - } + }); }); - - it('does not close a delivered body when the signal fires afterwards (SEAM-16)', async () => { - // Both native clients tie the response body's lifetime to the signal they were handed, so this - // only holds because the transport dispatches over a fork it detaches at delivery. - const transport = makeTransport(); - const controller = new AbortController(); - try { - const response = await transport.send( - Request.newBuilder().url(`${origin}/vendor`).build(), - undefined, - controller.signal, - ); - controller.abort(); - assert.equal(await response.text(), 'vendor status body'); - } finally { - await transport.close(); - } + await new Promise(resolve => { + server.listen(0, '127.0.0.1', resolve); }); + origin = `http://127.0.0.1:${server.address().port}`; + }); - it('keeps concurrent sends independent of one another (TRANSPORT-29, SEAM-12)', async () => { - const transport = makeTransport(); - try { - const responses = await Promise.all( - Array.from({length: 10}, (_unused, index) => - transport.send( - Request.newBuilder() - .url(`${origin}/echo`) - .headers( - Headers.newBuilder().set('X-Call', String(index)).build(), - ) - .build(), - ), - ), - ); - const seen = await Promise.all( - responses.map(async response => { - const echoed = JSON.parse(await response.text()); - return echoed.headers['x-call']; - }), - ); - assert.equal(new Set(seen).size, 10); - } finally { - await transport.close(); - } + after(async () => { + server.closeAllConnections(); + await new Promise(resolve => { + server.close(resolve); }); + }); - // The two halves of TRANSPORT-28 are tested apart everywhere else: body-file drives `writeTo` - // against a local sink, and transport-undici narrows on a hand-built `{kind: 'file'}` literal. - // Only here do a real factory and a real transport meet -- which matters most for undici, whose - // file path bypasses `writeTo` entirely for its own `createReadStream`. - describe('a real fileBody() over the wire (TRANSPORT-28, BODY-11)', () => { - let dir; - let path; - const source = fixtureBytes(300 * 1024); - - before(async () => { - dir = await mkdtemp(join(tmpdir(), 'dexpace-filebody-')); - path = join(dir, 'payload.bin'); - await writeFile(path, source); + for (const [name, makeTransport] of [ + ['transport-fetch', () => fetchTransport()], + ['transport-undici', () => undiciTransport()], + ]) { + describe(`${name} on the Node runtime`, () => { + it('returns a 302 raw and never follows it (TRANSPORT-1)', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder().url(`${origin}/redirect`).build(), + ); + assert.equal(response.status.code, 302); + assert.equal(response.headers.get('location'), '/echo'); + await response.close(); + } finally { + await transport.close(); + } }); - after(async () => { - await rm(dir, {recursive: true, force: true}); + it('surfaces a vendor status with a readable body (TRANSPORT-24)', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder().url(`${origin}/vendor`).build(), + ); + assert.equal(response.status.code, 520); + assert.equal(await response.text(), 'vendor status body'); + } finally { + await transport.close(); + } }); - it('sends the whole file byte-exactly', async () => { + it('writes a single-use streaming body exactly once, bytes intact (TRANSPORT-17)', async () => { + // Node streams a request body through `duplex: 'half'` (fetch) or a `Readable` (undici); Bun's + // handling of both is its own implementation, which is the whole reason this case is here. const transport = makeTransport(); + const counter = {writes: 0}; try { const response = await transport.send( Request.newBuilder() .method('POST') .url(`${origin}/echo`) - .body(fileBody(path)) + .body(countingBody(counter)) .build(), ); const echoed = JSON.parse(await response.text()); - assert.equal(echoed.body.length, source.byteLength); - assert.equal(sha(Buffer.from(echoed.body, 'utf8')), sha(source)); + assert.equal(echoed.body, 'payload'); + assert.equal(counter.writes, 1); } finally { await transport.close(); } }); - it('honors start and count', async () => { + it('exposes the response body as a stream that close() releases (TRANSPORT-25)', async () => { const transport = makeTransport(); try { const response = await transport.send( - Request.newBuilder() - .method('POST') - .url(`${origin}/echo`) - .body(fileBody(path, {start: 10, count: 20})) - .build(), + Request.newBuilder().url(`${origin}/echo`).build(), ); - const echoed = JSON.parse(await response.text()); - assert.equal( - sha(Buffer.from(echoed.body, 'utf8')), - sha(source.subarray(10, 30)), + assert.ok(response.body instanceof ReadableStream); + await response.close(); + await response.close(); // idempotent (BODY-15) + } finally { + await transport.close(); + } + }); + + it('classifies a per-call timeout as retryable, not cancellation (TRANSPORT-4)', async () => { + const transport = makeTransport(); + try { + await assert.rejects( + transport.send( + Request.newBuilder().url(`${origin}/slow`).build(), + RequestOptions.newBuilder().timeoutMs(50).build(), + ), + error => { + assert.equal(error.name, 'TransportFailureError'); + return true; + }, + ); + } finally { + await transport.close(); + } + }); + + it('classifies a dead port as a retryable transport failure (TRANSPORT-20)', async () => { + const transport = makeTransport(); + try { + await assert.rejects( + transport.send( + Request.newBuilder().url('http://127.0.0.1:1').build(), + ), + error => { + assert.equal(error.name, 'TransportFailureError'); + return true; + }, + ); + } finally { + await transport.close(); + } + }); + + it('maps a caller abort to a terminal cancellation (TRANSPORT-3)', async () => { + const transport = makeTransport(); + const controller = new AbortController(); + try { + const pending = transport.send( + Request.newBuilder().url(`${origin}/slow`).build(), + undefined, + controller.signal, + ); + setTimeout(() => { + controller.abort(); + }, 20).unref(); + await assert.rejects(pending, error => { + assert.equal(error.name, 'CancellationError'); + return true; + }); + } finally { + await transport.close(); + } + }); + + it('does not close a delivered body when the signal fires afterwards (SEAM-16)', async () => { + // Both native clients tie the response body's lifetime to the signal they were handed, so this + // only holds because the transport dispatches over a fork it detaches at delivery. + const transport = makeTransport(); + const controller = new AbortController(); + try { + const response = await transport.send( + Request.newBuilder().url(`${origin}/vendor`).build(), + undefined, + controller.signal, + ); + controller.abort(); + assert.equal(await response.text(), 'vendor status body'); + } finally { + await transport.close(); + } + }); + + it('keeps concurrent sends independent of one another (TRANSPORT-29, SEAM-12)', async () => { + const transport = makeTransport(); + try { + const responses = await Promise.all( + Array.from({length: 10}, (_unused, index) => + transport.send( + Request.newBuilder() + .url(`${origin}/echo`) + .headers( + Headers.newBuilder().set('X-Call', String(index)).build(), + ) + .build(), + ), + ), + ); + const seen = await Promise.all( + responses.map(async response => { + const echoed = JSON.parse(await response.text()); + return echoed.headers['x-call']; + }), ); + assert.equal(new Set(seen).size, 10); } finally { await transport.close(); } }); + + // The two halves of TRANSPORT-28 are tested apart everywhere else: body-file drives `writeTo` + // against a local sink, and transport-undici narrows on a hand-built `{kind: 'file'}` literal. + // Only here do a real factory and a real transport meet -- which matters most for undici, whose + // file path bypasses `writeTo` entirely for its own `createReadStream`. + describe('a real fileBody() over the wire (TRANSPORT-28, BODY-11)', () => { + let dir; + let path; + const source = fixtureBytes(300 * 1024); + + before(async () => { + dir = await mkdtemp(join(tmpdir(), 'dexpace-filebody-')); + path = join(dir, 'payload.bin'); + await writeFile(path, source); + }); + + after(async () => { + await rm(dir, {recursive: true, force: true}); + }); + + it('sends the whole file byte-exactly', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder() + .method('POST') + .url(`${origin}/echo`) + .body(fileBody(path)) + .build(), + ); + const echoed = JSON.parse(await response.text()); + assert.equal(echoed.body.length, source.byteLength); + assert.equal(sha(Buffer.from(echoed.body, 'utf8')), sha(source)); + } finally { + await transport.close(); + } + }); + + it('honors start and count', async () => { + const transport = makeTransport(); + try { + const response = await transport.send( + Request.newBuilder() + .method('POST') + .url(`${origin}/echo`) + .body(fileBody(path, {start: 10, count: 20})) + .build(), + ); + const echoed = JSON.parse(await response.text()); + assert.equal( + sha(Buffer.from(echoed.body, 'utf8')), + sha(source.subarray(10, 30)), + ); + } finally { + await transport.close(); + } + }); + }); }); - }); -} + } +}); From 3a0f708246657399853edcdb0a4b26c907554093 Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Sat, 29 Aug 2026 10:00:50 +0300 Subject: [PATCH 3/4] chore: resolve failing ci checks, fixes on the ci skill. --- .claude/skills/ci-preflight/SKILL.md | 22 ++++++++++++--- .claude/skills/ci-preflight/run-ci.mjs | 31 +++++++++++++++++++-- CLAUDE.md | 38 ++++++++++++++++++-------- package.json | 9 +++--- 4 files changed, 79 insertions(+), 21 deletions(-) diff --git a/.claude/skills/ci-preflight/SKILL.md b/.claude/skills/ci-preflight/SKILL.md index 57ba510..37ab5fd 100644 --- a/.claude/skills/ci-preflight/SKILL.md +++ b/.claude/skills/ci-preflight/SKILL.md @@ -33,15 +33,16 @@ Do not hand-run the thirteen commands instead. Two things go wrong when you do: ## The workflow 1. **Run it.** Add `--skip-install` only if you have not touched `package.json` since the - last install. + last install. **Add `--clean` before you push** — see below; a warm run cannot see a + whole class of defect that CI hits on its first step. 2. **All green** → say so plainly: CI is all good, naming the count (`all 14 steps passed`). Nothing else to do. 3. **Anything red** → report the findings to the user *first*: which gates failed, what each one means, and the fix you intend. One line per finding, not a transcript dump. 4. **Then resolve them**, using the playbook below. 5. **Re-verify.** Re-run the affected gates while iterating - (`--only lint,api --skip-install`), then **one full clean run before reporting done**. A - subset pass is not a green CI — fixes cross gate boundaries constantly (a lint fix edits + (`--only lint,api --skip-install`), then **one full `--clean` run before reporting done**. + A subset pass is not a green CI — fixes cross gate boundaries constantly (a lint fix edits an export, which moves the API report, which fails `api`). Report honestly at every step: if a gate still fails, say so with its output. Never describe @@ -63,6 +64,18 @@ Both of these will make you report a passing gate that CI rejects. run `build:core` first, so one bad type in `packages/core/src/` makes all three fail with the *same* `tsc` error and `gts lint .` never executes. Fix the compile error, then re-run `lint` — the formatting and rule findings are still there, unseen. +- **A warm tree hides missing build prerequisites.** CI checks out a tree with no `dist/` in + it; yours almost never is one. A package whose `exports` point at `dist/`, imported by name + from another package's `src/` with nothing building it first, resolves fine locally against + the leftovers of your last build and fails on a fresh clone. Every gate goes green here and + CI dies on step 2. **`--clean` is the answer** — it sweeps every `dist/` and `*.tsbuildinfo` + first, so the run starts where CI starts. It costs ~40s of rebuild. + + This is not hypothetical. PR #52 failed exactly this way: Phase 8a made + `@dexpace/transport-shared` the second published package imported by name from another + package's `src/`, `typecheck` and `lint` still pre-built only core, and a warm preflight + passed all 14 steps on the commit CI rejected. Fixed by `build:deps` — see CLAUDE.md, and + keep that list current when a new package crosses the same line. - **The coverage floor fails silently.** `bun test` enforces `bunfig.toml`'s `coverageThreshold` (0.8) by **exit code alone**. It prints no threshold message, and the summary still reads `0 fail`. The runner prints a `note:` when it detects this; without @@ -76,7 +89,7 @@ Both of these will make you report a passing gate that CI rejects. | Step | A failure means | First move | |---|---|---| | `install` | `bun.lock` disagrees with a `package.json`. The tree CI installs is not yours, so nothing after it is measuring the right thing — the runner stops here. | `bun install`, then commit `bun.lock`. | -| `typecheck` | `tsc --noEmit` over all 9 projects. | Real fix. Usual suspects: a missing `.js` extension on a relative import (NodeNext), a type import without `import type` (`verbatimModuleSyntax`), an enum/namespace/parameter property (`erasableSyntaxOnly`). | +| `typecheck` | `tsc --noEmit` over all 9 projects. | `Cannot find module '@dexpace/…'` means a build prerequisite is missing from `build:deps`, not a bad import — check with `--clean`. Otherwise a real fix; usual suspects: a missing `.js` extension on a relative import (NodeNext), a type import without `import type` (`verbatimModuleSyntax`), an enum/namespace/parameter property (`erasableSyntaxOnly`). | | `lint` | Formatting **and** type-aware rules; formatting is an error, not a warning. | `bun run fix` first — it clears every prettier finding. Hand-fix what survives: 70-line function cap, `max-depth` 3, `max-params` 3, explicit return types on exported members. Every `eslint-disable` needs a `-- reason`. | | `build` | Emit failed. **Blocks the ten gates below it**, which the runner reports `SKIP`. | Fix this before reading anything else; the skipped gates are unknown, not passing. | | `test` | A failing test, *or* the silent coverage floor (see above). | If the tail says `0 fail`, it is coverage — find the file that dropped below 0.8 in the printed table and test it. Otherwise fix the test or the code. | @@ -121,6 +134,7 @@ consumer-facing change still needs `bun run changeset`). | Flag | Effect | |---|---| | `--only a,b` | Run just these step ids. The iteration loop; still respects order and the build-gates-everything rule. | +| `--clean` | Sweep every `dist/` and `*.tsbuildinfo` first, so the run starts from the tree CI checks out. The pre-push default. ~40s. | | `--skip-install` | Skip the frozen-lockfile install. Safe when `package.json` is untouched. | | `--node-floor` | Also run `test:node` under Node 20.3.0 via mise/fnm/nvm. | | `--tail N` | Lines of a failing log to print (default 30). Raise for a wall of tsc errors. | diff --git a/.claude/skills/ci-preflight/run-ci.mjs b/.claude/skills/ci-preflight/run-ci.mjs index 9825215..27a0c7f 100644 --- a/.claude/skills/ci-preflight/run-ci.mjs +++ b/.claude/skills/ci-preflight/run-ci.mjs @@ -19,7 +19,7 @@ // summary and a tail of each failure reach stdout. import {spawnSync} from 'node:child_process'; -import {mkdirSync, writeFileSync} from 'node:fs'; +import {globSync, mkdirSync, rmSync, writeFileSync} from 'node:fs'; import {argv, cwd, exit, stdout, version} from 'node:process'; // Mirrors ci.yml step for step. `ci` is the workflow's own step name, so a failure here can be @@ -116,12 +116,19 @@ const STEP_TIMEOUT_MS = 10 * 60 * 1000; const LOG_DIR = 'node_modules/.cache/ci-preflight'; function parseArgs(args) { - const opts = {only: null, skipInstall: false, tail: 30, nodeFloor: false}; + const opts = { + only: null, + skipInstall: false, + tail: 30, + nodeFloor: false, + clean: false, + }; for (let i = 0; i < args.length; i++) { const arg = args[i]; if (arg === '--list') opts.list = true; else if (arg === '--skip-install') opts.skipInstall = true; else if (arg === '--node-floor') opts.nodeFloor = true; + else if (arg === '--clean') opts.clean = true; else if (arg === '--only') opts.only = (args[++i] ?? '').split(',').filter(Boolean); else if (arg.startsWith('--only=')) @@ -146,6 +153,9 @@ Runs every blocking step of .github/workflows/ci.yml against the working tree. --skip-install Skip the frozen-lockfile install. --node-floor Additionally run test:node under Node ${NODE_FLOOR}, CI's floor leg. Needs mise, fnm, or nvm; downloads the toolchain once. + --clean Delete every dist/ and *.tsbuildinfo first, so the run starts + from the state CI checks out. Catches missing build + prerequisites that a warm tree hides. Costs ~40s. --tail N Lines of a failing step's log to print (default 30). --list List step ids and exit. @@ -291,8 +301,25 @@ if (opts.list) { exit(0); } +// CI checks out a tree with no build artifacts in it; a working tree almost never is one. That gap +// hides a whole class of defect -- a package whose `exports` point at `dist/` being imported by name +// from another package's `src/` without anything building it first. Every gate passes locally +// against the stale `dist/` left over from the last build, and the fresh clone CI runs cannot +// resolve the module at all. Sweeping the artifacts is what makes the preflight a real rehearsal. +function cleanArtifacts() { + const targets = [ + ...globSync('packages/*/dist'), + ...globSync('packages/*/*.tsbuildinfo'), + ]; + for (const target of targets) rmSync(target, {recursive: true, force: true}); + stdout.write( + `clean: removed ${targets.length} build artifact(s) — starting from CI's state\n`, + ); +} + mkdirSync(LOG_DIR, {recursive: true}); const steps = selectSteps(opts); +if (opts.clean) cleanArtifacts(); stdout.write( `CI preflight — ${steps.length} step(s) from .github/workflows/ci.yml, in ${cwd()}\n`, ); diff --git a/CLAUDE.md b/CLAUDE.md index 1371a70..76baa6b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,21 +20,37 @@ All run from the repo root unless noted. ```bash bun install --frozen-lockfile -bun run build:core # tsc -b of core's declarations; incremental, and a prerequisite of the three below -bun run typecheck # build:core, then tsc --noEmit per package (core, then codec-json) -bun run lint # build:core, then gts lint . — formatting AND type-aware rules; fatal -bun run fix # build:core, then gts fix . — autofixes formatting/lint -bun run build # build:core, then plain tsc for codec-json → each package's dist/ +bun run build:core # tsc -b of core's declarations; incremental +bun run build:deps # build:core + transport-shared — every package another package's src + # imports BY NAME; a prerequisite of the four below +bun run typecheck # build:deps, then tsc --noEmit per package +bun run lint # build:deps, then gts lint . — formatting AND type-aware rules; fatal +bun run fix # build:deps, then gts fix . — autofixes formatting/lint +bun run build # build:deps, then plain tsc for the rest → each package's dist/ bun test # needs `build` first (see below); coverage on by default, 80% line floor bun run test:node # Node-runtime conformance against the BUILT artifact; needs `build` first ``` -**Anything that resolves `@dexpace/core` by package name needs core's `dist/` to exist**, from Phase 6a on — -`@dexpace/codec-json` reaches core only through its published entry point, and both `tsc` and Bun follow the -`types`/`main` fields there. `typecheck`, `lint`, `fix`, and `build` each run `build:core` first for that -reason, so every one of them works on a fresh clone. `build:core` is `tsc -b`, so a warm repeat is close to -free. Do not drop that prefix to "save a step": without it `typecheck` fails with 30 unresolved-module errors -the moment `dist/` is absent, which is exactly what a CI runner sees. +**Anything that resolves a workspace package by name needs that package's `dist/` to exist**, from Phase 6a +on — a consumer reaches it only through its published entry point, and both `tsc` and Bun follow the +`types`/`main` fields there. `typecheck`, `lint`, `fix`, and `build` each run `build:deps` first for that +reason, so every one of them works on a fresh clone. Both legs are `tsc`, so a warm repeat is close to free. +Do not drop that prefix to "save a step": without it `typecheck` fails with unresolved-module errors the +moment `dist/` is absent, which is exactly what a CI runner sees. + +**`build:deps` is the list, and it grows.** It is core plus `@dexpace/transport-shared` today. A package +belongs in it the moment another package's `src/` imports it *by name* and its `exports` point at `dist/`. +Phase 8a proved the cost of missing one: `transport-shared` landed as the second such package, `build:core` +stayed the prefix, and CI failed on `typecheck` at the first fresh clone while every local gate stayed green +against a warm `dist/`. `@dexpace/transport-conformance` is deliberately absent — it is `private` and its +`exports` name `./src/index.ts`, so it resolves unbuilt. Check the graph, not this sentence: + +```bash +for d in packages/*/; do grep -rhoE "from '@dexpace/[a-z-]+'" "$d/src" | sort -u; done +``` + +`node .claude/skills/ci-preflight/run-ci.mjs --clean` is what catches a missing entry — it sweeps every +`dist/` and `*.tsbuildinfo` first, so the run starts from the tree CI checks out rather than a warm one. `bun test` runs the unit suite on **Bun** and is scoped to `packages/` (`bunfig.toml`'s `[test] root`). **It needs `bun run build` to have run first**, from Phase 6a on: `@dexpace/codec-json`'s tests reach core diff --git a/package.json b/package.json index 85ced8f..37e5201 100644 --- a/package.json +++ b/package.json @@ -41,12 +41,13 @@ "fast-uri": "^3.1.5" }, "scripts": { - "lint": "bun run build:core && gts lint .", - "fix": "bun run build:core && gts fix .", + "lint": "bun run build:deps && gts lint .", + "fix": "bun run build:deps && gts fix .", "build:core": "tsc -b packages/core/tsconfig.build.json", - "typecheck": "bun run build:core && tsc -p packages/core/tsconfig.json --noEmit && tsc -p packages/codec-json/tsconfig.json --noEmit && tsc -p packages/logging-pino/tsconfig.json --noEmit && tsc -p packages/logging-debug/tsconfig.json --noEmit && tsc -p packages/body-file/tsconfig.json --noEmit && tsc -p packages/transport-shared/tsconfig.json --noEmit && tsc -p packages/transport-conformance/tsconfig.json --noEmit && tsc -p packages/transport-fetch/tsconfig.json --noEmit && tsc -p packages/transport-undici/tsconfig.json --noEmit", + "build:deps": "bun run build:core && tsc -p packages/transport-shared/tsconfig.build.json", + "typecheck": "bun run build:deps && tsc -p packages/core/tsconfig.json --noEmit && tsc -p packages/codec-json/tsconfig.json --noEmit && tsc -p packages/logging-pino/tsconfig.json --noEmit && tsc -p packages/logging-debug/tsconfig.json --noEmit && tsc -p packages/body-file/tsconfig.json --noEmit && tsc -p packages/transport-shared/tsconfig.json --noEmit && tsc -p packages/transport-conformance/tsconfig.json --noEmit && tsc -p packages/transport-fetch/tsconfig.json --noEmit && tsc -p packages/transport-undici/tsconfig.json --noEmit", "prebuild": "bun run --cwd packages/core prebuild", - "build": "bun run build:core && tsc -p packages/codec-json/tsconfig.build.json && tsc -p packages/logging-pino/tsconfig.build.json && tsc -p packages/logging-debug/tsconfig.build.json && tsc -p packages/body-file/tsconfig.build.json && tsc -p packages/transport-shared/tsconfig.build.json && tsc -p packages/transport-fetch/tsconfig.build.json && tsc -p packages/transport-undici/tsconfig.build.json", + "build": "bun run build:deps && tsc -p packages/codec-json/tsconfig.build.json && tsc -p packages/logging-pino/tsconfig.build.json && tsc -p packages/logging-debug/tsconfig.build.json && tsc -p packages/body-file/tsconfig.build.json && tsc -p packages/transport-fetch/tsconfig.build.json && tsc -p packages/transport-undici/tsconfig.build.json", "test": "bun test", "knowledge": "node scripts/knowledge.mjs", "test:scripts": "node --test 'scripts/*.test.mjs'", From 25e765e6460065f57c5b74061afed8d583bff707 Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Sat, 29 Aug 2026 10:28:18 +0300 Subject: [PATCH 4/4] chore: resolve failing ci checks, fixes on the ci skill. --- .claude/skills/ci-preflight/SKILL.md | 23 +++++-- .claude/skills/ci-preflight/run-ci.mjs | 68 +++++++++++++++++-- CLAUDE.md | 4 ++ .../transport-conformance/src/fixtures.ts | 28 ++++++-- .../transport-conformance/src/run-suite.ts | 22 +++++- 5 files changed, 128 insertions(+), 17 deletions(-) diff --git a/.claude/skills/ci-preflight/SKILL.md b/.claude/skills/ci-preflight/SKILL.md index 37ab5fd..683c1ca 100644 --- a/.claude/skills/ci-preflight/SKILL.md +++ b/.claude/skills/ci-preflight/SKILL.md @@ -33,8 +33,8 @@ Do not hand-run the thirteen commands instead. Two things go wrong when you do: ## The workflow 1. **Run it.** Add `--skip-install` only if you have not touched `package.json` since the - last install. **Add `--clean` before you push** — see below; a warm run cannot see a - whole class of defect that CI hits on its first step. + last install. **Before you push, add `--clean`** — a warm tree is blind to a whole class of + defect CI hits on its first step. (The pinned Bun needs no flag; it is the default.) 2. **All green** → say so plainly: CI is all good, naming the count (`all 14 steps passed`). Nothing else to do. 3. **Anything red** → report the findings to the user *first*: which gates failed, what each @@ -64,6 +64,20 @@ Both of these will make you report a passing gate that CI rejects. run `build:core` first, so one bad type in `packages/core/src/` makes all three fail with the *same* `tsc` error and `gts lint .` never executes. Fix the compile error, then re-run `lint` — the formatting and rule findings are still there, unseen. +- **Your Bun is not CI's Bun** — handled by default, but know why. `.bun-version` is what + `setup-bun` resolves, and Bun's `fetch` and `node:http` are independent implementations that + change between releases; a test can pass on yours and fail on CI with no code difference at + all. **The runner pins every step to `.bun-version` via mise automatically**, nested + `bun run` chains included. If mise cannot supply it, the run continues but says loudly that + it is measuring the wrong runtime — that banner is not decoration, and a green run under it + is not a green CI. `--path-bun` opts out deliberately, which is worth doing only to check + whether a newer Bun fixes something. + + PR #52 hit this twice in one run, both invisible on Bun 1.4.0: `node:http` emitted a + response carrying *both* `Transfer-Encoding: chunked` and `Content-Length` with an unchunked + body (undici rejected it, Bun's own `fetch` hung to a 5s timeout), and `fetch` served a + poisoned pooled connection to a later row, failing a timeout assertion ~30 rows from its + cause. Reproducing each took one command on the pinned version and was guesswork without it. - **A warm tree hides missing build prerequisites.** CI checks out a tree with no `dist/` in it; yours almost never is one. A package whose `exports` point at `dist/`, imported by name from another package's `src/` with nothing building it first, resolves fine locally against @@ -119,8 +133,8 @@ your report when they matter: async *root-level* `before` hook does not complete before subtests inside a `describe` when a file's only root children are suites — fixed in Node 22, and invisible to every other gate. Own hooks from an enclosing `describe`, never the file root. -- **Bun version.** CI pins `.bun-version`; your local `bun` may be newer. Rarely matters, - but it is the first thing to check if a gate fails in CI and passes locally. +- **Bun version.** Closed by default — the runner pins to `.bun-version` itself. The gap + reopens only when mise cannot supply that version, and the run says so in a banner. CI also runs `node-conformance` only after the `ci` job succeeds — so locally, a `test:node` failure alongside other failures is the same signal, just surfaced earlier. @@ -135,6 +149,7 @@ consumer-facing change still needs `bun run changeset`). |---|---| | `--only a,b` | Run just these step ids. The iteration loop; still respects order and the build-gates-everything rule. | | `--clean` | Sweep every `dist/` and `*.tsbuildinfo` first, so the run starts from the tree CI checks out. The pre-push default. ~40s. | +| `--path-bun` | Run on PATH's bun instead of `.bun-version`'s. The pinned Bun is the default; use this only to test a newer one. | | `--skip-install` | Skip the frozen-lockfile install. Safe when `package.json` is untouched. | | `--node-floor` | Also run `test:node` under Node 20.3.0 via mise/fnm/nvm. | | `--tail N` | Lines of a failing log to print (default 30). Raise for a wall of tsc errors. | diff --git a/.claude/skills/ci-preflight/run-ci.mjs b/.claude/skills/ci-preflight/run-ci.mjs index 27a0c7f..b6aa2a8 100644 --- a/.claude/skills/ci-preflight/run-ci.mjs +++ b/.claude/skills/ci-preflight/run-ci.mjs @@ -19,8 +19,14 @@ // summary and a tail of each failure reach stdout. import {spawnSync} from 'node:child_process'; -import {globSync, mkdirSync, rmSync, writeFileSync} from 'node:fs'; -import {argv, cwd, exit, stdout, version} from 'node:process'; +import { + globSync, + mkdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import {argv, cwd, env, exit, stdout, version} from 'node:process'; // Mirrors ci.yml step for step. `ci` is the workflow's own step name, so a failure here can be // matched to the job that would have caught it. `fix` is the mechanical remedy where one exists. @@ -113,6 +119,8 @@ const STEPS = [ const NODE_FLOOR = '20.3.0'; // Comfortably past the slowest gate (`api`, ~50s) without letting a hung one stall the run. const STEP_TIMEOUT_MS = 10 * 60 * 1000; +// setup-bun resolves this file, so it is the Bun every CI step actually runs on. +const PINNED_BUN = readFileSync('.bun-version', 'utf8').trim(); const LOG_DIR = 'node_modules/.cache/ci-preflight'; function parseArgs(args) { @@ -122,6 +130,7 @@ function parseArgs(args) { tail: 30, nodeFloor: false, clean: false, + pinnedBun: true, }; for (let i = 0; i < args.length; i++) { const arg = args[i]; @@ -129,6 +138,8 @@ function parseArgs(args) { else if (arg === '--skip-install') opts.skipInstall = true; else if (arg === '--node-floor') opts.nodeFloor = true; else if (arg === '--clean') opts.clean = true; + else if (arg === '--pinned-bun') opts.pinnedBun = true; + else if (arg === '--path-bun') opts.pinnedBun = false; else if (arg === '--only') opts.only = (args[++i] ?? '').split(',').filter(Boolean); else if (arg.startsWith('--only=')) @@ -156,6 +167,10 @@ Runs every blocking step of .github/workflows/ci.yml against the working tree. --clean Delete every dist/ and *.tsbuildinfo first, so the run starts from the state CI checks out. Catches missing build prerequisites that a warm tree hides. Costs ~40s. + --path-bun Run on PATH's bun instead of .bun-version's (${PINNED_BUN}). + The pinned Bun is the DEFAULT: Bun's fetch and node:http differ + between releases enough to pass locally and fail on CI. Use this + only to check whether a newer Bun fixes something. --tail N Lines of a failing step's log to print (default 30). --list List step ids and exit. @@ -178,7 +193,45 @@ function selectSteps(opts) { return steps; } -function run(step, tail) { +// The pinned Bun is the default, not an opt-in. `.bun-version` is what `setup-bun` resolves, and +// Bun's `fetch` and `node:http` are independent implementations that move between releases -- a +// rehearsal on a different one is not a rehearsal. Phase 8a lost a CI round to exactly that: three +// transport rows that pass on 1.4.0 fail on the pinned 1.3.14, two of them from malformed HTTP +// framing the newer Bun emits correctly. +// +// Prepending to PATH rather than wrapping each command in `mise x`: a root script like `typecheck` +// shells out to `bun run build:deps`, which shells out again. Only the environment reaches all of +// them. +function pinnedBunEnv() { + const active = spawnSync('bun', ['--version'], {encoding: 'utf8'}); + if (active.status === 0 && active.stdout.trim() === PINNED_BUN) { + stdout.write(`bun: ${PINNED_BUN} on PATH already matches .bun-version\n`); + return null; + } + const probe = spawnSync('mise', ['where', `bun@${PINNED_BUN}`], { + encoding: 'utf8', + }); + if (probe.status !== 0) { + // Loud, because the run that follows is measuring a runtime CI will not use. Not fatal: a + // preflight on the wrong Bun still catches everything that is not runtime-specific, and + // refusing to run at all would be worse than running with the caveat stated. + stdout.write( + `\n!! bun: .bun-version pins ${PINNED_BUN}; PATH has ` + + `${active.stdout.trim() || 'an unknown version'}, and mise cannot supply the pinned one.\n` + + ' Steps will run on the WRONG Bun — runtime-specific failures may not reproduce.\n' + + ` Fix with: mise install bun@${PINNED_BUN}\n\n`, + ); + return null; + } + const bin = `${probe.stdout.trim()}/bin`; + stdout.write( + `bun: pinning every step to ${PINNED_BUN} from .bun-version ` + + `(PATH has ${active.stdout.trim() || 'unknown'})\n`, + ); + return {...env, PATH: `${bin}:${env.PATH ?? ''}`}; +} + +function run(step, tail, childEnv) { const started = Date.now(); // `2>&1` inside the shell rather than two piped streams: spawnSync hands back stdout and stderr // as separate buffers, and concatenating them puts bun's own `$ script` echo *after* the compiler @@ -192,6 +245,7 @@ function run(step, tail) { // stalls behind it, which reads as "still running" and is the one outcome worse than a red run. timeout: STEP_TIMEOUT_MS, killSignal: 'SIGKILL', + ...(childEnv ? {env: childEnv} : {}), }); const seconds = Math.round((Date.now() - started) / 1000); const output = `$ ${step.cmd}\n\n${result.stdout ?? ''}${result.stderr ?? ''}`; @@ -254,7 +308,7 @@ function report(results, skipped, opts) { return 1; } -function runNodeFloor(opts) { +function runNodeFloor(opts, childEnv) { const managers = [ [ 'mise', @@ -288,6 +342,7 @@ function runNodeFloor(opts) { cmd: found[1], }, opts.tail, + childEnv, ); } @@ -320,6 +375,7 @@ function cleanArtifacts() { mkdirSync(LOG_DIR, {recursive: true}); const steps = selectSteps(opts); if (opts.clean) cleanArtifacts(); +const childEnv = opts.pinnedBun ? pinnedBunEnv() : null; stdout.write( `CI preflight — ${steps.length} step(s) from .github/workflows/ci.yml, in ${cwd()}\n`, ); @@ -333,7 +389,7 @@ for (const step of steps) { continue; } stdout.write(` ... ${step.id}\n`); - const result = run(step, opts.tail); + const result = run(step, opts.tail, childEnv); results.push(result); if (!result.ok && step.id === 'build') buildFailed = true; // A frozen-lockfile failure means the dependency tree on disk is not the one CI installs. @@ -347,7 +403,7 @@ for (const step of steps) { } if (opts.nodeFloor && !buildFailed) { - const floor = runNodeFloor(opts); + const floor = runNodeFloor(opts, childEnv); if (floor) results.push(floor); } else if (!opts.nodeFloor && results.some(r => r.id === 'test:node')) { const major = Number(version.slice(1).split('.')[0]); diff --git a/CLAUDE.md b/CLAUDE.md index 76baa6b..9a5ab7a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,6 +51,10 @@ for d in packages/*/; do grep -rhoE "from '@dexpace/[a-z-]+'" "$d/src" | sort -u `node .claude/skills/ci-preflight/run-ci.mjs --clean` is what catches a missing entry — it sweeps every `dist/` and `*.tsbuildinfo` first, so the run starts from the tree CI checks out rather than a warm one. +It also pins every step to `.bun-version`'s Bun by default (via mise, falling back to PATH's with a loud +banner): CI resolves that file, and Bun's `fetch`/`node:http` differ enough between releases that Phase 8a's +transport rows passed on 1.4.0 and failed three ways on the pinned 1.3.14. `--clean` plus that default is the +difference between "the gates pass here" and "CI will be green". `bun test` runs the unit suite on **Bun** and is scoped to `packages/` (`bunfig.toml`'s `[test] root`). **It needs `bun run build` to have run first**, from Phase 6a on: `@dexpace/codec-json`'s tests reach core diff --git a/packages/transport-conformance/src/fixtures.ts b/packages/transport-conformance/src/fixtures.ts index 06cb45d..cf8fc78 100644 --- a/packages/transport-conformance/src/fixtures.ts +++ b/packages/transport-conformance/src/fixtures.ts @@ -43,7 +43,15 @@ function route( case '/early-response': // Answers without ever draining the request body, so a streaming producer is still running // when the response is delivered -- the window TRANSPORT-19's post-delivery clause lives in. - res.writeHead(413, {'content-type': 'text/plain'}); + // + // `connection: close` because RFC 7230 6.3 requires it of a server that answers before + // draining the request body: the unread remainder would otherwise sit in a reusable socket and + // be parsed as the start line of whatever request came next. It is hygiene, not a fix -- the + // client is free to ignore it, and Bun 1.3.14 did, serving the resulting 400 to a later row + // from its own pool no matter what this server did (a socket destroy here changed nothing). + // The row that provokes this therefore runs against its own origin -- see `isolatedUrl` in + // run-suite.ts, which is what actually contains it. + res.writeHead(413, {'content-type': 'text/plain', connection: 'close'}); res.end('too large'); return; case '/vendor-status': @@ -52,11 +60,19 @@ function route( return; case '/malformed-content-type': // TRANSPORT-27: a syntactically invalid media type and a chunked (length-less) body. - res.writeHead(200, { - 'content-type': 'not-a-media-type', - 'transfer-encoding': 'chunked', - }); - res.end('body'); + // + // The chunked framing is *derived*, never declared: writing the body before `end()` with no + // declared length leaves the server no way to precompute one, so it must fall back to chunked. + // Setting `transfer-encoding: chunked` by hand looks more direct and is a trap -- Bun 1.3.14 + // (`.bun-version`, so exactly what CI runs) honours the header in the status line but still + // appends `Content-Length: 4` and writes the body UNCHUNKED. That response is malformed twice + // over, and the two transports disagree about how: undici rejects it with "Response body length + // does not match content-length header", while Bun's own `fetch` blocks for the chunk framing + // that never arrives until the test times out. Bun 1.4.0 emits it correctly, which is why this + // reproduced only on CI. Verified byte-for-byte on Bun 1.3.14, Bun 1.4.0, and Node 20.3.0. + res.writeHead(200, {'content-type': 'not-a-media-type'}); + res.write('body'); + res.end(); return; case '/drip': { // Headers land immediately, the body trickles: the shape a lazily-streamed response body and an diff --git a/packages/transport-conformance/src/run-suite.ts b/packages/transport-conformance/src/run-suite.ts index 57833b6..316110a 100644 --- a/packages/transport-conformance/src/run-suite.ts +++ b/packages/transport-conformance/src/run-suite.ts @@ -41,6 +41,21 @@ interface SuiteContext { readonly capabilities: TransportCapabilities; /** Resolves a fixture path against the server started in `beforeAll`; read lazily, at run time. */ url(path: string): string; + /** + * The same fixture, on a second origin, for rows that deliberately leave a connection unusable. + * + * A row that makes the server answer before draining the request body strands the remainder of + * that body in the socket. Whether the client then reuses it is the client's business, and a + * client that gets it wrong does not fail *here* -- it fails in whichever later row is handed the + * poisoned connection, which is a debugging problem of a different order. Bun 1.3.14 gets it + * wrong: it serves the resulting `400` from its pool, so `a per-call timeout is retryable` saw a + * 1ms resolve some thirty rows downstream. Neither `connection: close` nor destroying the socket + * server-side prevents it -- verified -- because the decision is entirely the client's. + * + * A separate origin is therefore the only thing this suite controls that contains the blast + * radius. Pathological rows get their own pool; every other row keeps the shared one. + */ + isolatedUrl(path: string): string; } /** @@ -281,7 +296,8 @@ function registerProducerRows(ctx: SuiteContext): void { }; const request = Request.newBuilder() .method('POST') - .url(ctx.url('/early-response')) + // Quarantined: this row is the one that strands a request body mid-socket. + .url(ctx.isolatedUrl('/early-response')) .body(body) .build(); const response = await transport.send(request); @@ -598,17 +614,21 @@ export function runTransportConformanceSuite( ): void { describe(`${name} conformance (TRANSPORT-1..30, SEAM-12/16/30, NFR-15)`, () => { let server: TestServer; + let isolated: TestServer; beforeAll(async () => { server = await startFixtureServer(); + isolated = await startFixtureServer(); }); afterAll(async () => { await server.close(); + await isolated.close(); }); const ctx: SuiteContext = { makeTransport, capabilities, url: path => `${server.url}${path}`, + isolatedUrl: path => `${isolated.url}${path}`, }; registerDispatchRows(ctx); registerStatusRows(ctx);