diff --git a/.drive/projects/forcing-function-apps/design-notes.md b/.drive/projects/forcing-function-apps/design-notes.md index 1ea4fbaf..657bdf7c 100644 --- a/.drive/projects/forcing-function-apps/design-notes.md +++ b/.drive/projects/forcing-function-apps/design-notes.md @@ -191,3 +191,57 @@ shipped inside Part B's refactor (implicit create, read-creates, contentType dropped), the complete unenforced-invariant inventory (T1–T9), deviations from the design doc, and decisions R1–R6 awaiting his ruling. No further dispatches until he rules. + +## RPC idempotency keys — settled design (2026-07-17, Will) + +Supersedes the `idempotent: true` per-method boolean considered earlier the +same day. The RPC protocol requires an `Idempotency-Key` on every call +(client-minted per logical call, reused across retries); `serve()` enforces +it, runs single-flight per key, and replays completed answers within the +retry envelope; handlers may read `ctx.idempotencyKey` for their own durable +control but need not. Rationale: a boolean is a human claim the framework +cannot check — the key is a mechanism it enforces, making every method +safely retryable with no declaration. Scope calibration from Will: this +protocol serves same-network service-to-service calls and guards the +cold-start bug specifically; in-memory control is sufficient, and the +cross-instance residual window is documented and accepted, not closed. +The bounded retry becomes permanent protocol semantics — the PRO-217 canary +retires itself when the platform heals, never the retry or the keys. +Slice: [slices/rpc-cold-start/](slices/rpc-cold-start/spec.md). + +### Amendment (2026-07-17, later): rename, scope citation, absorbed fixes + +The kind was renamed to service-rpc (#131, ADR-0036) while this slice was in +flight; the branch is rebased and all paths follow the new package and the +`src/exports/` entrypoint convention (ADR-0035). Two corrections from Will: + +- The accepted residual is no longer justified by "same network" — that + framing contradicts `connection-contracts.md` § Purpose and scope, which + defines internal as membership of the application's topology and puts + cross-target, cross-network edges explicitly in scope, with robustness + calibrated per edge against the named failure modes of the targets + carrying it. The slice now names its concrete failure (PRO-217's close + during connection establishment, before any handler runs) and names the + residual it does not close (a retry reaching a different instance than the + one that applied the call), whose escalation path is the handler's own + durable control via `ctx.idempotencyKey`. +- Three fixes promised when #114 was declined touch the same files this + slice rewrites, so they are absorbed rather than left to collide: a + request body size limit, masking handler exception messages instead of + returning them to callers, and dropping the client's redundant second + validation of responses. + +ADR-0037 is the number for the keyed protocol (0033–0036 taken); it ships +with the implementation in this slice's PR, never as a docs-only change. + +### RPC cold-start canary dropped (2026-07-20, Will accepted) + +The slice's own canary was built, run live, and dropped on the evidence +(revert `a46abfa`). The auth service boots in under ~1.5s (no state to +restore), narrower than the 2s coldness-proof margin, so 14 forced races +certified as cold zero times. The decisive reason is not the margin, though: +the RPC idempotency retry is permanent protocol semantics (ADR-0037), so +there is no workaround for a canary to time the removal of — and PRO-217, a +platform ingress bug, is already watched by the streams canary, which catches +it well because streams has the long boot window RPC lacks. The slice ships +the keyed protocol; the streams canary stays the platform-wide PRO-217 signal. diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md new file mode 100644 index 00000000..d0de8663 --- /dev/null +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/plan.md @@ -0,0 +1,94 @@ +# Dispatch plan: rpc-cold-start (idempotency keys) + +Contract source: [spec.md](spec.md). Branch: +`claude/streams-cold-start-rpc-37e5c1`, rebased onto main at/after `6ec2625` +(the [#131](https://github.com/prisma/compose/pull/131) service-rpc rename). +Three dispatches, sequential; hostile reviewer round after D1+D2, then D3 +closes. Orchestrator owns all docs (`gotchas.md`, `docs/`, `.drive/`, and +ADR-0037) — implementers report staleness and never edit. Evidence rules from +the streams slice apply verbatim: raw program output only; every reported +number is checked against the code's real format strings before it is +believed. + +## D1 — the keyed protocol, plus the three absorbed #114 fixes + +**Outcome:** in `packages/0-framework/2-authoring/service-rpc/`: +`makeClient` mints one `Idempotency-Key` per logical call and reuses it +across a bounded retry (250 ms / ×2 / 5 s cap / 5 attempts / jitter; retry +network errors + 5xx + 429, never other 4xx). `serve()` enforces the key +(keyless requests opt out — served once, no dedup), single-flights per in-flight key, replays completed +2xx/4xx for ~60 s under a justified LRU bound, never caches 5xx/throws, and +passes `ctx.idempotencyKey` as an optional third handler argument after +`deps`. Absorbed in the same pass: the request body size limit (413, +enforced while reading, not trusting `content-length`), masking handler and +output-validation exception messages behind a generic 500 while logging the +real error, and deleting the client's redundant response re-validation +(with the now-unnecessary `MethodSchemas` cast if it falls out). No +prisma-cloud import; no `idempotent` flag anywhere. + +**Completed when:** every client, server, and type-level test in the spec is +green with teeth confirmed red-by-mutation (including same-key-across- +attempts, fresh-key-per-call, 5xx-not-cached, and exception-message-not- +returned); both RPC-consuming example suites pass unchanged; repo checks +green with casts delta ≤ 0; committed with DCO dual sign-off. + +## D2 — the canary (scripts + CI) — ATTEMPTED, THEN DROPPED (2026-07-20) + +**Outcome: no RPC canary ships.** The canary was built and run live; the +evidence retired it (reverted in `a46abfa`). Two reasons, the second +decisive: + +1. It cannot certify a cold start against `examples/storefront-auth`. The + auth service restores no state and boots in under ~1.5s — narrower than + the 2s clock-skew margin the coldness proof requires. The live run forced + 14 genuine races (touch landed 376–1185ms before the listening line, + every sample) but certified none, correctly refusing to shrink the margin + to make its own numbers pass. +2. A cold-start canary exists to signal when a platform workaround can be + deleted. The RPC idempotency retry is **permanent protocol semantics** + (ADR-0037), not a workaround — there is nothing for a canary to time the + removal of. PRO-217 itself is a platform ingress bug already watched by + the streams cold-start canary, which catches it well precisely because + streams has the long boot window RPC lacks. + +Superseded original intent (kept for the record): a sibling of the streams +cold-start canary probing the auth service's rpc endpoint with a bare +single-attempt `fetch`. If a future RPC edge with a real restore-from-store +boot exists, and only if the retry ever becomes removable, revisit with an +independently measured skew margin rather than the streams canary's borrowed +constant. + +### Original D2 outcome (not shipped) + +**Outcome:** `scripts/rpc-cold-start-canary.ts` + `-classify.ts` + unit +tests, inheriting the cold-start canary's proven contract wholesale +(promote-race trigger, ≥60 s spacing including sample #0, log-confirmed +coldness with the 2 s margin, 14-hold bug-gone budget, first-close early +exit, `MAX_RUN_MS`, requirable exits, and a bug-gone message that retires +the canary and its gotchas paragraph but NEVER the retry or keys); job in +`e2e-deploy.yml` over `examples/storefront-auth`, probing the auth service's +rpc endpoint with a bare single-attempt `fetch` and a manually minted key — +never through a framework client, which the new retry would mask. + +**Completed when:** classify tests green with confirmed teeth; +`test:scripts` green; at least one live run reports `bug-present` with raw +per-sample output (a clean run today means a broken canary — stop and +report, do not ship); workspace left clean with project counts; committed. + +## D3 — hostile review, live re-proof, records, PR + +**Outcome:** reviewer pass over D1+D2. Attack priorities: a repeated key can +never double-execute within an instance; a replay can never leak one logical +call's answer to another, across methods or callers; a keyless request opts +out cleanly (served once, no dedup); the body cap holds without a truthful `content-length`; +no handler exception text reaches a caller while operators keep it; the +canary cannot be masked by the retry machinery; every reported number is +real. Findings closed. Full live round (deploy storefront-auth, canary +verify, destroy, zero leaks). + +Then the orchestrator writes, in this same PR: **ADR-0037** (calls carry an +idempotency key; the framework dedupes; retries are protocol semantics, with +the named residual and the connection-contracts scope calibration as its +reasoning), the `connection-contracts.md` protocol update, and the +`gotchas.md` PRO-217 service-RPC face. PR opened against main with the slice +narrative; review requested from Will. No auto-merge; merge on his word. diff --git a/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md new file mode 100644 index 00000000..baa796e5 --- /dev/null +++ b/.drive/projects/forcing-function-apps/slices/rpc-cold-start/spec.md @@ -0,0 +1,209 @@ +# Slice: service-RPC idempotency keys — safe retries for every call, plus the PRO-217 canary + +## At a glance + +`makeClient` carries no cold-start handling, so every service-to-service RPC +edge hits PRO-217's intermittent socket close raw on a cold target — and +PRO-217 is live (reproduced repeatedly on 2026-07-17 against log-confirmed +cold starts). A blanket retry was previously unsafe because a retried POST +could double-execute a write. + +Settled design (Will, 2026-07-17, superseding an earlier per-method +`idempotent: true` boolean): **the protocol requires an idempotency key on +every call**, and the framework implements idempotency control on both ends. +With the mechanism in place every method is safely retryable and no +per-method declaration exists — a boolean is a human claim the framework +cannot check; a key is a mechanism it enforces. + +**Package note:** the kind was renamed by [#131](https://github.com/prisma/compose/pull/131) +(ADR-0036). Everything below lives in +`packages/0-framework/2-authoring/service-rpc/`, package +`@internal/service-rpc`, public import path `@prisma/composer/service-rpc`, +with the public entrypoint at `src/exports/index.ts` (ADR-0035). Branch must +be rebased onto main at or after `6ec2625` before any code is written. + +## The design + +### Protocol + +- Every request `makeClient` sends carries an `Idempotency-Key` header: a + UUID minted **once per logical call** and reused **byte-identically across + every retry of that call**. Two separate logical calls never share a key. + `crypto.randomUUID()` — no new dependency. +- The generated client always sends the key. A request that arrives without + one (a hand-rolled or older caller) is served once, with no dedup or replay, + rather than rejected — it has opted out. `ctx.idempotencyKey` is `undefined` + for such a call. (Superseded the earlier "keyless → 400" rule, on the + grounds that the safe path is the always-keyed generated client, so keyless + requests never came from it and opt-out degrades transparently.) + +### Client (`makeClient`) + +- Every method gets a bounded retry: 250 ms initial, ×2, 5 s cap, 5 + attempts, jittered (the streams client's numbers; read its `client.ts` for + shape but **do not import from prisma-cloud** — service-rpc is + framework-layer and target-agnostic). Retry thrown network errors, 5xx and + 429; never any other 4xx. Same key on every attempt. +- No per-method configuration; no `idempotent` flag exists in this design. + +### Server (`serve()`) + +- **Single-flight per in-flight key:** a duplicate arriving while the first + attempt is still executing waits for it and receives the same response — + the handler runs exactly once. +- **Replay within the retry envelope:** completed 2xx and 4xx responses (they + are answers) are cached ~60 s and replayed byte-identically for a repeated + key, under an LRU bound whose size is justified in a comment — this cache + is resident in every RPC provider. +- **5xx and thrown errors are not cached** — they are the retryable + outcomes; a retry must re-execute. +- A replayed answer must be structurally incapable of crossing methods. +- **Handler context:** handlers gain an optional second argument — + `(input, deps, ctx)` carrying `ctx.idempotencyKey`, appended after the + existing `deps` argument so every existing handler still typechecks and + runs unchanged. Handlers may ignore it. + +### Robustness calibration, and the residual this leaves + +`docs/design/10-domains/connection-contracts.md` § *Purpose and scope* is the +governing scope statement: **internal means inside the application's +topology, not co-located on one network.** Components may come from +different targets, and such an edge necessarily crosses networks — that is in +scope. Protocol guarantees are calibrated per edge against **the named +failure modes of the targets carrying it**, and any proposal adding +robustness names the concrete failure it guards. + +The concrete failure this slice guards is **PRO-217**: the Prisma Compute +ingress closing a first-touch connection while a scale-to-zero target boots. +That close happens during connection establishment, **before any handler +runs** — nothing was applied, so retrying it cannot duplicate work +irrespective of dedupe. The keys additionally close the narrower window where +a response is lost after the handler applied it and the retry reaches the +**same** instance. + +**The residual, named not hidden:** in-memory control cannot cover a retry +that lands on a *different* instance than the one that applied the call +(instance death mid-request, or any edge where retries may be routed +elsewhere). This slice does not close it, and adds no durable store. The +escalation path is per-edge and belongs to the handler: `ctx.idempotencyKey` +lets a handler write the key inside its own transaction and obtain exactly- +once where its target's failure modes warrant it. Framework-side durable +dedupe is not proposed here — no concrete failure mode of a currently +supported target names it. + +### Status of the retry: permanent, not a compensation + +Bounded retry over keyed calls is correct protocol semantics for this kind — +it is **not** deleted when the platform heals. The canary's bug-gone message +retires the canary itself, its gotchas paragraph, and PRO-219's urgency +framing — never the retry or the keys. No comment on the retry or the key +machinery may say "remove when PRO-217 is fixed". + +## Absorbed fixes (promised on [#114](https://github.com/prisma/compose/pull/114), same files) + +These three were committed to when the oRPC PR was declined; they touch the +exact code this slice rewrites, so they land here rather than in a colliding +PR. + +1. **Request body size limit in `serve()`.** `await req.json()` is currently + unbounded. Cap it, answer over-limit with **413**, and do not trust + `content-length` alone — a lying or absent header must not bypass the cap, + so the bound is enforced while reading. Pick a limit appropriate to + internal RPC payloads and justify it in one comment. +2. **Stop leaking handler exception messages to callers.** The 500 path + currently returns `err.message` in the response body — an internal + exception string handed to the caller. Mask it: a generic failure message + to the caller, the real error logged server-side (`console.error`) so + operators keep it. Output-validation failures (a provider bug) are masked + the same way but logged distinguishably. **Input-validation 400s keep + their detail** — that is the caller's own malformed request and the + message is how they fix it. +3. **Remove the client's redundant second validation of responses.** + `serve()` already validates a handler's output against the method schema + before responding; `makeClient` re-validating it is duplicated work on + every call. Both ends of every edge are framework-provisioned (see the + scope statement above), so the second pass guards nothing. Expect this to + leave the client needing only method *names* off `__cmp` — if the + `MethodSchemas` blindCast in `client.ts` becomes unnecessary, delete it + (a cast-ratchet reduction, not a neutral change). + +## The canary (PRO-217, service-RPC face) + +A sibling of `scripts/cold-start-canary.ts`, inheriting every rule of its +2026-07-17 rebuild — requirements, not suggestions: + +- Fresh target via create → upload → start → **race the promote call** + (never wait for `running`); **≥60 s between samples, including before + sample #0**; coldness proven from the deployment's own boot log (2 s + cross-clock margin), never inferred from latency; `bug-gone` requires 14 + confirmed cold-start holds (20% target close rate, ≤5% false-clean); any + close is decisive; first close exits early; `MAX_RUN_MS` self-stop under + the job timeout; requirable exits (present → 0, gone → 1 with the cleanup + message, inconclusive → 0 + `::warning::`). +- **Probes the target's rpc endpoint directly with a bare single-attempt + `fetch`** carrying a manually minted key — every framework edge now + auto-retries, so a probe through a consumer would be masked by the very + machinery this slice ships. The raw platform behavior must stay observable. +- Rides `examples/storefront-auth`'s deployed auth service through the + existing deploy-verify-destroy action; own `-classify.ts` + unit tests; own + job in `e2e-deploy.yml`; NOT required until Will adds it. + +## Verification bar + +Client tests (counts asserted at a stub transport, the streams append-test +pattern — assert what the transport received, not what the client claims): + +- 503-then-success → resolves, **exactly two** requests, **same key on both**. +- Two separate logical calls → **different** keys. +- 404 → rejects after **exactly one** request; re-assert after a settle + window so a background retry would be caught. +- Thrown network error then success → resolves. + +Server tests: + +- Repeated key after completion → handler ran **once**, response replayed + byte-identically. +- Concurrent same-key → **one** execution (single-flight). +- Keyless → served once, no dedup (handler runs each time); `ctx.idempotencyKey` is `undefined`. +- 5xx not cached → a same-key retry re-executes. +- LRU bound evicts. +- A replay can never answer a different method. +- Over-limit body → 413, including when `content-length` is absent or lies. +- A handler throw → 500 whose body does **not** contain the exception + message, with the real error logged. +- Input-validation failure → 400 that **does** carry its detail. + +Type-level (`test-d`): existing two-argument handlers `(input, deps)` still +typecheck; `ctx.idempotencyKey` is typed on the three-argument form. + +Each of the above has its teeth confirmed by mutation — break the behavior, +watch that specific test fail, restore. Explicitly including: delete the +retry; mint a fresh key per attempt instead of per call; cache 5xx; return +the exception message. + +Also green: `turbo run typecheck test`, `pnpm lint` (exit 0), `pnpm +lint:casts` (**delta ≤ 0**), `pnpm lint:deps`, `pnpm test:scripts`, and both +RPC-consuming example suites (`examples/store`, `examples/storefront-auth`) +passing **unchanged** — proof the handler-context argument is non-breaking. + +Live: canary against a fresh deploy, raw output only. Expected verdict today +is `bug-present`; a clean run today means a broken canary — investigate, do +not report it as a result. + +## Records to write (orchestrator, in this PR — never a docs-only PR) + +- **ADR-0037** (0033–0036 are taken): service-RPC calls carry an idempotency + key; the framework dedupes; retries are protocol semantics. +- `docs/design/10-domains/connection-contracts.md`: the protocol section + gains the key, the dedupe behavior, and the named residual. +- `gotchas.md`: PRO-217's entry gains the service-RPC face and the canary's + removal guard. + +## Out of scope + +- Durable (cross-instance) idempotency storage — the handler's option via + `ctx.idempotencyKey`, never a framework requirement here. +- Middleware, metadata, streaming, rich errors — declined for this kind by + the scope statement. +- Adding the canary to the required-checks list (Will's manual step). +- The streams follow-ups (typed `streamDef`, audit debt items). diff --git a/docs/design/10-domains/connection-contracts.md b/docs/design/10-domains/connection-contracts.md index e1be19dc..2df3310f 100644 --- a/docs/design/10-domains/connection-contracts.md +++ b/docs/design/10-domains/connection-contracts.md @@ -184,6 +184,34 @@ generalizes the mechanism — the `serviceKey` connection param declares an opaq provisioning *need* that the deploy target resolves through its own registry, so core mints nothing and knows nothing about RPC. +### The binding retries safely, so a dropped request is not a lost call + +A network-bound provider may have scaled to zero and has to boot before it can +answer; a caller's first request can be dropped while that happens. The generated +client absorbs this: every call carries an `Idempotency-Key` header, and a call +whose request is dropped — a thrown network error, a `429`, or any `5xx` — is +retried with a bounded, jittered backoff. Any other `4xx` is not retried: a +malformed or unauthorized request stays wrong on a second attempt. + +The key is what makes retrying safe rather than a way to double-execute a write. +The client mints one key per *logical call* and sends the same key on every retry +of it, so the server can tell a retry from a new call: it runs one call per key, +lets a duplicate that arrives mid-flight wait for the first, and replays a +completed answer (a `2xx` or a `4xx` — never a `5xx`, which is the outcome a retry +exists to escape) to a later duplicate. The generated client always sends a key, +so every framework call is deduplicated; a request that arrives without one — a +hand-rolled or older caller — is served once, with no deduplication, rather than +rejected. + +This is a property of the binding, like the service key above: an in-memory or +mock binding has no network hop to drop a request, so it neither retries nor +deduplicates, and the contract says nothing about either. The retry is permanent +behavior of a network binding, not a workaround for one platform's cold starts; +[ADR-0037](../90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md) +decides it, including what its in-process deduplication does not cover — a retry +that reaches a different provider instance than the one that did the work — and why +that residue is left to a handler that needs to close it. + ## How compatibility is checked Enforcement happens in three places: @@ -196,9 +224,15 @@ Enforcement happens in three places: contract value), so a structurally-equivalent-but-distinct contract does not match. That is the RPC contract's current implementation, not a rule the framework or the Contract abstraction imposes. -3. **Run (per call) — validate input and output against the contract's schemas.** - Catches a provider that is typed-compatible but lies at runtime: a bug, drift, or - a legacy server wrapped in a Service that TypeScript never saw. +3. **Run (per call) — the server validates both directions against the contract's + schemas.** It validates the request against the method's input schema, rejecting a + malformed caller with `400` and a message describing what was wrong, and validates + its own handler's return against the output schema before responding — which + catches a provider that is typed-compatible but lies at runtime: a bug, drift, or + a legacy server wrapped in a Service that TypeScript never saw. The generated + client does not validate the response a second time; both ends of every connection + are framework-generated, so a second pass on every call would re-check what the + server already guaranteed. ### The compile-time check is plain TypeScript assignability diff --git a/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md b/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md new file mode 100644 index 00000000..91d0401b --- /dev/null +++ b/docs/design/90-decisions/ADR-0037-service-rpc-calls-carry-an-idempotency-key.md @@ -0,0 +1,137 @@ +# ADR-0037: Service RPC calls carry an idempotency key, so every call is safe to retry + +## Status + +Accepted + +## Decision + +Every call the generated service-RPC client makes carries an `Idempotency-Key` +header: one key per logical call, reused on every retry of that call. The +provider runs one call per key — a duplicate that arrives while the first is +still running waits for it, and a duplicate that arrives after an answer +replays that answer without running the handler again. Because duplicates are +absorbed by the protocol, **every method is retryable and no method declares +anything** — there is no per-method "is this safe to retry" flag. + +``` +POST /rpc/placeOrder POST /rpc/placeOrder +Idempotency-Key: 9f2c…e1 Idempotency-Key: 9f2c…e1 ← same key, a retry +Authorization: Bearer + provider: this key already ran → + ✗ connection dropped mid-boot replay its answer, don't + run the handler again +``` + +A handler that needs a stronger guarantee than the provider's own memory reads +the key from an optional third argument; handlers that don't are unchanged: + +```ts +serve(service, { + orders: { + placeOrder: async (input, deps, ctx) => { + // ctx.idempotencyKey lets a handler make its own work exactly-once. + await deps.db.insert({ ...input, requestKey: ctx.idempotencyKey }); + return { placed: true }; + }, + }, +}); +``` + +## Reasoning + +A service that has scaled to zero must boot before it can answer, and the +caller's first request can be dropped while it does — the connection is closed +during establishment, before any handler runs. The repair is for the client to +retry; the objection is that a retried `POST` might run a write twice. The key +is what turns the retry from dangerous into safe. + +The client generates one identifier per logical call — not per attempt — and +sends it on every attempt of that call. So a retry is always recognizable as +the same call, and two genuinely separate calls never collide. The provider +keys its work on it: a second request for a key still in flight waits for the +first instead of starting a second execution, and a second request after the +first has answered gets that same answer back. Duplicate suppression is a +mechanism the framework runs, so the method author is asked for nothing and +every method is retryable. + +What counts as "an answer" is the load-bearing distinction. A `2xx` and a `4xx` +are both conclusions — the call succeeded, or it was rejected — so replaying +either is correct. A `5xx` or a thrown error is not a conclusion; it is the +outcome a retry exists to escape, so it is never remembered and a retry +re-executes. The memory of answers exists only to absorb the retries of a call +still in flight, not to serve as a general result cache, so it holds a fixed +number of recent answers for about a minute. + +The key is what makes retrying safe, and it lives entirely in the generated +client, which pairs "mint a key" with "retry" inseparably and always sends the +key. A request that arrives *without* a key therefore never came from that +client — it is a hand-rolled or older caller — so it is run once, with no +deduplication, rather than rejected. This keeps the safe path automatic (the +generated client is always keyed) while letting an out-of-band caller, or a +`curl` during debugging, work transparently. The handler sees the key as +`undefined` for such a call and decides for itself whether that is acceptable. + +Retrying is permanent behavior of this kind, not a workaround for one platform. +Keyed retries are correct on any transport that can drop a request, so nothing +here is written to be removed later. + +### What this guarantees, and what it does not + +[`connection-contracts.md`](../10-domains/connection-contracts.md) sets the +scope this calibrates against: a connection is internal because both ends +belong to one application's topology, not because they share a network, and +robustness is justified per edge against the named failure modes of the targets +carrying it. + +The failure this guards is a request dropped while its target boots. That +happens before a handler runs, so nothing was applied and a retry cannot +duplicate work — the keys are not what make *that* case safe. What the keys add +is the narrower case where a handler did run and its answer was lost on the way +back: the retry finds the recorded answer instead of running the work twice. + +The provider's memory of answers lives in the serving process, so it cannot +cover a retry that reaches a *different* instance than the one that did the work +— after an instance dies mid-request, or on an edge whose retries route +elsewhere. That residue is deliberate: closing it would put a durable store +behind every provider, and no supported target's failure modes call for that. +An edge that does need it has the tool without the framework imposing it — the +handler receives the key and can record it inside its own transaction, making +its own work exactly-once by its own storage's guarantees. + +## Consequences + +- **Every method is retryable, and nothing declares it.** Retry behavior needs + no change to a contract, a method, or a handler. +- **The generated client always sends a key**, so every framework-to-framework + call is deduplicated. +- **A keyless request is served once, without deduplication.** A hand-rolled + request or a `curl` works transparently; a handler's `ctx.idempotencyKey` is + `undefined` for it. +- **Handlers may take a third argument** carrying the key. Existing + two-argument handlers are unaffected; the argument is optional. +- **A provider holds a bounded number of recent answers in memory**, sized to + absorb in-flight retries rather than to grow with traffic. +- **Cross-instance duplicate suppression is the handler's to add**, using the + key it is given, when its target's failure modes warrant it. + +## Alternatives considered + +- **A per-method `idempotent: true` flag, retrying only marked methods.** + Rejected: idempotence is a property of what a handler does to state, so + nothing at the RPC layer can verify the claim. The framework would retry on + an assurance it cannot check, and a method marked wrongly would duplicate + writes with nothing to catch it; the safe default ("not idempotent") also + guarantees under-marking, so the mechanism would protect almost nothing. The + key replaces a promise with a mechanism. +- **Retrying without keys.** Rejected: it turns a visible failure into a silent + duplicate write, on the assumption that every handler is repeat-safe — the + same unverifiable claim, made on the author's behalf. +- **Rejecting a keyless request outright.** Rejected: the only caller that can + omit a key is one outside the generated client, for which "no key, no + deduplication" is the honest and expected behavior; rejecting it breaks + `curl` and older clients to guard a case the safe path already covers. +- **Durable, cross-instance deduplication in the framework.** Rejected: it puts + a storage dependency behind every provider to close a window no supported + target's failure modes call for. A handler that needs it has the key to do it + itself. diff --git a/docs/design/90-decisions/README.md b/docs/design/90-decisions/README.md index 616f939a..1244a208 100644 --- a/docs/design/90-decisions/README.md +++ b/docs/design/90-decisions/README.md @@ -58,3 +58,4 @@ _Earlier drafts (ADR-0001, ADR-0002) were retired as the high-level design settl - [ADR-0034](ADR-0034-deploy-state-lives-in-the-stage-branch.md) — Deploy state lives in a framework-owned `prisma-composer-state` database in the stage's own Branch (production: the implicit default Branch). State has the environment's lifetime: platform-side Branch/Project deletion cleans it up with no framework involvement; the CLI deletes it last-among-members on destroy. Supersedes ADR-0009's workspace-level store. - [ADR-0035](ADR-0035-public-entrypoints-live-in-src-exports.md) — Public entrypoints live in `src/exports/` (one file per subpath; internals stay at the `src/` root); `@internal/tsdown-config` generates `package.json#exports` from object-named entries where safe, with two deliberate exceptions kept hand-maintained (the multi-pass `cron`/`storage`/`streams`, and the two published packages). Completes ADR-0028. - [ADR-0036](ADR-0036-the-rpc-kind-is-named-service-rpc.md) — The RPC kind is named **service RPC**: subpath `@prisma/composer/service-rpc`, unchanged call-site names (`rpc()`, `contract()`, `serve()`), kind brand stays `'rpc'`. Scope recorded in connection-contracts.md: edges internal to the application topology, agent-generatable by design — not an application API layer, not general distributed-systems infrastructure. +- [ADR-0037](ADR-0037-service-rpc-calls-carry-an-idempotency-key.md) — The generated service RPC client carries an `Idempotency-Key` on every call — one per logical call, reused across a bounded retry — and the provider deduplicates on it: one call per key, replaying completed 2xx/4xx answers (never 5xx) from a bounded in-process store. A keyless request (a hand-rolled or older caller) is served once without deduplication rather than rejected. Retrying is permanent protocol behavior, not a platform workaround, and there is no per-method opt-in — a flag would be an unverifiable claim. Handlers may read the key via an optional third argument for their own durable exactly-once. diff --git a/docs/guides/building-an-app.md b/docs/guides/building-an-app.md index a7197e73..082e30f0 100644 --- a/docs/guides/building-an-app.md +++ b/docs/guides/building-an-app.md @@ -47,8 +47,9 @@ export const authContract = contract({ On the producer, `serve()` turns the service's `expose` into a fetch handler. The handler map must cover every method — a missing or wrong-shaped handler -doesn't compile. Each handler receives the validated input (and the service's -own loaded deps as a second argument): +doesn't compile. Each handler receives the validated input, the service's own +loaded deps as a second argument, and an optional third argument carrying the +call's idempotency key (see below — most handlers ignore it): ```ts const handler = serve(service, { @@ -88,6 +89,25 @@ run it locally. service you run in a terminal, a fake, and `bootstrapService` all accept every call, and there's no key for you to supply. +### Calls retry safely for you + +You don't do anything for this either. A provider that has scaled to zero has +to boot before it answers, and a first call can be dropped mid-connection +while it does. The client absorbs that: every call carries an **idempotency +key**, a dropped call is retried with a backoff, and the provider runs one +call per key — a retry that arrives after the first already ran gets the first +answer back instead of running your handler twice. So `await auth.verify(...)` +just works across a cold start, and it works whether or not the call changes +state. You write nothing; the key is on the request and the deduplication is +in `serve()`. + +The deduplication rides on the key, so a request sent *without* one — a `curl`, +any hand-rolled request — simply isn't deduplicated: it runs once, which is what +you'd expect. The generated client always sends a key, so your service-to-service +calls always get it. If a handler needs a stronger guarantee than one instance's +memory — surviving a crash mid-call — its optional third argument carries the key +to write into its own transaction; most handlers never need it. + Two limits worth knowing: - **A key opens the whole service, not one method.** Any valid key reaches diff --git a/docs/guides/getting-started.md b/docs/guides/getting-started.md index 2405686d..ce83f1b4 100644 --- a/docs/guides/getting-started.md +++ b/docs/guides/getting-started.md @@ -329,6 +329,13 @@ a deploy creates them. [Building an app](building-an-app.md#calls-are-authenticated-for-you) has the details. +Two more things the generated client does for you, both invisible in your +code: every call carries an idempotency key and retries safely if the target +was still cold-starting, and the provider deduplicates on that key so a retry +never runs your handler twice. (A hand-rolled request without a key still +works — it just isn't deduplicated.) +[Building an app](building-an-app.md#calls-retry-safely-for-you) has these too. + Re-deploying is idempotent — it updates the same Project. For an isolated copy of the whole app (own services, own config), deploy a **stage**, and tear it down when you're done: diff --git a/gotchas.md b/gotchas.md index 70921c56..9a818502 100644 --- a/gotchas.md +++ b/gotchas.md @@ -396,6 +396,8 @@ The window is measurable, and much wider than first recorded. In `examples/strea **Removal guard.** The CI "Cold-start canary (PRO-217)" job (`scripts/cold-start-canary.ts`, in the E2E deploy workflow) touches freshly promoted instances each run, 60 s apart, and confirms from the deployment's own boot log that each touch was sent before the server's "listening" line — a touch that cannot be placed on one side of the boot counts for nothing rather than being guessed. It fails only when enough touches reached a genuine cold start **and** every one of them held; because this bug is intermittent, "every touch held" in a small run is the expected outcome of a run that is too small, so the job requires 14 confirmed cold-start holds before it will claim the bug is gone (at a conservative 20% close rate, 0.8^14 ≈ 4.4% — the chance of being fooled by luck). An inconclusive run passes with a warning annotation. That failure is the signal to remove the streams client's `IDEMPOTENT_BACKOFF` (the PRO-219 compensation) and the canary itself, the same contract as FT-5226's cold-connect canary. +**The service-RPC face, and why it has no canary of its own.** The same ingress closes a service-to-service RPC call's first connection while the target cold-starts. The generated `service-rpc` client absorbs it: every call carries an idempotency key and is retried with a bounded backoff, and the server dedupes on the key so a retry cannot double-execute (ADR-0037). Two things follow that differ from the streams face. First, this retry is **permanent protocol semantics, not a compensation** — it is correct on any transport that can drop a request, so nothing here is ever removed when PRO-217 is fixed; there is no workaround to time the removal of, and therefore no RPC-specific canary. Second, an RPC canary was built and dropped on the evidence anyway: an RPC service that restores no state (`examples/storefront-auth`'s `auth`) boots in under ~1.5 s — narrower than the 2 s clock-skew margin the coldness proof needs — so a probe cannot certify a cold start even though it reliably forces one (touch measured 376–1185 ms ahead of the listening line across 14 samples). PRO-217 stays watched by the streams canary above, which catches it because streams has the long boot window RPC lacks; when that canary reports the platform healed, the RPC keys and retry still stay, because they were never a workaround. + **Reproduction.** 1. Deploy two Compute services, A calling B over HTTP on each request to A. diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/client.test.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/client.test.ts index 5de4fe61..e28d7f09 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/client.test.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/client.test.ts @@ -8,15 +8,16 @@ const authContract = contract({ verify: rpc({ input: type({ token: 'string' }), output: type({ ok: 'boolean' }) }), }); +const okResponse = () => + new Response(JSON.stringify({ ok: true }), { headers: { 'content-type': 'application/json' } }); + describe('makeClient()', () => { - test('POSTs JSON to /rpc/ and returns the validated output', async () => { + test('POSTs JSON to /rpc/ and returns the parsed response', async () => { const requests: Request[] = []; const client = makeClient(authContract, 'http://auth.internal', { fetch: async (req) => { requests.push(req); - return new Response(JSON.stringify({ ok: true }), { - headers: { 'content-type': 'application/json' }, - }); + return okResponse(); }, }); @@ -34,9 +35,7 @@ describe('makeClient()', () => { const client = makeClient(authContract, 'http://auth.internal/api/v1', { fetch: async (req) => { requests.push(req); - return new Response(JSON.stringify({ ok: true }), { - headers: { 'content-type': 'application/json' }, - }); + return okResponse(); }, }); @@ -45,20 +44,10 @@ describe('makeClient()', () => { expect(requests[0]?.url).toBe('http://auth.internal/api/v1/rpc/verify'); }); - test('rejects a response that fails the output schema — a lying server is caught', async () => { + test('throws naming the method when the transport responds with a non-retryable non-OK status', async () => { + // 400 (not 500 — a 5xx would now retry, see the idempotency-key tests below). const client = makeClient(authContract, 'http://auth.internal', { - fetch: async () => - new Response(JSON.stringify({ ok: 'not-a-boolean' }), { - headers: { 'content-type': 'application/json' }, - }), - }); - - await expect(client.verify({ token: 't' })).rejects.toThrow(); - }); - - test('throws naming the method when the transport responds non-OK', async () => { - const client = makeClient(authContract, 'http://auth.internal', { - fetch: async () => new Response('nope', { status: 500 }), + fetch: async () => new Response('nope', { status: 400 }), }); await expect(client.verify({ token: 't' })).rejects.toThrow(/verify/); @@ -90,9 +79,7 @@ describe('makeClient()', () => { serviceKey: 'edge-key', fetch: async (req) => { requests.push(req); - return new Response(JSON.stringify({ ok: true }), { - headers: { 'content-type': 'application/json' }, - }); + return okResponse(); }, }); @@ -106,9 +93,7 @@ describe('makeClient()', () => { const client = makeClient(authContract, 'http://auth.internal', { fetch: async (req) => { requests.push(req); - return new Response(JSON.stringify({ ok: true }), { - headers: { 'content-type': 'application/json' }, - }); + return okResponse(); }, }); @@ -117,3 +102,98 @@ describe('makeClient()', () => { expect(requests[0]?.headers.has('authorization')).toBe(false); }); }); + +describe('makeClient() — idempotency key and retry', () => { + test('every request carries a non-empty Idempotency-Key header', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return okResponse(); + }, + }); + + await client.verify({ token: 't' }); + + expect(requests[0]?.headers.get('idempotency-key')).toBeTruthy(); + }); + + test('two separate logical calls mint two different keys', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return okResponse(); + }, + }); + + await client.verify({ token: 'a' }); + await client.verify({ token: 'b' }); + + expect(requests).toHaveLength(2); + expect(requests[0]?.headers.get('idempotency-key')).not.toBe( + requests[1]?.headers.get('idempotency-key'), + ); + }); + + test('a 503 then success resolves after exactly two requests, both carrying the same key', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return requests.length === 1 ? new Response('cold', { status: 503 }) : okResponse(); + }, + }); + + await expect(client.verify({ token: 't' })).resolves.toEqual({ ok: true }); + + expect(requests).toHaveLength(2); + const firstKey = requests[0]?.headers.get('idempotency-key'); + const secondKey = requests[1]?.headers.get('idempotency-key'); + expect(firstKey).toBeTruthy(); + expect(firstKey).toBe(secondKey); + }); + + test('a 429 is retried the same as a 5xx', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return requests.length === 1 ? new Response('slow down', { status: 429 }) : okResponse(); + }, + }); + + await expect(client.verify({ token: 't' })).resolves.toEqual({ ok: true }); + expect(requests).toHaveLength(2); + }); + + test('a thrown network error is retried and the call still resolves', async () => { + let calls = 0; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async () => { + calls += 1; + if (calls === 1) throw new Error('socket reset'); + return okResponse(); + }, + }); + + await expect(client.verify({ token: 't' })).resolves.toEqual({ ok: true }); + expect(calls).toBe(2); + }); + + test('a 404 rejects after exactly one request — no background retry follows', async () => { + const requests: Request[] = []; + const client = makeClient(authContract, 'http://auth.internal', { + fetch: async (req) => { + requests.push(req); + return new Response(JSON.stringify({ error: 'unknown method' }), { status: 404 }); + }, + }); + + await expect(client.verify({ token: 't' })).rejects.toThrow(); + expect(requests).toHaveLength(1); + + await new Promise((resolve) => setTimeout(resolve, 300)); // a retry would land here + expect(requests).toHaveLength(1); + }); +}); diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts index 52c6a359..0052702b 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve-handlers.test-d.ts @@ -60,6 +60,17 @@ test('the exhaustive, correctly-typed handler map is accepted', () => { }); }); +test('ctx.idempotencyKey is typed string | undefined on the three-argument handler form', () => { + serve(authService, { + rpc: { + verify: async ({ token }, { db }, ctx) => { + const key: string | undefined = ctx.idempotencyKey; + return { ok: token.length > 0 && db.validTokens.length >= 0 && key !== undefined }; + }, + }, + }); +}); + test('extra handler methods/ports beyond what is exposed are allowed (width)', () => { serve(authService, { rpc: { diff --git a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts index 3393a7a7..e7cef7a9 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/__tests__/serve.test.ts @@ -5,7 +5,12 @@ import { type } from 'arktype'; import { makeClient } from '../client.ts'; import { contract } from '../contract.ts'; import { rpc } from '../rpc.ts'; -import { RPC_ACCEPTED_KEYS_ENV, serve } from '../serve.ts'; +import { + MAX_BODY_BYTES, + REPLAY_CACHE_MAX_ENTRIES, + RPC_ACCEPTED_KEYS_ENV, + serve, +} from '../serve.ts'; const authContract = contract({ verify: rpc({ input: type({ token: 'string' }), output: type({ ok: 'boolean' }) }), @@ -52,6 +57,21 @@ function fakeAuthService(load: () => FakeDb) { >; } +/** A POST /rpc/verify request, defaulting to a valid idempotency key — every test below overrides only what it's testing. */ +function verifyRequest( + body: unknown, + opts?: { readonly idempotencyKey?: string | null; readonly headers?: Record }, +): Request { + const headers: Record = { 'content-type': 'application/json', ...opts?.headers }; + const key = opts?.idempotencyKey === undefined ? 'test-key' : opts.idempotencyKey; + if (key !== null) headers['idempotency-key'] = key; + return new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers, + body: JSON.stringify(body), + }); +} + describe('serve()', () => { test('round trip: a valid call reaches the handler and returns the typed result', async () => { const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); @@ -64,19 +84,16 @@ describe('serve()', () => { await expect(client.verify({ token: 'bad-token' })).resolves.toEqual({ ok: false }); }); - test('a bad input is rejected — the server-side arktype validation fires', async () => { + test('a bad input is rejected with 400, and the body carries the validator detail', async () => { const authService = fakeAuthService(() => ({ validTokens: [] })); const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 123 }), - }), - ); + const res = await handler(verifyRequest({ token: 123 })); expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error.length).toBeGreaterThan(0); + expect(body.error).toMatch(/token/i); // arktype's message names the bad field }); test('an unknown method 404s', async () => { @@ -94,25 +111,105 @@ describe('serve()', () => { expect(res.status).toBe(404); }); - test('a handler throw is a 500, not a crash', async () => { + test('a handler throw is a 500 that does not leak the exception message, and logs the real error', async () => { const authService = fakeAuthService(() => ({ validTokens: [] })); const handler = serve(authService, { rpc: { verify: async () => { - throw new Error('db unreachable'); + throw new Error('db credentials: hunter2'); }, }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 't' }), - }), - ); + const logged: unknown[][] = []; + const originalConsoleError = console.error; + console.error = (...args: unknown[]) => { + logged.push(args); + }; + try { + const res = await handler(verifyRequest({ token: 't' })); + + expect(res.status).toBe(500); + const bodyText = await res.text(); + expect(bodyText).not.toContain('hunter2'); + + const loggedRealError = logged.some((args) => + args.some((arg) => arg instanceof Error && arg.message.includes('hunter2')), + ); + expect(loggedRealError).toBe(true); + } finally { + console.error = originalConsoleError; + } + }); + + test('a handler returning schema-violating output is a masked 500, and logs it as a provider bug', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { + rpc: { + // `ok` must be a boolean; return a secret-bearing string instead. + // JSON.parse is typed `any`, so this defeats the output type at + // authoring without a cast — the runtime value is what serve()'s + // output validation must catch. + verify: async () => JSON.parse('{"ok":"leaked-secret-value"}'), + }, + }); + + const logged: unknown[][] = []; + const originalConsoleError = console.error; + console.error = (...args: unknown[]) => { + logged.push(args); + }; + try { + const res = await handler(verifyRequest({ token: 't' })); + + expect(res.status).toBe(500); + const bodyText = await res.text(); + expect(bodyText).not.toContain('leaked-secret-value'); + expect( + logged.some((args) => + args.some((a) => typeof a === 'string' && a.includes('provider bug')), + ), + ).toBe(true); + } finally { + console.error = originalConsoleError; + } + }); + + test('a request body that errors mid-read is a masked 500, not a rejected promise or a leaked error', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); - expect(res.status).toBe(500); + // A body stream that fails partway through with a secret-bearing error. + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('{"tok')); + controller.error(new Error('socket truncated: secret-xyz')); + }, + }); + const req = new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers: { 'content-type': 'application/json', 'idempotency-key': 'body-err-key' }, + body, + duplex: 'half', + }); + + const logged: unknown[][] = []; + const originalConsoleError = console.error; + console.error = (...args: unknown[]) => { + logged.push(args); + }; + try { + const res = await handler(req); + expect(res.status).toBe(500); + expect(await res.text()).not.toContain('secret-xyz'); + expect( + logged.some((args) => + args.some((a) => a instanceof Error && a.message.includes('secret-xyz')), + ), + ).toBe(true); + } finally { + console.error = originalConsoleError; + } }); test('the wrong HTTP verb on a known method is a 405', async () => { @@ -140,6 +237,317 @@ describe('serve()', () => { expect(loadCalls).toBe(1); }); + + test('a handler can read ctx.idempotencyKey — it matches the caller-supplied header', async () => { + let seenKey: string | undefined; + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }, ctx) => { + seenKey = ctx.idempotencyKey; + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + await handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: 'caller-key-42' })); + + expect(seenKey).toBe('caller-key-42'); + }); +}); + +describe('serve() — a keyless request opts out of deduplication', () => { + test('a request without the Idempotency-Key header succeeds, running the handler each time (no dedup)', async () => { + let handlerCalls = 0; + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }) => { + handlerCalls += 1; + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + const req = () => verifyRequest({ token: 'good-token' }, { idempotencyKey: null }); + const first = await handler(req()); + const second = await handler(req()); + + expect(first.status).toBe(200); + expect(await first.json()).toEqual({ ok: true }); + // Two keyless requests each ran the handler — no replay, because the caller + // sent no key to deduplicate on. + expect(handlerCalls).toBe(2); + expect(second.status).toBe(200); + }); + + test('an empty Idempotency-Key header is treated as no key (opts out, does not error)', async () => { + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const res = await handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: '' })); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + }); + + test('the handler sees ctx.idempotencyKey === undefined for a keyless call, and the key value for a keyed one', async () => { + const seen: Array = []; + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { + rpc: { + verify: async (_input, _deps, ctx) => { + seen.push(ctx.idempotencyKey); + return { ok: true }; + }, + }, + }); + + await handler(verifyRequest({ token: 't' }, { idempotencyKey: null })); + await handler(verifyRequest({ token: 't' }, { idempotencyKey: 'k-123' })); + + expect(seen).toEqual([undefined, 'k-123']); + }); +}); + +describe('serve() — idempotency dedupe', () => { + test('a repeated key after completion replays the same response byte-identically — the handler runs once', async () => { + let handlerCalls = 0; + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }) => { + handlerCalls += 1; + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + const req = () => verifyRequest({ token: 'good-token' }, { idempotencyKey: 'replay-key' }); + const first = await handler(req()); + const firstBody = await first.text(); + const second = await handler(req()); + const secondBody = await second.text(); + + expect(handlerCalls).toBe(1); + expect(second.status).toBe(first.status); + expect(secondBody).toBe(firstBody); + }); + + test('concurrent requests sharing a key single-flight onto one execution', async () => { + let handlerCalls = 0; + let releaseHandler: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseHandler = resolve; + }); + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }) => { + handlerCalls += 1; + await gate; // held open until the test releases it, below + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + const req = () => verifyRequest({ token: 'good-token' }, { idempotencyKey: 'concurrent-key' }); + const first = handler(req()); + const second = handler(req()); + releaseHandler(); + const [firstRes, secondRes] = await Promise.all([first, second]); + + expect(handlerCalls).toBe(1); + expect(await firstRes.text()).toBe(await secondRes.text()); + }); + + test('a 5xx response is not cached — a same-key retry re-executes the handler', async () => { + let handlerCalls = 0; + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { + rpc: { + verify: async () => { + handlerCalls += 1; + throw new Error('db unreachable'); + }, + }, + }); + + const req = () => verifyRequest({ token: 't' }, { idempotencyKey: 'failing-key' }); + const first = await handler(req()); + const second = await handler(req()); + + expect(first.status).toBe(500); + expect(second.status).toBe(500); + expect(handlerCalls).toBe(2); + }); + + test('the replay cache is LRU-bounded — the oldest entry is evicted once the bound is exceeded', async () => { + let handlerCalls = 0; + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { + verify: async ({ token }, { db }) => { + handlerCalls += 1; + return { ok: db.validTokens.includes(token) }; + }, + }, + }); + + const call = (key: string) => + handler(verifyRequest({ token: 'good-token' }, { idempotencyKey: key })); + + await call('key-0'); + for (let i = 1; i <= REPLAY_CACHE_MAX_ENTRIES; i++) { + await call(`key-${i}`); // pushes key-0 out once the bound is exceeded + } + expect(handlerCalls).toBe(REPLAY_CACHE_MAX_ENTRIES + 1); + + await call('key-0'); // evicted — re-executes instead of replaying + expect(handlerCalls).toBe(REPLAY_CACHE_MAX_ENTRIES + 2); + + await call(`key-${REPLAY_CACHE_MAX_ENTRIES}`); // still cached — replays + expect(handlerCalls).toBe(REPLAY_CACHE_MAX_ENTRIES + 2); + }, 20_000); + + test('a replayed answer cannot cross methods, even when the same idempotency key is reused', async () => { + const dualContract = contract({ + verify: rpc({ input: type({ token: 'string' }), output: type({ ok: 'boolean' }) }), + echo: rpc({ input: type({ note: 'string' }), output: type({ note: 'string' }) }), + }); + const db: DependencyEnd = dependency({ + name: 'db', + type: 'fake/db', + connection: { params: {}, hydrate: () => ({ validTokens: ['good-token'] }) }, + }); + const node = service({ + name: 'test-service', + extension: 'test/pack', + type: 'fake/rpc-test', + inputs: { db }, + params: {}, + build: { + extension: '@fake/adapter', + type: 'fake', + module: 'file:///test/service.ts', + entry: 'x', + }, + expose: { rpc: dualContract }, + }); + const dualService = { + ...node, + run: (_address: string, boot: () => Promise) => boot(), + load: () => ({ db: { validTokens: ['good-token'] } }), + } as unknown as RunnableServiceNode< + typeof node.inputs, + typeof node.params, + { rpc: typeof dualContract } + >; + + const handler = serve(dualService, { + rpc: { + verify: async ({ token }, { db }) => ({ ok: db.validTokens.includes(token) }), + echo: async ({ note }) => ({ note }), + }, + }); + + const sharedKey = 'shared-key'; + const verifyRes = await handler( + new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers: { 'content-type': 'application/json', 'idempotency-key': sharedKey }, + body: JSON.stringify({ token: 'good-token' }), + }), + ); + expect(await verifyRes.json()).toEqual({ ok: true }); + + const echoRes = await handler( + new Request('http://auth.internal/rpc/echo', { + method: 'POST', + headers: { 'content-type': 'application/json', 'idempotency-key': sharedKey }, + body: JSON.stringify({ note: 'hello' }), + }), + ); + // Must be echo's own answer, never verify's cached { ok: true }. + expect(await echoRes.json()).toEqual({ note: 'hello' }); + }); +}); + +describe('serve() — request body size cap', () => { + test('a body over the size limit is rejected with 413', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const oversized = 'x'.repeat(MAX_BODY_BYTES + 1); + const res = await handler(verifyRequest({ token: oversized })); + + expect(res.status).toBe(413); + }); + + test('the cap is enforced against bytes actually read, not content-length — a stream body with no content-length header is still capped', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const oversizedJson = JSON.stringify({ token: 'x'.repeat(MAX_BODY_BYTES + 1) }); + const bytes = new TextEncoder().encode(oversizedJson); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(bytes); + controller.close(); + }, + }); + + const req = new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers: { 'content-type': 'application/json', 'idempotency-key': 'k' }, + body, + duplex: 'half', + } as RequestInit); + + expect(req.headers.has('content-length')).toBe(false); + + const res = await handler(req); + expect(res.status).toBe(413); + }); + + test('the cap is enforced even when content-length lies about being small', async () => { + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const oversizedJson = JSON.stringify({ token: 'x'.repeat(MAX_BODY_BYTES + 1) }); + const bytes = new TextEncoder().encode(oversizedJson); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(bytes); + controller.close(); + }, + }); + + const req = new Request('http://auth.internal/rpc/verify', { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'idempotency-key': 'k', + 'content-length': '10', // a lie — the real body is far larger + }, + body, + duplex: 'half', + } as RequestInit); + + const res = await handler(req); + expect(res.status).toBe(413); + }); + + test('a body at or under the limit is accepted', async () => { + const authService = fakeAuthService(() => ({ validTokens: ['good-token'] })); + const handler = serve(authService, { + rpc: { verify: async ({ token }, { db }) => ({ ok: db.validTokens.includes(token) }) }, + }); + + const res = await handler(verifyRequest({ token: 'good-token' })); + + expect(res.status).toBe(200); + }); }); describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () => { @@ -175,11 +583,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () }); const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json', authorization: 'Bearer wrong-key' }, - body: JSON.stringify({ token: 't' }), - }), + verifyRequest({ token: 't' }, { headers: { authorization: 'Bearer wrong-key' } }), ); expect(res.status).toBe(401); @@ -199,13 +603,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 't' }), - }), - ); + const res = await handler(verifyRequest({ token: 't' })); expect(res.status).toBe(401); expect(handlerCalled).toBe(false); @@ -216,13 +614,17 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () const authService = fakeAuthService(() => ({ validTokens: [] })); const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 123 }), - }), - ); + const res = await handler(verifyRequest({ token: 123 })); + + expect(res.status).toBe(401); + }); + + test('401 fires before the idempotency-key requirement — a keyless request with no service key is 401, not 400', async () => { + process.env[RPC_ACCEPTED_KEYS_ENV] = JSON.stringify(['good-key']); + const authService = fakeAuthService(() => ({ validTokens: [] })); + const handler = serve(authService, { rpc: { verify: async () => ({ ok: true }) } }); + + const res = await handler(verifyRequest({ token: 't' }, { idempotencyKey: null })); expect(res.status).toBe(401); }); @@ -240,13 +642,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 'good-token' }), - }), - ); + const res = await handler(verifyRequest({ token: 'good-token' })); expect(res.status).toBe(401); expect(handlerCalled).toBe(false); @@ -258,13 +654,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () rpc: { verify: async ({ token }, { db }) => ({ ok: db.validTokens.includes(token) }) }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 'good-token' }), - }), - ); + const res = await handler(verifyRequest({ token: 'good-token' })); expect(res.status).toBe(200); }); @@ -282,13 +672,7 @@ describe('serve() — service-key enforcement (COMPOSER_RPC_ACCEPTED_KEYS)', () }, }); - const res = await handler( - new Request('http://auth.internal/rpc/verify', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ token: 'good-token' }), - }), - ); + const res = await handler(verifyRequest({ token: 'good-token' })); expect(res.status).toBe(401); expect(handlerCalled).toBe(false); diff --git a/packages/0-framework/2-authoring/service-rpc/src/client.ts b/packages/0-framework/2-authoring/service-rpc/src/client.ts index 10a2e044..031ea689 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/client.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/client.ts @@ -1,25 +1,13 @@ /** - * The RPC kind's network adapter — the client `rpc(contract)` hydrates to. - * Reads each method's Standard Schema pair off the contract's `__cmp[method]` - * runtime value (rpc()'s `{ input, output }`), POSTs JSON to - * `/rpc/`, and validates the response against the output schema - * before returning it (a provider can be typed-compatible and still lie at - * runtime — this is the per-call layer that catches that). When a - * `serviceKey` is supplied (ADR-0030), every request also carries - * `Authorization: Bearer `. + * The typed client `rpc(contract)` hydrates to: POSTs JSON to + * `/rpc/` per method, carrying an idempotency key (and the + * service key when given) and retrying per `callWithRetry`. It does not + * re-validate the response — `serve()` already did. */ import type { Contract } from '@internal/core'; import { blindCast } from '@internal/foundation/casts'; -import type { StandardSchemaV1 } from '@standard-schema/spec'; import type { Client, RpcFns } from './rpc.ts'; -import { standardValidate } from './standard-schema.ts'; - -/** rpc()'s runtime shape for one method: `{ input, output }` wearing a function's type. */ -interface MethodSchemas { - readonly input: StandardSchemaV1; - readonly output: StandardSchemaV1; -} /** * A fetch-shaped transport. Defaults to the real `fetch`; a served handler @@ -28,10 +16,23 @@ interface MethodSchemas { */ export type Transport = (req: Request) => Promise; -/** `/rpc/`, preserving a base URL's own path (e.g. a mount point). */ -function methodUrl(base: string, method: string): string { - const normalizedBase = base.endsWith('/') ? base : `${base}/`; - return new URL(`rpc/${method}`, normalizedBase).toString(); +/** Bounded jittered backoff for retrying a dropped call. `maxRetries` is retries after the first attempt. */ +const RETRY = { + initialDelayMs: 250, + multiplier: 2, + maxDelayMs: 5_000, + maxRetries: 5, +}; + +const IDEMPOTENCY_KEY_HEADER = 'Idempotency-Key'; + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** Whether a non-OK response is safe to retry: 429 or any 5xx, never another 4xx. */ +function isRetryableStatus(status: number): boolean { + return status === 429 || status >= 500; } /** The server's `{ error }` body, if the response has one — undefined otherwise. */ @@ -46,40 +47,79 @@ async function errorDetail(res: Response): Promise { } } +/** `/rpc/`, preserving a base URL's own path (e.g. a mount point). */ +function methodUrl(base: string, method: string): string { + const normalizedBase = base.endsWith('/') ? base : `${base}/`; + return new URL(`rpc/${method}`, normalizedBase).toString(); +} + +/** + * Sends one call over `send`, retrying a thrown error, 429, or 5xx with + * full-jitter backoff. `buildRequest` runs per attempt but carries the same + * idempotency key each time — only the transport call repeats, not the key. + */ +async function callWithRetry( + send: Transport, + buildRequest: () => Request, + method: string, +): Promise { + let delay = RETRY.initialDelayMs; + let retries = 0; + + for (;;) { + let res: Response; + try { + res = await send(buildRequest()); + } catch (err) { + if (retries >= RETRY.maxRetries) throw err; + retries += 1; + await sleep(Math.random() * delay); + delay = Math.min(delay * RETRY.multiplier, RETRY.maxDelayMs); + continue; + } + + if (res.ok) return res.json(); + + if (!isRetryableStatus(res.status) || retries >= RETRY.maxRetries) { + const detail = await errorDetail(res); + throw new Error( + `RPC call "${method}" failed: ${res.status} ${res.statusText}` + + (detail !== undefined ? ` — ${detail}` : ''), + ); + } + + retries += 1; + await sleep(Math.random() * delay); + delay = Math.min(delay * RETRY.multiplier, RETRY.maxDelayMs); + } +} + export function makeClient>( contract: C, url: string, opts?: { fetch?: Transport; serviceKey?: string }, ): Client { const send = opts?.fetch ?? fetch; - const headers: Record = { 'content-type': 'application/json' }; + const baseHeaders: Record = { 'content-type': 'application/json' }; if (opts?.serviceKey !== undefined) { - headers['Authorization'] = `Bearer ${opts.serviceKey}`; + baseHeaders['Authorization'] = `Bearer ${opts.serviceKey}`; } const client: Record Promise> = {}; - for (const [method, schemas] of Object.entries( - blindCast< - Record, - 'rpc() stores each method input/output Standard Schemas on the function value; RpcFns types only the call signature' - >(contract.__cmp), - )) { + for (const method of Object.keys(contract.__cmp)) { client[method] = async (input: unknown) => { - const res = await send( - new Request(methodUrl(url, method), { - method: 'POST', - headers, - body: JSON.stringify(input), - }), + const idempotencyKey = crypto.randomUUID(); + const body = JSON.stringify(input); + return callWithRetry( + send, + () => + new Request(methodUrl(url, method), { + method: 'POST', + headers: { ...baseHeaders, [IDEMPOTENCY_KEY_HEADER]: idempotencyKey }, + body, + }), + method, ); - if (!res.ok) { - const detail = await errorDetail(res); - throw new Error( - `RPC call "${method}" failed: ${res.status} ${res.statusText}` + - (detail !== undefined ? ` — ${detail}` : ''), - ); - } - return standardValidate(schemas.output, await res.json()); }; } diff --git a/packages/0-framework/2-authoring/service-rpc/src/exports/index.ts b/packages/0-framework/2-authoring/service-rpc/src/exports/index.ts index 307117af..b8c10ed0 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/exports/index.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/exports/index.ts @@ -5,6 +5,11 @@ * client.ts; `serve()` generates a provider's fetch handler straight off a * service's `expose`. All web-standard (fetch/Request/Response) — runs * anywhere those exist, no node/bun coupling. + * + * Every call carries an idempotency key and retries safely: the client + * mints one per logical call and bounded-retries with it; the server + * requires it and dedupes on it. A handler's optional third argument, + * typed `RpcHandlerContext`, carries that key as `ctx.idempotencyKey`. */ export type { Transport } from '../client.ts'; @@ -12,5 +17,5 @@ export { makeClient } from '../client.ts'; export { contract } from '../contract.ts'; export type { Client } from '../rpc.ts'; export { perBindingToken, RPC_PEER_KEY, rpc } from '../rpc.ts'; -export type { Handlers } from '../serve.ts'; +export type { Handlers, RpcHandlerContext } from '../serve.ts'; export { RPC_ACCEPTED_KEYS_ENV, serve } from '../serve.ts'; diff --git a/packages/0-framework/2-authoring/service-rpc/src/serve.ts b/packages/0-framework/2-authoring/service-rpc/src/serve.ts index cbc1c739..d1deab4f 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/serve.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/serve.ts @@ -32,8 +32,14 @@ type AnyRunnable = RunnableServiceNode; type CmpOf = C extends Contract ? Cmp : never; +/** A handler's optional third argument. Handlers may ignore it. */ +export interface RpcHandlerContext { + /** The call's idempotency key (same across its retries), or `undefined` for a keyless caller that opted out of dedup. */ + readonly idempotencyKey: string | undefined; +} + type HandlerFor = Fn extends (input: infer I) => Promise - ? (input: I, deps: LoadedDeps) => Promise + ? (input: I, deps: LoadedDeps, ctx: RpcHandlerContext) => Promise : never; /** Every exposed port's methods, turned into a handler map typed off S's own `expose` and `load()`. */ @@ -51,15 +57,142 @@ interface MethodSchemas { readonly output: StandardSchemaV1; } -type RpcHandler = (input: unknown, deps: unknown) => Promise; +type RpcHandler = (input: unknown, deps: unknown, ctx: RpcHandlerContext) => Promise; + +/** A response, reduced to what the replay cache needs to reproduce it byte-identically. */ +interface Outcome { + readonly status: number; + readonly bodyText: string; +} + +function outcome(body: unknown, status = 200): Outcome { + return { status, bodyText: JSON.stringify(body) }; +} -function jsonResponse(body: unknown, status = 200): Response { - return new Response(JSON.stringify(body), { - status, +function toResponse(o: Outcome): Response { + return new Response(o.bodyText, { + status: o.status, headers: { 'content-type': 'application/json' }, }); } +/** The generic message every caller-facing 500 carries — the real error goes to `console.error` instead. */ +const INTERNAL_ERROR_MESSAGE = 'Internal server error'; + +/** Request body cap. Internal RPC payloads are small records, not uploads; 1 MiB bounds a slow request's memory. */ +export const MAX_BODY_BYTES = 1_048_576; + +class RequestBodyTooLargeError extends Error {} + +/** Reads the body as text, aborting past `maxBytes` of bytes actually read — `content-length` is caller-supplied, so untrusted. */ +async function readBoundedBody(req: Request, maxBytes: number): Promise { + const reader = req.body?.getReader(); + if (reader === undefined) return ''; + + const decoder = new TextDecoder(); + let text = ''; + let total = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + total += value.byteLength; + if (total > maxBytes) { + await reader.cancel(); + throw new RequestBodyTooLargeError(); + } + text += decoder.decode(value, { stream: true }); + } + text += decoder.decode(); + return text; +} + +/** Completed answers held for replay before LRU eviction. Fixed, so the per-provider cache is bounded memory, not traffic-scaled. */ +export const REPLAY_CACHE_MAX_ENTRIES = 1000; + +/** How long a completed 2xx/4xx answer stays replayable for a repeated key. */ +const REPLAY_TTL_MS = 60_000; + +type CacheEntry = + | { readonly kind: 'pending'; readonly promise: Promise } + | { readonly kind: 'completed'; readonly outcome: Outcome; readonly completedAt: number }; + +/** + * Per-method, per-key deduplication. A duplicate arriving mid-execution + * single-flights onto the same promise; a completed 2xx/4xx replays for + * REPLAY_TTL_MS; a 5xx is never kept, since that is what a retry re-executes. + * Keyed by method first, so a replay can never answer a different method. + */ +class IdempotencyStore { + private readonly byMethod = new Map>(); + // LRU order over completed entries; Map insertion order is the list, so re-inserting moves a key to the end. + private readonly lruOrder = new Map(); + + async dispatch(method: string, key: string, run: () => Promise): Promise { + const bucket = this.bucketFor(method); + const existing = bucket.get(key); + + if (existing?.kind === 'pending') { + return existing.promise; + } + if (existing?.kind === 'completed') { + if (Date.now() - existing.completedAt < REPLAY_TTL_MS) { + this.touch(method, key); + return existing.outcome; + } + bucket.delete(key); + this.lruOrder.delete(this.lruKey(method, key)); + } + + const promise = run(); + bucket.set(key, { kind: 'pending', promise }); + + let result: Outcome; + try { + result = await promise; + } catch (err) { + bucket.delete(key); + throw err; + } + + if (result.status >= 500) { + bucket.delete(key); // retryable outcome — a retry must re-execute + } else { + bucket.set(key, { kind: 'completed', outcome: result, completedAt: Date.now() }); + this.touch(method, key); + } + return result; + } + + private bucketFor(method: string): Map { + let bucket = this.byMethod.get(method); + if (bucket === undefined) { + bucket = new Map(); + this.byMethod.set(method, bucket); + } + return bucket; + } + + private lruKey(method: string, key: string): string { + return `${method}\0${key}`; + } + + /** Marks (method, key) most-recently-used, evicting the oldest completed entry once over the bound. */ + private touch(method: string, key: string): void { + const lruKey = this.lruKey(method, key); + this.lruOrder.delete(lruKey); + this.lruOrder.set(lruKey, { method, key }); + + if (this.lruOrder.size > REPLAY_CACHE_MAX_ENTRIES) { + const oldestKey = this.lruOrder.keys().next().value; + const oldest = oldestKey === undefined ? undefined : this.lruOrder.get(oldestKey); + if (oldestKey !== undefined && oldest !== undefined) { + this.lruOrder.delete(oldestKey); + this.byMethod.get(oldest.method)?.delete(oldest.key); + } + } + } +} + /** The provisioned accepted key set, or undefined when the deploy never provisioned one (local/test — enforcement off). */ function acceptedKeys(): readonly string[] | undefined { const raw = process.env[RPC_ACCEPTED_KEYS_ENV]; @@ -108,6 +241,8 @@ function bearerToken(req: Request): string { return header?.startsWith(BEARER_PREFIX) ? header.slice(BEARER_PREFIX.length) : ''; } +const IDEMPOTENCY_KEY_HEADER = 'Idempotency-Key'; + /** * Flattens every exposed port's methods into one method → {schemas, handler} * table. RPC dispatch is flat (`/rpc/`), so a method name exposed by @@ -144,11 +279,14 @@ function methodTable( } /** - * Routes `POST /rpc/`: parses JSON, validates input, calls the - * handler with `service.load()`'s deps, validates the output, and responds - * JSON. An unknown method or invalid input is a 4xx; a handler (or output - * validation) failure is a 5xx — either way the process does not crash. - * `load()` is called exactly once, here, before the handler ever runs. + * Routes `POST /rpc/`: checks the service key, requires an + * Idempotency-Key, single-flights/replays through `IdempotencyStore`, and — + * per call — parses JSON within the body cap, validates input, calls the + * handler with `service.load()`'s deps plus `{ idempotencyKey }`, validates + * the output, and responds JSON. A handler or output-validation failure + * masks its message behind a generic 500 and logs the real error; an + * unknown method or invalid input is a 4xx. `load()` is called exactly + * once, here, before the handler ever runs. */ export function serve>( service: S, @@ -162,46 +300,85 @@ export function serve>( >(handlers), ); const deps = service.load(); + const idempotency = new IdempotencyStore(); return async (req: Request): Promise => { const accepted = acceptedKeys(); if (accepted !== undefined && !isAcceptedKey(bearerToken(req), accepted)) { - return jsonResponse({ error: 'Unauthorized: missing or invalid service key' }, 401); + return toResponse(outcome({ error: 'Unauthorized: missing or invalid service key' }, 401)); } const { pathname } = new URL(req.url); const methodName = /^\/rpc\/([^/]+)$/.exec(pathname)?.[1]; if (methodName === undefined) { - return jsonResponse({ error: `Not found: ${pathname}` }, 404); + return toResponse(outcome({ error: `Not found: ${pathname}` }, 404)); } const method = table.get(methodName); if (method === undefined) { - return jsonResponse({ error: `Unknown RPC method "${methodName}"` }, 404); + return toResponse(outcome({ error: `Unknown RPC method "${methodName}"` }, 404)); } if (req.method !== 'POST') { - return jsonResponse({ error: `Method "${methodName}" requires POST` }, 405); + return toResponse(outcome({ error: `Method "${methodName}" requires POST` }, 405)); } - let body: unknown; - try { - body = await req.json(); - } catch { - return jsonResponse({ error: 'Request body must be JSON' }, 400); - } + // An empty header is the same as no header: the caller sent no key. + const idempotencyKey = req.headers.get(IDEMPOTENCY_KEY_HEADER.toLowerCase()) || undefined; + const ctx: RpcHandlerContext = { idempotencyKey }; - let input: unknown; - try { - input = await standardValidate(method.input, body); - } catch (err) { - return jsonResponse({ error: err instanceof Error ? err.message : String(err) }, 400); - } + const run = async (): Promise => { + let bodyText: string; + try { + bodyText = await readBoundedBody(req, MAX_BODY_BYTES); + } catch (err) { + if (err instanceof RequestBodyTooLargeError) { + return outcome({ error: `Request body exceeds the ${MAX_BODY_BYTES}-byte limit` }, 413); + } + // A body-stream I/O error: mask and log like any other internal + // failure — letting it reject would break serve()'s Response contract. + console.error(`serve(): reading the request body for "${methodName}" failed:`, err); + return outcome({ error: INTERNAL_ERROR_MESSAGE }, 500); + } - try { - const result = await method.handler(input, deps); - return jsonResponse(await standardValidate(method.output, result)); - } catch (err) { - return jsonResponse({ error: err instanceof Error ? err.message : String(err) }, 500); - } + let body: unknown; + try { + body = JSON.parse(bodyText); + } catch { + return outcome({ error: 'Request body must be JSON' }, 400); + } + + let input: unknown; + try { + input = await standardValidate(method.input, body); + } catch (err) { + return outcome({ error: err instanceof Error ? err.message : String(err) }, 400); + } + + try { + const result = await method.handler(input, deps, ctx); + let output: unknown; + try { + output = await standardValidate(method.output, result); + } catch (err) { + console.error( + `serve(): handler for "${methodName}" returned output that failed schema validation — this is a provider bug:`, + err, + ); + return outcome({ error: INTERNAL_ERROR_MESSAGE }, 500); + } + return outcome(output); + } catch (err) { + console.error(`serve(): handler for "${methodName}" threw:`, err); + return outcome({ error: INTERNAL_ERROR_MESSAGE }, 500); + } + }; + + // Keyed calls dedupe/replay through the store; a keyless one opts out (the + // generated client always sends a key, so this is a hand-rolled/legacy caller). + const result = + idempotencyKey === undefined + ? await run() + : await idempotency.dispatch(methodName, idempotencyKey, run); + return toResponse(result); }; } diff --git a/packages/0-framework/2-authoring/service-rpc/src/standard-schema.ts b/packages/0-framework/2-authoring/service-rpc/src/standard-schema.ts index 0c34791e..1d203fc7 100644 --- a/packages/0-framework/2-authoring/service-rpc/src/standard-schema.ts +++ b/packages/0-framework/2-authoring/service-rpc/src/standard-schema.ts @@ -1,8 +1,9 @@ /** * Runs a Standard Schema validator over an unknown value, returning the * parsed output — or throwing with the validator's own issues on failure. - * Shared by the RPC client (validating a response) and serve() (validating a - * request and a handler's return). + * Used by serve() to validate a request's input and its handler's return; + * the client does not re-validate the response, since serve() already + * guaranteed it against the same schema. */ import type { StandardSchemaV1 } from '@standard-schema/spec'; diff --git a/packages/1-prisma-cloud/2-shared-modules/cron/src/__tests__/serve-schedule.test.ts b/packages/1-prisma-cloud/2-shared-modules/cron/src/__tests__/serve-schedule.test.ts index 403b35f7..39e97d68 100644 --- a/packages/1-prisma-cloud/2-shared-modules/cron/src/__tests__/serve-schedule.test.ts +++ b/packages/1-prisma-cloud/2-shared-modules/cron/src/__tests__/serve-schedule.test.ts @@ -52,6 +52,9 @@ const schedule = defineSchedule({ tick: '2s', mrr: '5s' }); function triggerRequest(jobId: string): Request { return new Request('http://cron.internal/rpc/trigger', { method: 'POST', + // No idempotency key: a hand-built request without one is served normally + // (it has opted out of dedup). Production calls go through makeClient, + // which mints a key and gets dedup; this exercises the keyless path. headers: { 'content-type': 'application/json' }, body: JSON.stringify({ jobId }), }); diff --git a/skills/prisma-composer/SKILL.md b/skills/prisma-composer/SKILL.md index 1f641823..ff5ec88e 100644 --- a/skills/prisma-composer/SKILL.md +++ b/skills/prisma-composer/SKILL.md @@ -157,6 +157,19 @@ deployed `/rpc/` to check it works** — an unwired caller always gets `401`, which looks like a broken deploy and isn't. Debug through a consumer, or locally. +**Calls carry an idempotency key and retry safely for you.** Every call the +generated client makes carries an `Idempotency-Key`; a call dropped while the +target cold-starts is retried with a backoff, and `serve()` runs one call per +key — a retry that arrives after the first completed replays that answer +instead of re-running the handler. So every method is safely retryable and no +contract declares anything about it (do not add an "is this idempotent" flag — +the framework does not have one). Two consequences for you: a handler may take +an **optional third argument** `(input, deps, ctx)` and read `ctx.idempotencyKey` +(`string | undefined` — it's absent for a keyless caller) if it needs exactly-once +beyond one instance's memory (most don't); and a request without the header is +served once without deduplication rather than rejected, so a hand-rolled probe +works but gets no retry safety. + | | | | --- | --- | | Locally / in tests | nothing is provisioned, so `serve()` passes every call through — never supply a key in `inputs` |