Retry surfaces the typed error (#72) by Wahbeh-Mohammad · Pull Request #88 · dexpace/nodejs-sdk

Wahbeh-Mohammad · 2026-09-05T07:28:42Z

Closes #72. Part of the audit remediation umbrella #67, milestone 3, wave 3. Decision D10 of
docs/audit-67-decisions.md.

What changed

retryStep and dispatchWithRetry now surface the final attempt's own error, class and
identity untouched. withTrail became attachTrail
(packages/core/src/retry/engine.ts:269-279): it returns the outcome unchanged and records the
prior attempts in a side table instead of wrapping the error in suppress(outcome.error, folded, 'retry attempts exhausted').

The wrapper made the surfaced class a function of how many attempts ran — one attempt gave
TransportFailureError, three gave a SuppressedError holding it at .error. The sharp edge is
the cancellation row: abortToSdkError maps a backoff abort to CancellationError at
engine.ts:394, and the wrapper undid that mapping on the very next line, so
instanceof CancellationError was false for every reachable backoff cancellation — a cancelled
wait always has a non-empty trail. That is exactly what XCUT-1's conformance clause ("assert the
surfaced error is the cancellation type") is for.

This is a correction rather than a deviation. RETRY-34 says the prior failures are "attached to the
surfaced exception as suppressed", which is the JVM's addSuppressed: the exception stays what it
is and grows a list. It never asked for the exception to be replaced by a container. No
docs/deviations.md row (D10).

The trail

New @public export from @dexpace/core:

retryAttempts(error: unknown): readonly unknown[]

Oldest first, the surfaced instance excluded (RETRY-34's skip-self clause), frozen, [] for
anything that never went through a retry loop. Backed by a module-private WeakMap in the new
packages/core/src/retry/attempt-trail.ts, written once per terminal failure.

A side table rather than an own property on the error, because the engine did not construct the
throwable it is surfacing: it may be frozen or non-extensible (so defineProperty in the failure
path would itself throw and replace the failure the caller cares about), it may be a primitive
(which carries nothing at all — those pass through unannotated, with no trail entry), and
.suppressed already means "the one secondary" on SuppressedErrorLike. A WeakMap rather than a
Map so the entry dies with the error instead of pinning every failed request's error graph.

One behaviour beyond the letter of D10, flagged: an empty trail deletes any entry a previous run
left. Error singletons are ordinary in fakes and in transports that reuse one instance, and
RETRY-34's "on eventual success the prior trail MUST be discarded" buys nothing if the next run to
surface that same instance still reports the old one.

suppress() keeps its RECOV-12 job — withReleaseFailure still pairs a release failure with the
primary it must not mask, in this file and in six other subsystems.

Test rows added

Every row below was confirmed red against the parent commit's engine and green against this one
(measured by checking out HEAD~1's engine.ts, rebuilding, and running the suites).

File	Row
`packages/core/src/retry/attempt-trail.test.ts` (new, 13 cases)	the accessor: oldest-first, frozen, identity-keyed (no `cause` walk), primitives, frozen errors, thrown functions, trail copied, empty clears, latest wins
`packages/core/src/retry/engine.test.ts`	the final attempt's error is surfaced by identity, priors beside it
`packages/core/src/retry/engine.test.ts`	`instanceof TransportFailureError` for `maxAttempts` 1 and 3, with the dispatch count proving the budgets differed
`packages/core/src/retry/engine.test.ts`	abort during a 60s backoff at `maxAttempts: 5` → `instanceof CancellationError`, trail `[first]`
`packages/core/src/retry/engine.test.ts`	success discards the trail; skip-self leaves `[]`; three attempts list flat, oldest first, not a nested pair chain
`tests/conformance/xcut/retry-safety.conformance.test.ts`	composed pipeline, GET at a closed port: `TransportFailureError` at `maxAttempts` 1 and 3
`tests/conformance/xcut/retry-safety.conformance.test.ts`	two priors reachable via `retryAttempts()`, self excluded
`tests/conformance/xcut/cancellation-and-timeout.conformance.test.ts`	`expect(surfaced).toBeInstanceOf(CancellationError)` at the top level — the existing row asserted only on a chain walk, and its own comment said why: the top level was the wrapper. A caller writing `catch` has no walk
`tests/conformance/xcut/cancellation-and-timeout.conformance.test.ts`	the provoking 500 survives in the trail as a buffered `HttpStatusError` (RECOV-16)
`tests/node-conformance/retry.test.mjs`	"same wrapper on both runtimes" → "no wrapper on either", plus `instanceof CancellationError` over a real `AbortSignal` and a real timer

chainOf's doc comment in the cancellation suite is rewritten rather than deleted: the walk still
earns its place proving the raw abort survived as cause and that no TimeoutError hides one hop
down (XCUT-3), but it is no longer the only assertion.

Docs and API surface

packages/core/etc/core.api.md regenerated (bun run api:local in packages/core) — one added
line, retryAttempts.
retryStep's TSDoc, retry-dispatch.ts's @throws prose, and
RetryDiscardedResponseError's "suppressed trail" wording.
docs/sdk-documentation/pipelines.md — a new first retry bullet with a worked example that
check-fences.mjs typechecks against the built package.
docs/sdk-documentation/errors.md — :188's "suppressed trail" wording, plus a statement in the
CancellationError/TransportFailureError section that installing a retry pillar does not change
either instanceof check.
docs/sdk-documentation/write-a-response-handler.md — RECOV-12's release pairing is now
documented as the only reason this SDK builds a SuppressedError. Verified against every
suppress() and withReleaseFailure() call site in packages/core/src.

Deviations from the task file

One file edited outside the stated partition: tests/node-conformance/retry.test.mjs. Its
header names RETRY-34's suppress() branch as runtime-divergent point 2, and its case asserted
outcome.error.name === 'SuppressedError'. That case is red the moment D10 lands, so test:node
and the preflight cannot be green without it. No wave-3 sibling touches the Node tree (D2 gives #74
auth/ plus transport-conformance, #75 rx/), so the conflict risk the partition exists to
prevent is nil here. Reported rather than silently absorbed.

Found stale, NOT fixed (outside the partition)

docs/deviations.md:144-146, item 3, carries three file:line citations this PR shifts:

Citation	Now
`packages/core/src/retry/engine.ts:358` (`runWithRetry`)	`:367`
`packages/core/src/retry/retry-step.ts:142`	`:150`
`packages/core/src/retry/retry-dispatch.ts:53`	`:55`

docs/deviations.md is explicitly not this task's, and it is the wave-3 merge seam #74 and #75
append to. Item 3 sits mid-file, so a fix here would add a second, different conflict surface.
Flagging for the supervisor. Nothing gates these citations — probe.mjs reports "no drift found"
and no CI step reads them.

docs/audit-67-decisions.md:157 likewise cites retry/engine.ts:385 and withTrail at :386;
that is the supervisor-owned ledger and is now historical rather than current.

Deferred — release machinery

Suspended for this run under D1. Recoverable:

Minor changeset for @dexpace/core. Two consumer-visible changes: (a) a new @public export,
retryAttempts(error: unknown): readonly unknown[]; (b) what a retrying pipeline throws once it
gives up changes class — TransportFailureError / CancellationError / HttpStatusError where a
SuppressedError used to be. Breaking for anyone reading .error off the caught throwable,
which the shipped docs told them to do for the retry trail; retryAttempts() is the replacement.
Minor rather than major only because the package is pre-1.0.
Patch note for the shipped .d.ts prose changed on retryStep and RetryDiscardedResponseError.
docs/first-release.md untouched, though "what the retry pillar throws" is squarely in its
"free before the first version bump" class.

Gate

node .claude/skills/ci-preflight/run-ci.mjs --clean, once, at the end, no --node-floor
(concurrent agents). Bun pinned to 1.3.14 from .bun-version.

clean: removed 29 build artifact(s) — starting from CI's state
bun: pinning every step to 1.3.14 from .bun-version (PATH has 1.4.0)
CI preflight — 20 step(s) from .github/workflows/ci.yml, in /home/mohammad/Projects/dexpace/wt-72

  PASS  install                    0s
  PASS  verify:knowledge-structure   0s
  PASS  typecheck                 23s
  PASS  lint                      12s
  PASS  build                      9s
  PASS  test                       6s
  PASS  test:scripts               0s
  PASS  api                       16s
  PASS  lint:publish               7s
  PASS  verify:dual-consumption    0s
  PASS  verify:consumer-types      1s
  PASS  verify:seam-1              0s
  PASS  verify:sse-37              0s
  PASS  verify:runtime-floor       0s
  PASS  verify:test-partition      0s
  PASS  test:examples              0s
  PASS  verify:import-cycles       0s
  PASS  verify:reproducible-build  34s
  PASS  audit                      0s
  PASS  test:node                  0s

CI preflight: all 20 steps passed.

test:node ran on Node v26.2.0. CI also runs the 20.3.0 floor leg, which is the one that takes
suppress()'s fallback branch — the branch this PR removes from the retry path.

The retry engine wrapped its terminal failure in `suppress(error, folded, 'retry attempts exhausted')`, which made the surfaced CLASS a function of how many attempts ran: one attempt gave `TransportFailureError`, three gave a wrapper with it at `.error`. The row that catches it is XCUT-1's conformance clause -- "assert the surfaced error is the cancellation type" -- because a cancellation during backoff always has a non-empty trail, so `abortToSdkError` mapped the abort to `CancellationError` at engine.ts:385 and the wrapper undid the mapping at :386. RETRY-34 asks for the prior failures to be "attached to the surfaced exception as suppressed", which is the JVM's `addSuppressed`: the exception stays what it is and grows a list. It does not ask for the exception to be replaced by a container. So `withTrail` becomes `attachTrail`: the outcome's error is returned unchanged, and the priors go into a module-private WeakMap in the new `retry/attempt-trail.ts`, read back through a `@public` `retryAttempts(error)`. A side table rather than an own property, because the engine did not construct the throwable it is surfacing: it may be frozen (so `defineProperty` in the failure path would itself throw and replace the failure the caller cares about), it may be a primitive (which carries nothing at all, and passes through unannotated), and `.suppressed` already means "the one secondary" on `SuppressedErrorLike`. `suppress()` keeps its RECOV-12 job -- `withReleaseFailure` still pairs a release failure with the primary it must not mask. RETRY-34's skip-self guard is unchanged, and an empty trail now DELETES any entry a previous run left, so a transport reusing one error instance reports the run that just surfaced it rather than a stale one. Decision D10 of docs/audit-67-decisions.md. Refs #72, #67.

Four rows across the two suites the issue names, all four red against the parent commit's engine and green against this one: retry-safety.conformance.test.ts - one GET at a closed port surfaces `TransportFailureError` for maxAttempts 1 AND for 3; the dispatch count is what proves the budgets actually differed (XCUT-1) - the two priors are reachable through `retryAttempts()`, oldest first, with the surfaced instance excluded (RETRY-34) cancellation-and-timeout.conformance.test.ts - `expect(surfaced).toBeInstanceOf(CancellationError)` at the TOP level, which is XCUT-1's conformance clause unqualified. The existing row asserted only on a chain walk, and its own comment said why: the top level was the `SuppressedError`. A caller writing `catch` has no walk. - the 500 that provoked the retry is still reachable in the trail as a buffered `HttpStatusError` (RETRY-34/RECOV-16) `chainOf`'s doc comment is rewritten rather than deleted: the walk still earns its place proving the raw abort survived as `cause` and that no `TimeoutError` hides one hop down (XCUT-3), but it is no longer the only assertion. Refs #72, #67.

…mpts reads the rest - pipelines.md gains the note as the FIRST of the retry bullets, with a worked example that `check-fences.mjs` typechecks against the built package. The lead sentence said "two notes each"; it is three now. - errors.md: the `CancellationError`/`TransportFailureError` narrowing section states outright that installing a retry pillar does not change either check, which is the whole point of #72. The `RetryDiscardedResponseError` row at :188 stops calling the trail "suppressed". - write-a-response-handler.md: RECOV-12's release pairing is now documented as the ONLY reason this SDK builds a `SuppressedError`. Verified against every `suppress()` and `withReleaseFailure()` call site in packages/core/src, all of which are a close() that threw with an error already in flight. Both new cross-links resolve against pipelines.md's real headings (checked by script, not by eye). Refs #72, #67.

…pper" `tests/node-conformance/retry.test.mjs` names four runtime-divergent points in its header, and point 2 was RETRY-34's trail going through `suppress()` -- whose native-vs-fallback branch is decided by the runtime, and whose two legs this suite's matrix actually runs (`lts/*` has the global, the pinned `20.3.0` floor does not). #72 takes that branch off the retry path, so the case that asserted "the wrapper has the same shape on either runtime" would now be asserting a shape nothing builds. It becomes the stronger claim: neither leg produces a wrapper, `outcome.error` IS the last attempt's error, and `retryAttempts()` reads the trail through the `@dexpace/core` specifier and the built `dist/`, as a consumer does. The real-timer abort case gains one line: `outcome.error instanceof CancellationError`, over a real `AbortSignal` and a real `setTimeout`, which is the half the unit suite's injected clock cannot reach. OUT OF THE TASK FILE'S STATED PARTITION, deliberately and reported: the partition lists the two `tests/conformance/xcut/` files but not the Node tree, and this case asserts the exact behaviour D10 changes, so `test:node` (and therefore the preflight) is red without it. No wave-3 sibling touches this file -- D2 gives #74 auth plus transport-conformance and #75 rx. Refs #72, #67.

…metic `retryAttempts(caught).length + 1` was documented as "how many sends the pillar made". It is not, and it is wrong on exactly the path #72 exists for: a cancellation observed at the RETRY-32 gate is synthesized by the engine (`engine.ts:394`), never raised by a send, so the trail already covers every attempt and the sum overstates by one. Two more reachable paths make the same sum wrong, both in the engine's own catch: `stampAttempt` throws before `dispatch` is called (`runAttempt` stamps first), and a `Clock.sleep` rejecting for something other than an abort fails after the attempt it followed is already in the trail. And narrowing the catch does not rescue it. `abortToSdkError` branches on `isTimeoutSignal` (`cancellation.ts:37-39`), so the RETRY-32 gate can synthesize a `TransportFailureError` too — which means `pipelines.md`'s worked example was wrong under the very class it narrowed to. That example no longer counts anything; it iterates the priors, which is what the accessor is for. Three prose sites reworded to state what the trail actually holds — one entry per attempt that failed BEFORE the surfaced error — plus the caveat: the surfaced error is an attempt's own only when it came from one. Two new engine cases pin it mechanically rather than by assertion in prose: a caller abort and a timeout abort each after ONE send, both asserting `dispatch.sends === 1` beside `retryAttempts(...).length === 1`, so any future `+ 1` claim has a red test under it. `core.api.md` unchanged: TSDoc prose only, api-extractor reports no signature change. Round 2 of #72. Refs #72, #67.

Wahbeh-Mohammad added 5 commits September 5, 2026 10:48

Wahbeh-Mohammad force-pushed the audit/67/72-retry-typed-error branch from 68f904c to 4578841 Compare September 5, 2026 07:48

Wahbeh-Mohammad merged commit 2ea8b3e into audit/remediation-67 Sep 5, 2026
3 checks passed

This was referenced Sep 5, 2026

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

Merged

Retry surfaces the typed error, not a SuppressedError wrapper #72

Closed

Wahbeh-Mohammad deleted the audit/67/72-retry-typed-error branch September 7, 2026 18:42

Navigation Menu

Uh oh!

Retry surfaces the typed error (#72) - #88

Retry surfaces the typed error (#72)#88
Wahbeh-Mohammad merged 5 commits into
audit/remediation-67from
audit/67/72-retry-typed-error

Wahbeh-Mohammad commented Sep 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Wahbeh-Mohammad commented Sep 5, 2026

What changed

The trail

Test rows added

Docs and API surface

Deviations from the task file

Found stale, NOT fixed (outside the partition)

Deferred — release machinery

Gate

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant