Files
2026-05-27 23:29:53 +01:00

97 lines
8.7 KiB
Markdown

# Retry — `withCircuitBreaker` convenience wrapper
**Package**: `@suaveplan/retry`
**Status**: PROPOSED
**Type**: feature
## Why
`@suaveplan/retry` ships both retry strategies (`retry`, `retryAttempt`, `retrySync`) and a circuit breaker (`CircuitBreaker` with closed → open → half-open state machine), but it does not ship the convenience wrapper that combines them. Consumers wanting "retry until the breaker opens, then short-circuit" — the canonical resilience pattern — currently have to wire it themselves:
```ts
const breaker = new CircuitBreaker({ failureThreshold: 5, resetTimeoutMs: 30_000 })
const result = await retry(() => breaker.execute(fn), { maxAttempts: 3, strategy: 'exponential' })
return result.value
```
This is correct but error-prone — the order matters (retry outside, breaker inside), the unwrap of `result.value` is easy to forget, and the breaker handle is leaked across the call site if anyone needs to inspect state. Upstream shipped `withCircuitBreaker(cbConfig, retryConfig?)` (`packages/universal/retry/src/retry/retry.ts` L241-L260) which packages this composition behind a single factory call returning `{ execute, getCircuitBreaker }`. Genesis dropped it without a replacement.
The 2026-05-02 audit found nine genesis packages already depending on `@suaveplan/retry` (`auth`, `auth-jwt-tenant`, `db-sqlite`, `surrealdb`, `network-websockets`, `webhook-client`, `storage-s3`, `elevenlabs`, `retry` itself), and at least three of them (`webhook-client`, `storage-s3`, `network-websockets`) are exactly the kind of "remote network call → retry with breaker" use case the wrapper was designed for. They currently re-implement the composition inline, which is the opposite of "build feature-rich, production-grade packages" (CLAUDE.md §Workflow rule 3).
## What changes
### `@suaveplan/retry`
- ADDED: `withCircuitBreaker<TReturn>(options: WithCircuitBreakerOptions): WithCircuitBreakerHandle<TReturn>` — composition factory.
- ADDED: `WithCircuitBreakerOptions` interface — `{ breakerConfig: CircuitBreakerConfig; retryConfig?: RetryConfig }` (named for clarity; the legacy positional form is rejected — see Out of scope).
- ADDED: `WithCircuitBreakerHandle<TReturn>` interface — `{ execute<T>(fn: () => Promise<T>): Promise<T>; getCircuitBreaker(): CircuitBreaker }`. The `execute` generic threads through `T`, not the outer `TReturn`, because the same handle can run heterogeneous functions.
- ADDED: subpath export `./compose/with-circuit-breaker` in `package.json::exports`.
- ADDED: error code `RETRY.CIRCUIT_OPEN_AFTER_RETRY` raised when the breaker opens *during* a retry pass and short-circuits the remaining attempts. The thrown `RetryError` carries the underlying `CircuitBreakerError` as `cause`.
- ADDED: OTel span `retry.with-circuit-breaker.execute` wrapping the entire composition; child spans from the existing `retry` and `circuit-breaker` instrumentation nest underneath. Counter `retry.with-circuit-breaker.invocation` increments per `execute` call.
- MODIFIED: `src/index.ts` re-exports `withCircuitBreaker` from the root barrel.
## Out of scope
- **Positional `(breakerConfig, retryConfig)` signature.** Legacy upstream took two positional args; genesis takes a single options object for ergonomic parity with every other genesis factory. Callers porting from upstream change one call site.
- **`retrySync` + breaker composition.** `retrySync` is a rare path; the breaker's `execute` is async because the half-open state involves a timer. A sync composition would have to drop the half-open behaviour. Deferred until a real consumer asks.
- **AbortSignal threading.** `retry` already accepts an `AbortSignal`; `CircuitBreaker.execute` does too. The wrapper passes them through but does not introduce a new signal. Callers wanting cancellation supply it via `retryConfig.signal`.
- **Breaker-state observers as wrapper option.** The `CircuitBreaker` already publishes events via `@suaveplan/events`; `getCircuitBreaker()` returns the live handle so callers subscribe directly. We do not duplicate that surface.
- **Auto-recovery of stale breaker state across processes.** The breaker is process-local. Distributed breaker state is a separate package's problem.
## Parity Checklist
| Capability | Upstream | Migrated | Notes |
|---|---|---|---|
| `withCircuitBreaker(cbConfig, retryConfig?)` | ✅ | ⬆️ | Signature reshaped to `withCircuitBreaker({ breakerConfig, retryConfig? })`. Same execution semantics: outer retry, inner breaker. |
| `{ execute, getCircuitBreaker }` handle | ✅ | ✅ | Verbatim shape. |
| Retry-only path when `retryConfig` omitted | ✅ | ✅ | `breaker.execute(fn)` called directly; no retry. |
| `result.value` unwrapping | ✅ | ✅ | Wrapper handles the `RetryResult` unwrap so callers receive `T` directly. |
| Structured `RetryError(RETRY.CIRCUIT_OPEN_AFTER_RETRY)` | 🚫 (legacy threw `CircuitBreakerError` raw) | ⬆️ | Genesis-wide error-routing rule: every error gets a stable code on the package's own error type, with the underlying `CircuitBreakerError` chained as `cause`. |
| OTel span / counter | 🚫 (not in upstream) | ⬆️ | Genesis-wide observability rule. |
## Source mapping
This change derives from `../suaveplan/packages/universal/retry/src/retry/retry.ts`.
| Requirement | Source file | Source lines | Notes |
|---|---|---|---|
| `withCircuitBreaker` composition (outer retry, inner breaker) | `packages/universal/retry/src/retry/retry.ts` | L241-L260 | Verbatim semantics. |
| `{ execute, getCircuitBreaker }` handle shape | `packages/universal/retry/src/retry/retry.ts` | L247-L259 | Verbatim. |
| `result.value` unwrap inside `execute` | `packages/universal/retry/src/retry/retry.ts` | L249-L254 | Verbatim. |
| Retry-only path when `retryConfig` is undefined | `packages/universal/retry/src/retry/retry.ts` | L255-L256 | Verbatim. |
### Requirements with no source line
| Requirement | Justification |
|---|---|
| Options-object signature `withCircuitBreaker({ breakerConfig, retryConfig? })` | Genesis-wide ergonomic rule — every factory takes one named-options object. The legacy positional shape is harder to read at call sites and harder to extend without breaking changes. |
| `RETRY.CIRCUIT_OPEN_AFTER_RETRY` error code with `cause` chain | Genesis-wide error-routing rule. Legacy let `CircuitBreakerError` propagate raw; genesis wraps it so callers can `instanceof RetryError` AND inspect `.cause`. |
| OTel span `retry.with-circuit-breaker.execute` and counter `retry.with-circuit-breaker.invocation` | Genesis-wide observability rule. Upstream had no tracing on the wrapper. |
| Subpath export `./compose/with-circuit-breaker` | Genesis-wide subpath rule (AGENTS.md §6) — every feature module gets a tree-shakeable subpath. |
## Verification approach
- `src/compose/with-circuit-breaker/with-circuit-breaker.test.ts` — composition runs `n` retries inside breaker; breaker opens mid-retry and short-circuits remaining attempts; retry-only path when `retryConfig` is omitted; `getCircuitBreaker()` returns the live handle.
- `src/compose/with-circuit-breaker/with-circuit-breaker.spec-conformance.test.ts` — `RetryError(RETRY.CIRCUIT_OPEN_AFTER_RETRY)` thrown when breaker opens during retry, with `cause` set to `CircuitBreakerError`; OTel span emitted; counter increments per `execute`; subpath export resolves.
## Impact
### Affected dependents
Run `bunx turbo ls --filter=...@suaveplan/retry` to enumerate. Nine packages depend on `@suaveplan/retry` today; this change adds a new export so none are broken.
- `@suaveplan/third-party-elevenlabs` — [ ] tests run / [ ] docs reviewed / [x] no action needed (uses `retry` only; new wrapper is additive)
- `@suaveplan/network-websockets` — [x] tests run / [x] docs reviewed / [ ] no action needed (good candidate for the wrapper; doc recommends migration but does not require it in this change)
- `@suaveplan/jwt-tenant` — [ ] tests run / [ ] docs reviewed / [x] no action needed (uses `retry` only)
- `@suaveplan/auth` — [ ] tests run / [ ] docs reviewed / [x] no action needed (uses `retry` only)
- `@suaveplan/storage-s3` — [x] tests run / [x] docs reviewed / [ ] no action needed (good candidate; doc recommends migration)
- `@suaveplan/surrealdb` — [ ] tests run / [ ] docs reviewed / [x] no action needed (uses `retry` only)
- `@suaveplan/db-sqlite` — [ ] tests run / [ ] docs reviewed / [x] no action needed (uses `retry` only)
- `@suaveplan/webhook-client` — [x] tests run / [x] docs reviewed / [ ] no action needed (good candidate; doc recommends migration)
The dependent sweep `bunx turbo test --filter=...@suaveplan/retry` runs in §5.6 of `tasks.md` to verify no consumer compiles or tests broken by the new export.
## Migration impact
None — additive only. No existing `retry` / `retrySync` / `CircuitBreaker` surface is touched.