fix(core): an unusable backoff throws at construction instead of vanishing

CHANGELOG.md

@@ -689,6 +689,29 @@ npm release are grouped under the in-development version that introduced them.
     cap exists for. Nothing about the public API changes; a stream that used to die at 8 MB now
     finishes, in bounded memory.
 
+- **An unusable `backoff` throws at construction instead of vanishing.**
+  ([#651](https://github.com/rejifald/StitchAPI/issues/651) §3) `backoff` takes a curve or the
+  `{ curve, base, max }` envelope — never a function — so `backoff: () => 6000` is correctly a type
+  error. Casting past it (which people do when they believe a feature exists) constructed clean and
+  then did **nothing**: the function was never invoked, and the waits fell back to the default curve
+  on the default 100ms base. Measured gaps of `100, 200`ms where the config asked for 6000, with no
+  throw, no event, and nothing in the trace that reads as wrong.
+
+    `expandShorthand` folds the bare form of the slot into `{ curve }` (P12), so every value a cast
+    can let through — a function, a bare `6000`, a misremembered `'exponential'` — lands on `curve`,
+    where `backoffDelay` matched neither `'fixed'` nor `'expo-jitter'` and fell through to the plain
+    `expo` branch. It now throws `bad backoff` from `stitch()`, mirroring the
+    `bad rate: …` an unparseable `throttle.rate` has thrown at construction since
+    [#618](https://github.com/rejifald/StitchAPI/pull/618). Silently degrading a resilience policy is
+    the one place a fallback is worse than a crash, and this one degraded in the **permissive**
+    direction — a shorter wait than asked for, which is the half that hurts.
+
+    The check sits at the fold itself, on the value it just produced, so one comparison over the
+    dominant field covers every authoring form and names the offending **fragment** rather than the
+    merged result. A `backoff` that sets `base`/`max` and no curve keeps the `expo-jitter` default,
+    and all three curve names are unaffected in either spelling. **Not a semver break in practice**: only a value `tsc` already
+    rejected can reach the throw, and its previous behaviour was to ignore what you wrote.
+
 - **A `bigint` path parameter no longer vanishes from the URL.** `stitch({ path: '/v1/things/{id}' })`
   called with `{ params: { id: 1234567890123456789n } }` built `https://api.test/v1/things/` — the id
   simply gone, no error, no event, no drift finding. A request meant for one item silently addressed

packages/core/src/stitch.ts

@@ -30,6 +30,7 @@ import { createStoreThrottle, memoryStore } from './store';
 import { graphqlSurface, httpSurface } from './surface';
 import { consoleSink, createTrace, exportsFromEnv, multiplex } from './trace';
 import {
+    type AtLeastOne,
     type CacheOptions,
     type CacheOutcome,
     type Clock,
@@ -212,12 +213,48 @@ function expandShorthand(cfg: Partial<StitchConfig>): void {
     // Nested fold (P24): `backoff` is itself a scalar-or-envelope slot, so the bare curve
     // normalizes too — `__config` never carries the string form (P0). Read back through the
     // normalised shape the loop just wrote.
+    //
+    // **And the fold is where an unusable curve dies** (issue #651 §3), on the value it just
+    // produced — `bad backoff`, the same slot-named message and the same construction-time timing
+    // `parseRate` has for an unparseable `throttle.rate`. Silently degrading a resilience
+    // policy is the one place a fallback is worse than a crash, and this one degraded in the
+    // PERMISSIVE direction: `backoff: () => 6000` cast past its type folded to `{ curve: <fn> }`,
+    // matched neither branch of `backoffDelay`, and walked the plain `expo` curve on the default
+    // 100ms base — measured gaps of 100, 200ms where the config asked for 6000, with nothing said
+    // anywhere. Checking HERE rather than after the merge is what makes ONE check cover the whole
+    // slot: every spelling a cast can let through (a function, a bare `6000`, a misremembered
+    // `'exponential'`, an array, `null`) is non-object, so the fold has just put it on `curve`. It
+    // also names the offending FRAGMENT rather than the merged result, and reuses the accessor the
+    // fold already needs — which, with the terse message, is what buys the whole check its 33
+    // bytes on a path that had 33 left.
+    //
+    // Only a PRESENT curve is judged, and only against the three values `backoffDelay` dispatches
+    // on. Nothing here decides what a *reasonable* backoff is.
     const retry = cfg.retry as RetryOptions | undefined;
-    if (retry?.backoff !== undefined)
-        cfg.retry = {
-            ...retry,
-            backoff: envelope(retry.backoff, 'curve'),
-        };
+    if (retry?.backoff !== undefined) {
+        // Read the folded envelope's `curve` at `string`, not at `BackoffCurve`. The whole point of
+        // the check is a value the TYPE already excludes, so narrowing against the union would leave
+        // `never` and the exhaustive comparison would read as dead code to tsc and eslint alike —
+        // which is the shape of a guard that cannot fire, not the one that fires here every time a
+        // cast lets something through.
+        const backoff = envelope(retry.backoff, 'curve') as { curve?: string };
+        // Three shapes here are load-bearing for SIZE, not for style, and this check lands one
+        // byte inside the gate — so measure before touching any of them:
+        //   • `'expo-jitter'` FIRST, so the `'expo'` literal after it compresses as a
+        //     back-reference into the one already emitted (4 B; alphabetising costs them back);
+        //   • a `!==` chain rather than `['expo', …].includes(…)` (the array measured 4 B worse);
+        //   • a message with no interpolated value (5 B). `parseRate` can afford `bad rate: ${r}`
+        //     and this cannot — the offending value is in the caller's own `stitch({…})` literal,
+        //     which is the one consolation. Restore it the moment the budget has room.
+        if (
+            backoff.curve !== undefined &&
+            backoff.curve !== 'expo-jitter' &&
+            backoff.curve !== 'expo' &&
+            backoff.curve !== 'fixed'
+        )
+            throw new Error('bad backoff');
+        cfg.retry = { ...retry, backoff } as AtLeastOne<RetryOptions>;
+    }
     // Nested fold (P25): `stream.buffer` is a scalar-or-envelope slot too — the bare char count
     // folds to `{ chars }`, so `__config` never carries the number form (P0).
     const stream = cfg.stream as StreamOptions | undefined;

packages/core/src/types.ts

@@ -1056,6 +1056,16 @@ export interface RetryOptions {
     /**
      * Backoff policy. A bare curve is the P12 shorthand for `{ curve }`
      * (`backoff: 'fixed'` ≡ `backoff: { curve: 'fixed' }`); the envelope adds `base`/`max`.
+     *
+     * A `curve` outside {@link BackoffCurve} **throws** at construction — `bad backoff`, the
+     * same fail-loud stance as {@link ThrottleOptions.rate}, and for the same reason: the
+     * quiet path here is the PERMISSIVE one. An unreadable curve fell through to plain `expo` on the
+     * default 100ms base, so a typo shortened the wait instead of lengthening it, and a `backoff` the
+     * runtime cannot read is a resilience policy that silently isn't there.
+     *
+     * There is deliberately **no function form** — the curve plus its two bounds is the whole
+     * vocabulary. A computed wait per attempt belongs to a `Surface`: `interpret` returns a
+     * `SurfaceOutcome` whose `after` is honoured as the next delay (issue #609).
      */
     backoff?: BackoffCurve | AtLeastOne<BackoffOptions>;
     /**

packages/core/test/resilience-config-guard.spec.ts

@@ -0,0 +1,137 @@
+// Issue #651 §3 — a `backoff` the retry loop cannot read must THROW at construction, the way an
+// unparseable `throttle.rate` already does (`bad rate: …`, util.ts `parseRate`), rather than
+// degrading to a default nobody asked for. The message names the slot the way that precedent
+// does — `bad backoff` — but stops there rather than quoting the value: the entry's gzip gate had
+// one byte left for this whole check, and the interpolation cost five. The offending value is in
+// the caller's own `stitch({…})` literal, which is what makes that trade survivable.
+//
+// The reported case: `backoff: () => 6000` is correctly a TYPE error, but a cast past it (which
+// people write when they believe a feature exists) constructed clean and then never invoked the
+// function — `expandShorthand` folds the bare form into `{ curve: <fn> }`, `backoffDelay` matches
+// neither `'fixed'` nor `'expo-jitter'`, and the wait silently became the plain `expo` curve on the
+// default 100ms base. Measured gaps of 100, 200ms where the config asked for 6000. Because the fold
+// runs first, every authoring spelling of the slot lands on `curve`, so the cases below are one
+// check seen from four directions rather than four checks.
+//
+// The other resilience slots were swept for the same defect and are NOT guarded here — the core
+// entry's gzip budget is measured in tens of bytes and this check spends ~34 of them; see the PR
+// for the measured cost of extending to each. What they still do on a value cast past the type is therefore unchanged,
+// and the last block below is the part that matters for them: no legitimate shape may start
+// throwing as a side effect of this one.
+import { stitch } from '../src';
+import type { StitchConfig } from '../src/types';
+
+// A construction attempt with a value the type system rejects — the cast IS the reported scenario.
+const bad = (extra: Record<string, unknown>) => () =>
+    stitch({ url: 'https://example.test/x', ...extra } as never);
+// The same, for a config that is legal and must keep constructing. Deliberately NOT cast: these
+// have to typecheck as well as run.
+const ok = (extra: Partial<StitchConfig>) => () =>
+    stitch({ url: 'https://example.test/x', ...extra });
+
+describe('an unusable `backoff` throws at construction (#651 §3)', () => {
+    test('the function form — a type error, and a cast past it no longer vanishes', () => {
+        // The reported reproduction: typechecked, then vanished, before this guard existed.
+        expect(bad({ retry: { attempts: 3, backoff: () => 6000 } })).toThrow(
+            /^bad backoff$/,
+        );
+    });
+
+    test('a curve name that is not one of the three', () => {
+        expect(bad({ retry: { attempts: 3, backoff: 'exponential' } })).toThrow(
+            /^bad backoff$/,
+        );
+        expect(
+            bad({ retry: { attempts: 3, backoff: { curve: 'linear' } } }),
+        ).toThrow(/^bad backoff$/);
+    });
+
+    test('a bare number — `backoff: 6000` is not the P12 shorthand (`curve` is)', () => {
+        expect(bad({ retry: { attempts: 3, backoff: 6000 } })).toThrow(
+            /^bad backoff$/,
+        );
+    });
+
+    test('the shorthand fold is what makes one check cover every spelling', () => {
+        // Not an object ⇒ the bare form ⇒ folded onto `curve`, whatever it was.
+        expect(bad({ retry: { attempts: 3, backoff: ['expo'] } })).toThrow(
+            /^bad backoff$/,
+        );
+        expect(bad({ retry: { attempts: 3, backoff: null } })).toThrow(
+            /^bad backoff$/,
+        );
+    });
+
+    test('it throws at CONSTRUCTION, not on the first retry', () => {
+        // The distinction the issue asks for: nothing is called, and it still fails.
+        let built = false;
+        expect(() => {
+            stitch({
+                url: 'https://example.test/x',
+                retry: { attempts: 3, backoff: 'exponential' },
+            } as never);
+            built = true;
+        }).toThrow(/^bad backoff$/);
+        expect(built).toBe(false);
+    });
+
+    test('the precedent this mirrors: `throttle.rate` already threw at construction', () => {
+        expect(bad({ throttle: 'fast' })).toThrow(/bad rate: fast/);
+    });
+});
+
+describe('every legitimate resilience shape still constructs', () => {
+    test('all three curves, in both the bare and the envelope spelling', () => {
+        for (const curve of ['expo', 'expo-jitter', 'fixed'] as const) {
+            expect(
+                ok({ retry: { attempts: 3, backoff: curve } }),
+            ).not.toThrow();
+            expect(
+                ok({ retry: { attempts: 3, backoff: { curve } } }),
+            ).not.toThrow();
+        }
+    });
+
+    test('a `backoff` envelope that sets no curve keeps the default', () => {
+        expect(
+            ok({ retry: { attempts: 3, backoff: { base: 50 } } }),
+        ).not.toThrow();
+        expect(
+            ok({ retry: { attempts: 3, backoff: { base: '1s', max: '10s' } } }),
+        ).not.toThrow();
+    });
+
+    test('numbers and duration tokens on every widened slot (P17)', () => {
+        expect(
+            ok({
+                retry: {
+                    attempts: 3,
+                    on: 429,
+                    respect: false,
+                    backoff: { curve: 'expo', base: '1s', max: 10_000 },
+                },
+                timeout: { total: '10s', each: 3000 },
+                throttle: { rate: '2/s', concurrency: 4, lease: '45s' },
+                circuit: { failures: 5, cooldown: 30_000 },
+            }),
+        ).not.toThrow();
+    });
+
+    test('the scalar shorthands and the positional circuit tuple', () => {
+        expect(ok({ retry: 3 })).not.toThrow();
+        expect(ok({ timeout: '5s' })).not.toThrow();
+        expect(ok({ timeout: 5000 })).not.toThrow();
+        expect(ok({ throttle: '2/s' })).not.toThrow();
+        expect(ok({ circuit: [5, '30s'] })).not.toThrow();
+    });
+
+    test('an omitted slot, and a slot whose guarded field is omitted', () => {
+        expect(ok({})).not.toThrow();
+        expect(ok({ retry: { on: 503 } })).not.toThrow();
+        expect(ok({ timeout: { total: '1s' } })).not.toThrow();
+        expect(ok({ throttle: { pool: 'host', rate: '1/s' } })).not.toThrow();
+        // `circuit` with neither required field still defers to `createCircuit`'s
+        // required-by-design throw at CALL time (CONTRACT.md P15) — this guard must not pre-empt it.
+        expect(ok({ circuit: { key: 'k' } })).not.toThrow();
+    });
+});