diff --git a/CHANGELOG.md b/CHANGELOG.md index 198ac711..5d7d5f15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,37 @@ npm release are grouped under the in-development version that introduced them. ### Added +- **`QUERY` is a first-class method — a read that carries a request body.** + ([draft-ietf-httpbis-safe-method-w-body](https://datatracker.ietf.org/doc/draft-ietf-httpbis-safe-method-w-body/)) + `method: 'QUERY'` already sent its body and already cached correctly under + `cache: { methods: 'QUERY' }`, because the cache key folds the body in. What it did **not** have + was the engine's agreement that it is a read: "safe method" was spelled `=== 'GET' || === 'HEAD'` + inline, so everything else was a write by default. + + One `isSafeMethod` predicate now answers that question in the two places that ask it — RFC 9110 + §9.2.1's safe set (`GET`, `HEAD`, `OPTIONS`, `TRACE`) plus `QUERY`: + + - **No `Idempotency-Key` on a safe method.** A stitch with `idempotency` configured no longer + stamps a dedupe token on a `QUERY` — there is no side effect to collapse, and the header would + have varied the cache key on every send. `OPTIONS`/`TRACE` stop being stamped too; they were + only ever getting a key because they were not `GET` or `HEAD`. + - **The construction nudge follows.** Declaring `idempotency` on a `QUERY` now logs the same + "the key is sent on writes only" hint a `GET` gets, so the drop is never silent. Silenced by + `idempotency.warn = false` as before. + + **A 301/302 no longer downgrades a `QUERY` to a bodyless `GET`** (default `fetch` transport). + That downgrade is a historical exception granted to `POST`, and the draft rules it out by name + for `QUERY`; applying it dropped the body, which silently turned a filtered read into an + unfiltered one. `303` still redirects to a `GET` — for a `QUERY` that is what it means. `POST`, + `PUT`, `PATCH` and `DELETE` redirect exactly as before. + + **Three method tests, not one.** `encodeRequestBody` still drops a body on `GET`/`HEAD` **only** + and is deliberately not routed through the safety predicate: that branch enforces a transport + constraint (`fetch` throws on a `GET` with a body), and widening it to "safe" would have deleted + the payload of every `QUERY` — the one thing that already worked. The cacheable-method default + is likewise untouched at `['GET','HEAD']`; `QUERY` is now documented as a valid `cache.methods` + entry, opt-in like a GraphQL `POST`. + - **`throttle.concurrency` goes fleet-wide too, by lease.** ([ADR 0025](docs/adr/0025-fleet-wide-concurrency-by-lease.md)) [ADR 0024](docs/adr/0024-the-fleet-wide-pacing-cell.md) made the rate budget fleet-wide and left the concurrency cap per-process, so `concurrency: 10` across eight workers was really a fleet cap diff --git a/README.md b/README.md index 59702cf4..84d710d9 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@

- Zero runtime dependencies · ~24 kB min+gzip — a typical import { stitch } tree-shakes to ~21 kB, and with no transitive tree there is nothing else to install or audit. The size is an enforced budget in CI, not an aspiration. + Zero runtime dependencies · ~24 kB min+gzip — a typical import { stitch } tree-shakes to ~22 kB, and with no transitive tree there is nothing else to install or audit. The size is an enforced budget in CI, not an aspiration.

@@ -164,7 +164,7 @@ No server, no codegen, no config files, no implicit inheritance — **only expli - **Pluggable state store** — throttle counters and sessions behind a 3-method store; swap in Redis/Postgres to go distributed. - **Zero-infra observability** — tracing is **off by default**; opt in per stitch or via `STITCH_TRACE_*` env vars. No collector, no dashboard. - **Four front doors, one definition** — in-process function, CLI (`stitch run`), HTTP (`stitch serve`), and MCP (`stitch mcp`). -- **Zero runtime dependencies** — `"dependencies": {}`, built on global `fetch`, tree-shakeable; **~24 kB min+gzip** for the whole entry, **~21 kB** for a typical `import { stitch }`. +- **Zero runtime dependencies** — `"dependencies": {}`, built on global `fetch`, tree-shakeable; **~24 kB min+gzip** for the whole entry, **~22 kB** for a typical `import { stitch }`. ## Install diff --git a/apps/docs/app/(home)/components/metrics.tsx b/apps/docs/app/(home)/components/metrics.tsx index 88db573c..b01ea559 100644 --- a/apps/docs/app/(home)/components/metrics.tsx +++ b/apps/docs/app/(home)/components/metrics.tsx @@ -10,7 +10,7 @@ const metrics = [ body: 'The whole stitchapi entry, tree-shaken — and it is an enforced budget in CI, not an aspiration.', }, { - value: '~21 kB', + value: '~22 kB', unit: 'import { stitch }', body: 'Pay only for what you import: every surface beyond http lives behind its own subpath, so the core trims down.', }, diff --git a/apps/docs/app/(home)/playground/playground-completions.generated.ts b/apps/docs/app/(home)/playground/playground-completions.generated.ts index 7f092f6f..aba57cc7 100644 --- a/apps/docs/app/(home)/playground/playground-completions.generated.ts +++ b/apps/docs/app/(home)/playground/playground-completions.generated.ts @@ -20,8 +20,8 @@ export const PLAYGROUND_COMPLETIONS: Record = { { label: "method", type: "property", - detail: "string", - info: "HTTP method; defaults to `GET`.", + detail: "KnownMethod | (string & {})", + info: "HTTP method; defaults to `GET`. Any verb the transport accepts, including **`QUERY`** — the safe, idempotent, cacheable method that carries a **request body** (`draft-ietf-httpbis-safe-method-w-body`), for a read whose filter is too large or too structured for a URL. The engine classifies `QUERY` as a read: no `Idempotency-Key` is stamped on it, and a 301/302 re-sends it as a `QUERY` rather than downgrading it to a bodyless `GET`. It keeps its body (unlike `GET`/`HEAD`, where the transport forbids one), and it is a valid CacheOptions.methods entry — opt in, since caching a body-carrying request is never a default.", }, { label: "wire", diff --git a/apps/docs/content/docs/concepts/principles.mdx b/apps/docs/content/docs/concepts/principles.mdx index ec2ff9ef..5f36fc9f 100644 --- a/apps/docs/content/docs/concepts/principles.mdx +++ b/apps/docs/content/docs/concepts/principles.mdx @@ -107,7 +107,7 @@ package practices with your bundle. Concretely, the whole `stitch` entry is **~24 kB minified + gzipped** (≈61 kB raw), and because every surface beyond `http` is a separate subpath -import, a typical `import { stitch }` tree-shakes to **~21 kB**. With zero +import, a typical `import { stitch }` tree-shakes to **~22 kB**. With zero runtime dependencies, that figure is the entire cost — not the tip of a transitive tree. diff --git a/apps/docs/content/docs/getting-started/installation.mdx b/apps/docs/content/docs/getting-started/installation.mdx index 04ed85aa..3937b7eb 100644 --- a/apps/docs/content/docs/getting-started/installation.mdx +++ b/apps/docs/content/docs/getting-started/installation.mdx @@ -7,7 +7,7 @@ Install the package, import `stitch`, and turn your first endpoint into a typed, callable function. `stitchapi` has zero dependencies and runs anywhere `fetch` does — Node, the browser, and edge runtimes. The whole entry is **~24 kB minified + gzipped** — and with no dependencies, there is no transitive tree -behind it (a typical `import { stitch }` tree-shakes to ~21 kB). +behind it (a typical `import { stitch }` tree-shakes to ~22 kB). **Validators are bring-your-own.** Because `stitchapi` ships with zero diff --git a/apps/docs/content/docs/guides/authoring/stitch.mdx b/apps/docs/content/docs/guides/authoring/stitch.mdx index 79ae126e..41f8ea02 100644 --- a/apps/docs/content/docs/guides/authoring/stitch.mdx +++ b/apps/docs/content/docs/guides/authoring/stitch.mdx @@ -38,7 +38,8 @@ The handful of fields that shape every stitch: - **`baseUrl`** + **`path`** — the request target. `path` may contain `{param}` slots and a `?query` string; in string form the whole string becomes `path`. -- **`method`** — the HTTP verb. Defaults to `GET`. +- **`method`** — the HTTP verb. Defaults to `GET`. Any verb your transport + accepts, including [`QUERY`](#the-query-method). - **The input object** — `params`, `query`, `headers`, and `body` all travel in one `StitchInput` you pass at call time: `await createUser({ params: { id: 1 } })`. - **The generic** — `stitch(...)` types the awaited value `T`. @@ -49,6 +50,52 @@ The handful of fields that shape every stitch: per feature. +## The QUERY method + +Some reads don't fit in a URL. A faceted search, a big list of ids, a nested +filter — encode it as a query string and you hit length limits, proxies that +truncate, and logs that now hold your filter. The usual workaround is to `POST` +the filter and give up everything a read gets: no caching, and a client that +can't tell the call apart from a write. + +`QUERY` ([`draft-ietf-httpbis-safe-method-w-body`](https://datatracker.ietf.org/doc/draft-ietf-httpbis-safe-method-w-body/)) +is the method for exactly that — **safe**, **idempotent**, and **cacheable**, +with a request body. Set it like any other verb: + +```ts +const search = stitch({ + method: 'QUERY', + baseUrl: 'https://api.example.com', + path: '/orders', + // Responses are spec-cacheable, but opt in — the key folds in the body, so two + // different filters get two entries and cannot collide. + cache: { ttl: '1m', methods: 'QUERY' }, +}); + +await search({ body: { status: ['open', 'held'], region: 'eu', limit: 200 } }); +``` + +StitchAPI treats it as the read it is: + +- **The body is sent.** `GET`/`HEAD` have theirs dropped — the transport forbids + one — but `QUERY` keeps it, JSON-encoded like any other body. +- **No idempotency key.** With `idempotency` configured, a `QUERY` is not + stamped with an `Idempotency-Key`; there is no side effect to dedupe. Setting + `idempotency` on one logs the same construction nudge a `GET` gets. +- **A 301/302 stays a `QUERY`.** The downgrade-to-`GET` on a permanent or + temporary redirect is a historical exception granted to `POST`, and the draft + says it does not apply here — downgrading would drop the body, turning a + filtered read into an unfiltered one. A `303` still means "GET the result at + the `Location`", for a `QUERY` as for anything else. +- **`cache.methods` accepts it.** Not in the default `['GET','HEAD']`: like a + GraphQL `POST`, caching a body-carrying request is explicit. + + + `QUERY` is a young method. Check that your server — **and every proxy, CDN, + and WAF between you and it** — actually routes it before reaching for it; + intermediaries have been known to reject or mangle an unfamiliar verb. + + For every field and its default, see [Reference → stitch()](/docs/reference/stitch) and [Reference → Config types](/docs/reference/config-types). diff --git a/apps/docs/content/docs/guides/resilience/idempotency.mdx b/apps/docs/content/docs/guides/resilience/idempotency.mdx index ab1c91f0..31518a8e 100644 --- a/apps/docs/content/docs/guides/resilience/idempotency.mdx +++ b/apps/docs/content/docs/guides/resilience/idempotency.mdx @@ -84,6 +84,17 @@ to silence it when the keyless-retry case (a proxy dedupe, say) is deliberate. See [Reference → Config types](/docs/reference/config-types) for the full `IdempotencyOptions` shape. +## Reads never get a key + +The key rides on **writes only**. Declare `idempotency` on a read and the engine +drops it and logs the same kind of nudge — almost always a missing +`method: 'POST'`. "A read" means every method +[RFC 9110 §9.2.1](https://www.rfc-editor.org/rfc/rfc9110#section-9.2.1) calls +_safe_ — `GET`, `HEAD`, `OPTIONS`, `TRACE` — plus +[`QUERY`](/docs/guides/authoring/stitch#the-query-method), which is safe **and +carries a request body**. Carrying a body is not what makes a call a write, so a +`QUERY` is not stamped: there is no side effect for the server to collapse. + ## See also diff --git a/apps/docs/content/docs/reference/config-types.mdx b/apps/docs/content/docs/reference/config-types.mdx index 46cced5b..d1606b83 100644 --- a/apps/docs/content/docs/reference/config-types.mdx +++ b/apps/docs/content/docs/reference/config-types.mdx @@ -136,6 +136,14 @@ request, so it cannot drift from what it names. +`methods` defaults to `['GET','HEAD']`. Two body-carrying reads opt in by naming +their method: a GraphQL **query** (`methods: 'POST'`) and +[`QUERY`](/docs/guides/authoring/stitch#the-query-method) +(`methods: 'QUERY'`), whose responses are cacheable by spec. Neither is a default — +a `POST`'s read-vs-mutate intent cannot be inferred, and caching a body-carrying +request is a decision worth writing down. The key already folds the request body in, +so two different `QUERY` bodies to the same URL get two entries and cannot collide. + #### CacheFingerprintOptions The shape behind `cache.fingerprint` — how a stored value is detected as stale against diff --git a/apps/docs/lib/source.ts b/apps/docs/lib/source.ts index e8ff2e49..f9a4b195 100644 --- a/apps/docs/lib/source.ts +++ b/apps/docs/lib/source.ts @@ -25,7 +25,7 @@ Search these docs instead of loading the whole file: this site is also a hosted - Capability, not credential: an agent invokes a stitch and gets structured, validated, traceable data; the secret stays behind the boundary. - One context-frugal **code-mode** tool (run_stitch + list_stitches + describe_stitch), not one tool per endpoint — adding APIs never floods the context window. - No server, no codegen, no config files — a URL and one example response is enough; only explicit composition (no ambient/global config a stitch silently inherits). -- Zero-dependency core, ~24 kB min+gzip for the whole entry (~21 kB for a tree-shaken import { stitch }), validator-agnostic (bring your own Standard Schema / Zod), and it runs in the browser. +- Zero-dependency core, ~24 kB min+gzip for the whole entry (~22 kB for a tree-shaken import { stitch }), validator-agnostic (bring your own Standard Schema / Zod), and it runs in the browser. - Composes with your data layer: a stitch is the queryFn for TanStack Query / SWR — it owns the call's resilience; your query layer owns view state. ## Quickstart diff --git a/packages/core/README.md b/packages/core/README.md index 00fd280b..b8c586b2 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -120,7 +120,7 @@ No server, no codegen, no config files, no implicit inheritance — **only expli - **CLI, HTTP & MCP surfaces** - the definition your code imports is also runnable from the shell (`stitch run ` streams JSONL events), served over HTTP (`stitch serve`), or exposed to agents over MCP (`stitch mcp`) — the same stitch behind every front door. - **Typed URLs** - full [RFC 6570](https://datatracker.ietf.org/doc/html/rfc6570) URI templates (`{id}`, `{+path}`, `{?q,sort}`, explode `*`, prefix `:n`), and a `qs`-style query builder that serializes nested objects (`a[b]=c`) and arrays — both dependency-free. - **Pluggable transport** - `fetch` by default; drop in the shipped `axiosAdapter`, or any `Adapter` function, to route requests through axios or another HTTP client. -- **Zero runtime dependencies** - `"dependencies": {}`; built on the platform's global `fetch`; tree-shakeable. The whole entry is **~24 kB min+gzip**; a typical `import { stitch }` trims to **~21 kB** — and with no transitive tree, that is the entire cost. +- **Zero runtime dependencies** - `"dependencies": {}`; built on the platform's global `fetch`; tree-shakeable. The whole entry is **~24 kB min+gzip**; a typical `import { stitch }` trims to **~22 kB** — and with no transitive tree, that is the entire cost. ## Documentation @@ -162,7 +162,7 @@ const { stitch } = require("stitchapi"); The runtime ships with zero dependencies. Schema validation is bring-your-own — pass a [Zod](https://zod.dev) schema or any [Standard Schema](https://standardschema.dev) validator ([Valibot](https://valibot.dev), [ArkType](https://arktype.io), …); none of them is bundled. The examples below use Zod for familiarity. -**Bundle size.** The whole `stitchapi` entry is **~24 kB minified + gzipped** (66 kB raw, ~21 kB brotli); because the package is side-effect-free and every surface beyond `http` lives behind its own subpath import, a typical `import { stitch }` tree-shakes to **~21 kB min+gzip**. With zero dependencies, that is the _whole_ cost — there is no transitive tree to install or audit. +**Bundle size.** The whole `stitchapi` entry is **~24 kB minified + gzipped** (66 kB raw, ~22 kB brotli); because the package is side-effect-free and every surface beyond `http` lives behind its own subpath import, a typical `import { stitch }` tree-shakes to **~22 kB min+gzip**. With zero dependencies, that is the _whole_ cost — there is no transitive tree to install or audit. ## Quick start diff --git a/packages/core/scripts/bundle-size.mjs b/packages/core/scripts/bundle-size.mjs index 55a338af..11976a73 100644 --- a/packages/core/scripts/bundle-size.mjs +++ b/packages/core/scripts/bundle-size.mjs @@ -273,19 +273,45 @@ const KB = 1024; // are UNCHANGED at 24 / 21 this time — 23.91 still rounds to 24 — so no README or docs figure // moves; verified against the `bundle-advertised-size` tether rather than assumed, which is the // mistake the ADR 0024 raise made. +// Budgets raised for the first-class QUERY method — issue #462 part 1 (24.10→24.20 / +// 21.50→21.65 KB; measured 24.12 / 21.56 against a `main` at 24.08 / 21.50, so the whole change +// is +42 / +58 BYTES). `main` had 0.02 KB left on the entry and ONE byte on `import { stitch }` +// (22015 of a 22016 B ceiling), so the entry was full in the literal sense and the next core-path +// change of any size was going to pay for the raise. This is that change. +// +// What the bytes buy: one named `isSafeMethod` predicate (RFC 9110 §9.2.1's safe set plus QUERY) +// replacing the inline `=== 'GET' || === 'HEAD'` in `applyIdempotency` and in the construction +// nudge that mirrors it, so a QUERY — a READ that carries a request body — stops being stamped +// with an `Idempotency-Key`; plus the QUERY exemption from the 301/302 downgrade-to-GET, which +// draft-ietf-httpbis-safe-method-w-body requires by name. Attributed exactly: +33 B is the helper +// and its two rerouted call sites, +21 B is carrying OPTIONS/TRACE in the safe set so the +// predicate means what RFC 9110 says it means, +4 B is the redirect exemption. None of it can +// move behind a subpath — `applyIdempotency` is in `buildRequest`, the nudge is in `makeStitch`, +// and `fetchAdapter` is the default transport; all three run for every stitch. +// +// A MINIMUM step (0.08 / 0.09 KB headroom), not the ~0.2 KB this gate usually restores. The +// change is 58 bytes and the entry is full: keeping the ceiling tight keeps that signal, exactly +// as #477/#524/#485 did. +// +// The ADVERTISED figure moves for `import { stitch }`: 22015 B is 21.499 KB and rounds to 21, +// 22073 B is 21.556 and rounds to 22, so every site quoting it goes ~21 → ~22 kB (the whole entry +// stays ~24). Eight sites, propagated under the `bundle-advertised-size` drift tether — both +// READMEs, the installation and principles pages, the home-page metrics component, and the docs' +// own source blurb. Recorded here because it is the number the project advertises, and a raise +// that left the docs claiming the old one is the exact drift that tether exists to catch. // `advertised: true` means the READMEs/docs quote this scenario's rounded gzip kB — see the // `--json` note below for why that flag, not the row's presence, drives the drift tether. const SCENARIOS = [ { name: 'stitchapi — whole entry', code: `export * from './index.mjs';`, - budget: 24.1 * KB, + budget: 24.2 * KB, advertised: true, }, { name: 'import { stitch }', code: `export { stitch } from './index.mjs';`, - budget: 21.5 * KB, + budget: 21.65 * KB, advertised: true, }, { diff --git a/packages/core/src/engine.ts b/packages/core/src/engine.ts index 481fd349..ad1ac55c 100644 --- a/packages/core/src/engine.ts +++ b/packages/core/src/engine.ts @@ -44,6 +44,7 @@ import { buildQuery, expandPath, getPath, + isSafeMethod, newRunContext, now, parseDuration, @@ -156,7 +157,10 @@ function joinUrl(base: string, path: string): string { // Inject a stable Idempotency-Key on writes. The key is computed once per logical call (here, // in buildRequest) and the attempt loop reuses the same request, so it stays constant across -// retries. GET/HEAD are skipped, and a caller-provided header (case-insensitive) wins. +// retries. SAFE methods are skipped, and a caller-provided header (case-insensitive) wins. +// "Safe" rather than "GET/HEAD" because QUERY is a read that carries a body: stamping a +// dedupe token on a request that changes nothing is a category error, and the header would +// vary the cache key on every send. function applyIdempotency( cfg: ResolvedStitchConfig, input: StitchInput, @@ -164,7 +168,7 @@ function applyIdempotency( headers: Record, ): void { if (!cfg.idempotency) return; - if (method === 'GET' || method === 'HEAD') return; // writes only + if (isSafeMethod(method)) return; // writes only const header = cfg.idempotency.header ?? 'Idempotency-Key'; if ( Object.keys(headers).some( diff --git a/packages/core/src/http-adapter.ts b/packages/core/src/http-adapter.ts index 1318535e..dc72b4da 100644 --- a/packages/core/src/http-adapter.ts +++ b/packages/core/src/http-adapter.ts @@ -239,9 +239,23 @@ async function followRedirects( // Fetch redirect model: 303 (and 301/302 on a non-GET/HEAD) become a bodyless GET; 307/308 // keep method and body. Rebuild headers without content-type when the body is dropped — // and always via a fresh object, since a same-origin headersForRedirect aliased the input. + // + // QUERY is exempt from the 301/302 arm, and this is spec text, not a safety inference: + // the downgrade-to-GET is a HISTORICAL exception granted to POST (RFC 9110 §15.4.2/3), + // and draft-ietf-httpbis-safe-method-w-body says of it in as many words — "the exceptions + // for redirecting a POST as a GET request after a 301 or 302 response do not apply to + // QUERY requests"; the server is asking for "a similar QUERY request to the new target + // URI". Downgrading would drop the body, and the body is the query, so the redirect + // would silently turn a filtered read into an unfiltered one. NOT `isSafeMethod`: + // OPTIONS and TRACE are safe too and no spec exempts them, so they keep today's + // behaviour. 303 stays unconditional — the draft agrees that a 303 to a QUERY means the + // result "can be accomplished via a normal retrieval request" at the Location. if ( status === 303 || - (status < 303 && method !== 'GET' && method !== 'HEAD') + (status < 303 && + method !== 'GET' && + method !== 'HEAD' && + method !== 'QUERY') ) { method = 'GET'; body = undefined; @@ -322,6 +336,11 @@ export function encodeRequestBody(req: AdapterRequest): { contentType?: string; } { const method = req.method.toUpperCase(); + // GET/HEAD only — deliberately NOT `isSafeMethod`. This is a TRANSPORT constraint, not a + // safety rule: `fetch` throws a TypeError when a GET/HEAD init carries a body, and XHR + // ignores it. QUERY is equally safe and MUST keep its body — the body IS the query + // (draft-ietf-httpbis-safe-method-w-body) — so widening this to "safe" would silently + // delete the payload of every QUERY request. if (method === 'GET' || method === 'HEAD') return { body: undefined }; if (req.body === undefined || req.body === null) return { body: undefined }; if (typeof req.body === 'string') return { body: req.body }; diff --git a/packages/core/src/stitch.ts b/packages/core/src/stitch.ts index 903e533b..2c642f4d 100644 --- a/packages/core/src/stitch.ts +++ b/packages/core/src/stitch.ts @@ -72,6 +72,7 @@ import { import { deepMerge, envelope, + isSafeMethod, newRunContext, readEnv, redactSecretsDeep, @@ -382,8 +383,12 @@ export function compose(config: Fragment): ResolvedStitchConfig { // both silenced by `idempotency.warn = false` and both scoped to the **default HTTP surface**: a // surface (graphql → POST) can force the method after construction, so its writes aren't knowable // here, and we don't guess. -// 1. On a read (GET/HEAD) the engine drops the key (writes only) — almost always a missing -// `method`, so the write protection the author expects silently isn't there. +// 1. On a read (any SAFE method — GET/HEAD, and QUERY, which is a read that carries a body) +// the engine drops the key (writes only) — almost always a missing `method`, so the write +// protection the author expects silently isn't there. This shares `isSafeMethod` with +// `applyIdempotency` rather than restating GET/HEAD, so the nudge cannot drift out of step +// with the drop it is warning about: a method the engine silently skips is a method this +// says so about. // 2. The *random* default key only dedupes a replay of the same request, and `retry` is what // replays it; with no `retry` it usually has nothing to collapse. (Not useless in every case — // a proxy/transport resending the request below the stitch carries the same key for a server @@ -424,7 +429,7 @@ function warnConstruction(cfg: ResolvedStitchConfig): void { // method semantics" skip has to ask which surface — not whether there is one. if (!idem || idem.warn === false || cfg.kind.id !== 'http') return; const method = (cfg.method ?? 'GET').toUpperCase(); - if (method === 'GET' || method === 'HEAD') { + if (isSafeMethod(method)) { console.warn( `stitchapi: \`${name}\` sets \`idempotency\` on a ${method}, but the key is sent on ` + `writes only — set \`method: 'POST'\`, or drop \`idempotency\`.`, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 78b0861f..bdb07a52 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1211,9 +1211,11 @@ export interface CircuitOptions { * set a correlation/trace header like `traceparent` or `X-Request-Id`; those identify a request * for logs and spans and belong to tracing, not dedupe. * - * The key is sent on **writes only**; setting `idempotency` on a read (GET/HEAD) drops it and logs - * a construction nudge — almost always a missing `method: 'POST'`. Both nudges fire only on the - * default HTTP surface and are silenced by `warn: false`. + * The key is sent on **writes only**; setting `idempotency` on a read drops it and logs a + * construction nudge — almost always a missing `method: 'POST'`. "A read" is every method RFC 9110 + * §9.2.1 calls *safe* (GET, HEAD, OPTIONS, TRACE) plus `QUERY`, which is safe but carries a body — + * so a body on the request is not what makes it a write. Both nudges fire only on the default HTTP + * surface and are silenced by `warn: false`. */ export interface IdempotencyOptions { /** @@ -1327,6 +1329,12 @@ export interface CacheOptions { * its method (`'POST'`) — a POST's read-vs-mutate intent cannot be inferred, so it is * explicit. Coalescing applies to exactly this set; mutations are never cached. A bare string * is shorthand for a one-element list (CONTRACT.md P7). + * + * `'QUERY'` is a valid entry, and its responses are spec-cacheable + * ([draft-ietf-httpbis-safe-method-w-body](https://datatracker.ietf.org/doc/draft-ietf-httpbis-safe-method-w-body/)). + * It is **not** in the default set — like a GraphQL POST, caching a body-carrying request is + * opt-in. The key already folds the request body in, so two different `QUERY` bodies to the + * same URL get two entries and cannot collide: `cache: { ttl: '1m', methods: 'QUERY' }`. */ methods?: string | string[]; /** In-process LRU cap on live entries (the store stays dumb). Default 1000. */ @@ -1577,6 +1585,14 @@ export interface PaginateOptions { /** Safety cap on pages. Default 50. */ pages?: number; } +/** + * The HTTP methods StitchAPI knows about — the RFC 9110 verbs plus `QUERY` + * (`draft-ietf-httpbis-safe-method-w-body`). Purely an **autocomplete list**: `method` is + * `KnownMethod | (string & {})`, so any other verb your transport accepts still typechecks. + */ +export type KnownMethod = + 'GET' | 'HEAD' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' | 'OPTIONS' | 'QUERY'; + export interface StitchConfig { /** Label used in events and traces; defaults to `path` or `'stitch'`. */ name?: string; @@ -1586,8 +1602,17 @@ export interface StitchConfig { * declaration round-trips as JSON — Decision 11); the live object stays on `__rawConfig`. */ kind?: Surface; - /** HTTP method; defaults to `GET`. */ - method?: string; + /** + * HTTP method; defaults to `GET`. Any verb the transport accepts, including **`QUERY`** — the + * safe, idempotent, cacheable method that carries a **request body** + * (`draft-ietf-httpbis-safe-method-w-body`), for a read whose filter is too large or too + * structured for a URL. The engine classifies `QUERY` as a read: no `Idempotency-Key` is + * stamped on it, and a 301/302 re-sends it as a `QUERY` rather than downgrading it to a + * bodyless `GET`. It keeps its body (unlike `GET`/`HEAD`, where the transport forbids one), + * and it is a valid {@link CacheOptions.methods} entry — opt in, since caching a + * body-carrying request is never a default. + */ + method?: KnownMethod | (string & {}); /** * Wire-format options — request body encoding, response decoding, and urlencoded array * serialisation, grouped by category rather than by request/response phase (CONTRACT.md P24). diff --git a/packages/core/src/util.ts b/packages/core/src/util.ts index ca65c69c..05d22277 100644 --- a/packages/core/src/util.ts +++ b/packages/core/src/util.ts @@ -220,6 +220,31 @@ export function stripTrailingSlashes(s: string): string { return end === s.length ? s : s.slice(0, end); } +// ---- HTTP method classification ------------------------------------------- +// RFC 9110 §9.2.1's safe set — "essentially read-only", so a server may treat the request as +// having no side effect — plus QUERY (draft-ietf-httpbis-safe-method-w-body: "QUERY requests +// are safe with regard to the target resource"). QUERY is the reason this set exists as a named +// predicate rather than a fourth inline `=== 'GET' || === 'HEAD'`: it is a **read that carries a +// request body**, so "safe" and "GET-shaped" stopped being the same question. +// +// This answers exactly one question — *is this request a read?* — and is deliberately NOT used +// for the other two method tests in the transport, which look alike and are not: +// • `encodeRequestBody` drops the body on GET/HEAD only. That is a fetch/XHR constraint (a +// GET with a body is a TypeError), not a safety rule — routing it through here would delete +// the body of every QUERY, i.e. the one thing that already worked. +// • `followRedirects` exempts GET/HEAD/QUERY from the 301/302 downgrade-to-GET. That is the +// draft's explicit carve-out from a HISTORICAL POST exception, not a safety rule — OPTIONS +// and TRACE are safe and are not exempted by any spec text. +const SAFE_METHODS = new Set(['GET', 'HEAD', 'OPTIONS', 'TRACE', 'QUERY']); + +/** + * Is `method` **safe** (RFC 9110 §9.2.1) — a read, requesting no change to the target resource? + * Safe methods are also idempotent, so this is the predicate for "the engine must not treat this + * as a write". Case-insensitive; an unknown verb is assumed unsafe (fail closed). + */ +export const isSafeMethod = (method: string): boolean => + SAFE_METHODS.has(method.toUpperCase()); + export const isObj = (x: unknown): x is Record => !!x && typeof x === 'object' && !Array.isArray(x); diff --git a/packages/core/test-d/known-method.test-d.ts b/packages/core/test-d/known-method.test-d.ts new file mode 100644 index 00000000..6a238e04 --- /dev/null +++ b/packages/core/test-d/known-method.test-d.ts @@ -0,0 +1,37 @@ +// `StitchConfig.method` is `KnownMethod | (string & {})` (issue #462 part 1, gap 4): the union arm +// gives an IDE the eight verbs to autocomplete — `QUERY` among them, which is the whole point, since +// nothing else would tell an author it exists — while `string & {}` keeps the field open. Widening a +// `string` this way is only safe if it is PURELY additive, so that is what this pins: everything that +// typechecked before still does. +import { stitch } from '../src'; +import type { KnownMethod, Stitch } from '../src'; + +import { expectAssignable, expectType } from 'tsd'; + +// ── The known verbs, including QUERY ──────────────────────────────────────── +expectType>( + stitch({ url: 'https://api.example.com/orders', method: 'QUERY' }), +); +for (const m of [ + 'GET', + 'HEAD', + 'POST', + 'PUT', + 'PATCH', + 'DELETE', + 'OPTIONS', + 'QUERY', +] as const) + expectAssignable(m); + +// ── Still open: a custom verb is not an error ─────────────────────────────── +// The door stays open for WebDAV, a vendor verb, or a method the spec adds next — the union is an +// autocomplete list, never an allowlist. +expectType>( + stitch({ url: 'https://api.example.com/x', method: 'PROPFIND' }), +); +// Including a method that is only known at runtime, which a closed union would have rejected. +declare const runtimeVerb: string; +expectType>( + stitch({ url: 'https://api.example.com/x', method: runtimeVerb }), +); diff --git a/packages/core/test/query-method.spec.ts b/packages/core/test/query-method.spec.ts new file mode 100644 index 00000000..0500248e --- /dev/null +++ b/packages/core/test/query-method.spec.ts @@ -0,0 +1,387 @@ +// QUERY as a first-class method (issue #462 part 1). +// +// QUERY is the safe, idempotent, *cacheable* method that carries a request body +// (draft-ietf-httpbis-safe-method-w-body) — "a GET whose filter is too big or too structured for a +// URL". It worked here by accident: the engine's notion of a safe method was hard-coded to +// GET/HEAD in four places, so a QUERY was treated as a write everywhere except the one branch that +// happened to let its body through. +// +// The three method tests in the transport LOOK alike and ask different questions; this suite pins +// each one separately, because conflating them is exactly how a QUERY loses its body: +// • `isSafeMethod` — "is this a read?" → no Idempotency-Key, and the construction nudge that +// mirrors that drop. GET/HEAD/OPTIONS/TRACE/QUERY. +// • `encodeRequestBody` — "may this method carry a body on the wire?" → GET/HEAD only (a +// transport constraint: `fetch` throws on a GET with a body). QUERY keeps its body. +// • `followRedirects` — "does a 301/302 downgrade this to a bodyless GET?" → the historical POST +// exception, which the draft says explicitly does NOT apply to QUERY. GET/HEAD/QUERY exempt. +// +// Every GET/HEAD/POST assertion below is a regression guard on unchanged behaviour, not new +// behaviour. +import { fetchAdapter, stitch } from '../src'; +import { encodeRequestBody } from '../src/http-adapter'; +import { mockAdapter } from '../src/testing'; +import type { Adapter, AdapterRequest } from '../src/types'; +import { isSafeMethod } from '../src/util'; +import { startMockServer } from './support/mock-server'; +import type { MockServer } from './support/mock-server'; + +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +// Keep this suite's trace sink out of the console (matches the other transport specs). +process.env['STITCH_TRACE_FILE'] = join( + tmpdir(), + `stitch-query-method-${process.pid}.jsonl`, +); + +const req = (over: Partial = {}): AdapterRequest => ({ + url: 'https://api.test/search', + method: 'QUERY', + headers: {}, + ...over, +}); + +// Pull a header case-insensitively from a recorded request. +const hdr = (h: Record, name: string): string | undefined => { + const k = Object.keys(h).find( + (x) => x.toLowerCase() === name.toLowerCase(), + ); + return k ? h[k] : undefined; +}; + +// A scripted fetch that answers a redirect chain and records the method/body/url of every hop, so +// a test can assert what the redirect TARGET was actually sent. Typed as `typeof fetch` (the arrow +// is structurally assignable, so no cast is needed). +function redirectingFetch(steps: { status: number; location?: string }[]): { + fetch: typeof fetch; + hops: { url: string; method: string; body: unknown }[]; +} { + const hops: { url: string; method: string; body: unknown }[] = []; + let i = 0; + const fetch: typeof globalThis.fetch = (input, init) => { + hops.push({ + url: input as string, + method: init?.method ?? 'GET', + body: init?.body, + }); + const step = steps[Math.min(i, steps.length - 1)]; + i++; + const headers = new Headers(); + if (step?.location) headers.set('location', step.location); + const redirecting = + step !== undefined && step.status >= 300 && step.status < 400; + if (!redirecting) headers.set('content-type', 'application/json'); + return Promise.resolve( + new Response(redirecting ? null : '{"ok":true}', { + status: step?.status ?? 200, + headers, + }), + ); + }; + return { fetch, hops }; +} + +describe('isSafeMethod — one predicate for "is this a read?"', () => { + test('RFC 9110 §9.2.1 safe set, plus QUERY', () => { + for (const m of ['GET', 'HEAD', 'OPTIONS', 'TRACE', 'QUERY']) + expect(isSafeMethod(m)).toBe(true); + // Case-insensitive: `cfg.method` is uppercased by the engine, but the helper is also + // reachable from an adapter, where the method is whatever the caller wrote. + expect(isSafeMethod('query')).toBe(true); + }); + + test('writes, and an unknown verb, are not safe (fail closed)', () => { + for (const m of ['POST', 'PUT', 'PATCH', 'DELETE', 'PURGE', '']) + expect(isSafeMethod(m)).toBe(false); + }); +}); + +describe('encodeRequestBody — a QUERY keeps its body', () => { + test('the body survives, JSON-encoded, with a content-type', () => { + const out = encodeRequestBody( + req({ body: { filter: { tags: ['a', 'b'] } } }), + ); + expect(out.body).toBe('{"filter":{"tags":["a","b"]}}'); + expect(out.contentType).toBe('application/json'); + }); + + test('a QUERY form/multipart body encodes like any other body-bearing method', () => { + expect( + encodeRequestBody(req({ body: { a: 1, b: 2 }, bodyType: 'form' })) + .body, + ).toBe('a=1&b=2'); + expect( + encodeRequestBody(req({ body: { f: 'v' }, bodyType: 'multipart' })) + .body, + ).toBeInstanceOf(FormData); + }); + + test('GET/HEAD still drop a body — the transport, not safety, is what decides here', () => { + expect( + encodeRequestBody(req({ method: 'GET', body: { a: 1 } })).body, + ).toBeUndefined(); + expect( + encodeRequestBody(req({ method: 'HEAD', body: { a: 1 } })).body, + ).toBeUndefined(); + // OPTIONS is safe too, and is NOT body-stripped — proof this branch is not `isSafeMethod`. + expect( + encodeRequestBody(req({ method: 'OPTIONS', body: { a: 1 } })).body, + ).toBe('{"a":1}'); + }); +}); + +describe('idempotency — a safe method is never stamped with a key', () => { + // Drive the engine through the published mock transport and read the request it built. + const sent = async (method: string): Promise => { + const api = mockAdapter({ respond: { body: { ok: true } } }); + const s = stitch({ + method, + url: 'https://api.test/search', + adapter: api, + trace: false, + idempotency: { warn: false }, // the nudge is asserted separately, below + }); + await s({ body: { q: 'ada' } }); + return api.lastRequest() as AdapterRequest; + }; + + test('a QUERY gets no Idempotency-Key — and still sends its body', async () => { + const r = await sent('QUERY'); + expect(r.method).toBe('QUERY'); + expect(hdr(r.headers, 'idempotency-key')).toBeUndefined(); + expect(r.body).toEqual({ q: 'ada' }); + }); + + test('GET/HEAD are unchanged, and OPTIONS/TRACE now classify as the reads they are', async () => { + for (const m of ['GET', 'HEAD', 'OPTIONS', 'TRACE']) + expect(hdr((await sent(m)).headers, 'idempotency-key')).toBe( + undefined, + ); + }); + + test('a write still gets one — POST/PUT/PATCH/DELETE are untouched', async () => { + for (const m of ['POST', 'PUT', 'PATCH', 'DELETE']) + expect( + hdr((await sent(m)).headers, 'idempotency-key'), + ).toBeTruthy(); + }); + + test('a caller-supplied key still wins on a write, and is not invented on a QUERY', async () => { + const api = mockAdapter({ respond: { body: { ok: true } } }); + const make = (method: string): ReturnType => + stitch({ + method, + url: 'https://api.test/search', + adapter: api, + trace: false, + idempotency: { warn: false }, + }); + await make('POST')({ headers: { 'Idempotency-Key': 'mine' } }); + expect(hdr(api.lastRequest()!.headers, 'idempotency-key')).toBe('mine'); + await make('QUERY')({ headers: { 'Idempotency-Key': 'mine' } }); + expect(hdr(api.lastRequest()!.headers, 'idempotency-key')).toBe('mine'); + }); +}); + +describe('the construction nudge tracks the drop it warns about', () => { + const nudgeFor = (method: string): string | undefined => { + const warn = vi + .spyOn(console, 'warn') + .mockImplementation(() => undefined); + try { + stitch({ + method, + url: 'https://api.test/search', + trace: false, + idempotency: true, + }); + return warn.mock.calls[0]?.[0] as string | undefined; + } finally { + warn.mockRestore(); + } + }; + + test('a QUERY with `idempotency` is nudged, exactly as a GET is', () => { + // Without this the key would be silently dropped with nothing said — the failure mode the + // nudge exists to prevent, reintroduced by the engine-side fix. + const msg = nudgeFor('QUERY'); + expect(msg).toContain('writes only'); + expect(msg).toContain('QUERY'); + expect(nudgeFor('GET')).toContain('writes only'); // unchanged + }); + + test('a write is not nudged for being a read (it has a `retry`, so no second nudge either)', () => { + const warn = vi + .spyOn(console, 'warn') + .mockImplementation(() => undefined); + try { + stitch({ + method: 'POST', + url: 'https://api.test/search', + trace: false, + retry: { attempts: 2 }, + idempotency: true, + }); + expect(warn).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); +}); + +describe('redirects — a 301/302 does not downgrade a QUERY', () => { + test('a 301 re-sends the QUERY, body intact, to the new target', async () => { + const { fetch: spy, hops } = redirectingFetch([ + { status: 301, location: 'https://api.test/v2/search' }, + { status: 200 }, + ]); + await fetchAdapter({ fetch: spy })({ + url: 'https://api.test/search', + method: 'QUERY', + headers: {}, + body: { q: 'ada' }, + }); + expect(hops).toHaveLength(2); + expect(hops[1]?.url).toBe('https://api.test/v2/search'); + expect(hops[1]?.method).toBe('QUERY'); + expect(hops[1]?.body).toBe('{"q":"ada"}'); + }); + + test('a 302 likewise — while a POST is still downgraded to a bodyless GET', async () => { + const script = [ + { status: 302, location: 'https://api.test/moved' }, + { status: 200 }, + ]; + const q = redirectingFetch(script); + await fetchAdapter({ fetch: q.fetch })({ + url: 'https://api.test/search', + method: 'QUERY', + headers: {}, + body: { q: 'ada' }, + }); + expect(q.hops[1]?.method).toBe('QUERY'); + + const p = redirectingFetch(script); + await fetchAdapter({ fetch: p.fetch })({ + url: 'https://api.test/search', + method: 'POST', + headers: {}, + body: { q: 'ada' }, + }); + expect(p.hops[1]?.method).toBe('GET'); // the historical POST exception, unchanged + expect(p.hops[1]?.body).toBeUndefined(); + }); + + test('a 303 still means "GET the result over there", for a QUERY too', async () => { + const { fetch: spy, hops } = redirectingFetch([ + { status: 303, location: 'https://api.test/results/7' }, + { status: 200 }, + ]); + await fetchAdapter({ fetch: spy })({ + url: 'https://api.test/search', + method: 'QUERY', + headers: {}, + body: { q: 'ada' }, + }); + expect(hops[1]?.method).toBe('GET'); + expect(hops[1]?.body).toBeUndefined(); + }); + + test('a 307 keeps method and body, as it always did', async () => { + const { fetch: spy, hops } = redirectingFetch([ + { status: 307, location: 'https://api.test/v2/search' }, + { status: 200 }, + ]); + await fetchAdapter({ fetch: spy })({ + url: 'https://api.test/search', + method: 'QUERY', + headers: {}, + body: { q: 'ada' }, + }); + expect(hops[1]?.method).toBe('QUERY'); + expect(hops[1]?.body).toBe('{"q":"ada"}'); + }); +}); + +describe('cache — QUERY is opt-in, and keys on the request body', () => { + // Counts origin calls so "served from cache" is observable as "the origin was not called". + const counting = (): { adapter: Adapter; calls: () => number } => { + let calls = 0; + const adapter: Adapter = (r) => + Promise.resolve({ + status: 200, + headers: {}, + body: { n: (calls += 1), echo: r.body }, + }); + return { adapter, calls: () => calls }; + }; + + test('`methods: "QUERY"` caches; two different bodies do not collide', async () => { + const { adapter, calls } = counting(); + const search = stitch({ + method: 'QUERY', + url: 'https://api.test/search', + adapter, + trace: false, + cache: { ttl: '60s', tenancy: 'app', methods: 'QUERY' }, + }); + expect(await search({ body: { q: 'ada' } })).toEqual({ + n: 1, + echo: { q: 'ada' }, + }); + expect(await search({ body: { q: 'ada' } })).toEqual({ + n: 1, + echo: { q: 'ada' }, + }); // hit + expect(calls()).toBe(1); + // A different body is a different query — it must MISS, not be served the first answer. + expect(await search({ body: { q: 'grace' } })).toEqual({ + n: 2, + echo: { q: 'grace' }, + }); + expect(calls()).toBe(2); + }); + + test('the default `[GET, HEAD]` still does not cache a QUERY (opt-in, unchanged)', async () => { + const { adapter, calls } = counting(); + const search = stitch({ + method: 'QUERY', + url: 'https://api.test/search', + adapter, + trace: false, + cache: { ttl: '60s', tenancy: 'app' }, + }); + await search({ body: { q: 'ada' } }); + await search({ body: { q: 'ada' } }); + expect(calls()).toBe(2); + }); +}); + +describe('end to end, over a real socket', () => { + let server: MockServer; + beforeAll(async () => { + server = await startMockServer(); + }); + afterAll(async () => { + await server.close(); + }); + + test('a QUERY stitch sends its body and no Idempotency-Key', async () => { + server.route('QUERY', '/search', { body: { hits: 2 } }); + const search = stitch({ + method: 'QUERY', + baseUrl: server.url, + path: '/search', + trace: false, + idempotency: { warn: false }, + }); + expect(await search({ body: { filter: { tag: 'ada' } } })).toEqual({ + hits: 2, + }); + const call = server.calls('/search')[0]!; + expect(call.method).toBe('QUERY'); + expect(call.body).toEqual({ filter: { tag: 'ada' } }); + expect(call.headers['content-type']).toBe('application/json'); + expect(call.headers['idempotency-key']).toBeUndefined(); + }); +});