From 560fb458004e495d8047c06b2d836065bcf12bf2 Mon Sep 17 00:00:00 2001 From: AayushMainali-Github Date: Sat, 25 Jul 2026 04:16:01 +0000 Subject: [PATCH] feat(http): add an opt-in retry policy Closes #35 --- .changeset/olive-eagles-shave.md | 9 + README.md | 58 ++++- ...006-disable-automatic-retries-in-v0.1.0.md | 5 +- docs/adr/0009-opt-in-retry-policy.md | 103 +++++++++ docs/adr/README.md | 22 +- docs/architecture.md | 16 +- src/client/client.ts | 22 +- src/http/index.ts | 8 + src/http/retry.ts | 214 ++++++++++++++++++ src/http/transport.ts | 63 +++++- src/index.ts | 2 + src/types/client-options.ts | 15 ++ src/types/index.ts | 1 + src/types/retry.ts | 64 ++++++ tests/integration/client.integration.test.ts | 68 +++++- tests/unit/client/retry.test.ts | 87 +++++++ tests/unit/http/helpers.ts | 3 + tests/unit/http/retry.test.ts | 188 +++++++++++++++ tests/unit/http/transport-retry.test.ts | 199 ++++++++++++++++ tests/unit/http/transport.test.ts | 1 + tests/unit/public-api.test.ts | 6 + 21 files changed, 1133 insertions(+), 21 deletions(-) create mode 100644 .changeset/olive-eagles-shave.md create mode 100644 docs/adr/0009-opt-in-retry-policy.md create mode 100644 src/http/retry.ts create mode 100644 src/types/retry.ts create mode 100644 tests/unit/client/retry.test.ts create mode 100644 tests/unit/http/retry.test.ts create mode 100644 tests/unit/http/transport-retry.test.ts diff --git a/.changeset/olive-eagles-shave.md b/.changeset/olive-eagles-shave.md new file mode 100644 index 0000000..7fe9fae --- /dev/null +++ b/.changeset/olive-eagles-shave.md @@ -0,0 +1,9 @@ +--- +"cnosdb-client": minor +--- + +Add an opt-in `retry` policy. Retries stay off unless configured, so the default remains one call, one request; enabling them retries `ping`, `query`, and `queryTable` on timeouts, connection failures, HTTP 429, and 5xx other than 501. Writes are retried only with `retryWrites`, and `execute` is never retried, because the client cannot tell whether a failed attempt took effect. + +Backoff doubles from `backoff.initialMs` up to `backoff.maxMs` with full jitter by default, a `Retry-After` header overrides the computed delay within that cap, and an `AbortSignal` ends the sequence immediately including mid-backoff. + +`timeoutMs` keeps its existing meaning as the budget for a single attempt rather than becoming a deadline across the sequence; `retry.maxElapsedMs` bounds the total. The reasoning, and the amendment to ADR-0006, are recorded in ADR-0009. diff --git a/README.md b/README.md index cf4ec12..a2d56c6 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ A small, dependency-free TypeScript client for the CnosDB HTTP API. - Deterministic Line Protocol serialization from plain JavaScript objects. - Typed errors for authentication, rate limiting, timeouts, network failures, and malformed responses. - Per-client and per-request timeouts, plus `AbortSignal` cancellation. +- Opt-in retries with jittered backoff, off by default and never retrying a write unless you say so. - Zero runtime dependencies; built on the platform `fetch`. - Ships ESM, CommonJS, and strict TypeScript declarations with source maps. - No import-time side effects, so unused exports tree-shake away. @@ -81,7 +82,8 @@ console.log(rows); | `password` | `string` | — | Basic-auth password; may be empty. | | `database` | `string` | `"public"` | Default database. | | `tenant` | `string` | `"cnosdb"` | Default tenant. | -| `timeoutMs` | `number` | `10000` | Default request timeout. | +| `timeoutMs` | `number` | `10000` | Timeout for a single attempt. | +| `retry` | `RetryOptions` | — | Retry policy; off unless supplied. | | `precision` | `"ms" \| "us" \| "ns"` | `"ms"` | Default write precision. | | `compression` | `"none" \| "gzip"` | `"none"` | Compression for write payloads. | | `headers` | `Record` | — | Extra headers sent with every request. | @@ -332,6 +334,57 @@ await pending; // rejects with CnosDBRequestError, code "ABORT_ERR" Override the timeout per request with `timeoutMs`. +## Retries + +Retries are off by default: one call, one request. A client that retries a +write silently can duplicate points and corrupt aggregates long before anyone +notices, so nothing is retried until you ask. + +```ts +const client = new CnosDBClient({ + url: "http://localhost:8902", + retry: { + attempts: 3, + backoff: { initialMs: 100, maxMs: 2_000, jitter: true }, + retryWrites: false, + }, +}); +``` + +| Option | Default | Description | +| ------------------- | ------- | ------------------------------------------------- | +| `attempts` | — | Total attempts including the first; `1` disables. | +| `backoff.initialMs` | `100` | First delay. | +| `backoff.maxMs` | `2000` | Cap on any single delay. | +| `backoff.jitter` | `true` | Spread the delay randomly across the interval. | +| `retryWrites` | `false` | Also retry writes. | +| `maxElapsedMs` | — | Wall-clock ceiling for the whole sequence. | + +**What gets retried.** `ping`, `query`, and `queryTable` are retried, because +they are the methods that return rows. Writes are retried only with +`retryWrites`. `execute` is never retried, whatever you configure: it exists for +statements whose point is their effect, and the client cannot tell whether a +failed attempt took hold before the connection dropped. If you send an `INSERT` +through `query` with retries on, it will be retried — send it through `execute`. + +**Which failures.** Timeouts, connection failures, HTTP 429, and 5xx other than 501. Rejected credentials, malformed SQL, an oversized payload, and a caller +abort are final, because the server will decide them the same way next time. + +**Timing.** `timeoutMs` is the budget for one attempt, not for the sequence, so +enabling retries does not shrink the time any single attempt gets. Use +`maxElapsedMs` for a bound on the total. Backoff doubles from `initialMs` up to +`maxMs`, and with jitter the actual wait is a random point below that, so a +fleet of clients that failed together does not come back in lockstep. A +`Retry-After` header wins over the computed delay, capped by `maxMs`. + +An `AbortSignal` ends the sequence immediately, including during a backoff. + +`retryWrites` is worth a moment's thought before enabling. CnosDB does not +deduplicate, so a retried write whose first attempt actually landed writes the +points twice. That is harmless when a repeat overwrites the same series and +timestamp, and quietly wrong otherwise. The reasoning is recorded in +[ADR-0009](docs/adr/0009-opt-in-retry-policy.md). + ## Error handling ```ts @@ -403,7 +456,8 @@ splitPoints(points: readonly Point[], options: SplitOptions): Generator ``` Exported types: `CnosDBClientOptions`, `RequestOptions`, `QueryOptions`, -`WriteOptions`, `PingResult`, `Point`, `PointFieldValue`, `TimePrecision`, +`WriteOptions`, `RetryOptions`, `BackoffOptions`, `SplitOptions`, `PingResult`, +`QueryTable`, `Point`, `PointFieldValue`, `TimePrecision`, `Compression`, `FetchLike`, and `CnosDBErrorOptions`. ## Compatibility diff --git a/docs/adr/0006-disable-automatic-retries-in-v0.1.0.md b/docs/adr/0006-disable-automatic-retries-in-v0.1.0.md index 45deb2b..6dcc561 100644 --- a/docs/adr/0006-disable-automatic-retries-in-v0.1.0.md +++ b/docs/adr/0006-disable-automatic-retries-in-v0.1.0.md @@ -1,6 +1,6 @@ # ADR-0006: Disable automatic retries in v0.1.0 -**Status:** Accepted +**Status:** Accepted, amended by [ADR-0009](0009-opt-in-retry-policy.md) **Date:** 2026-07-24 ## Context @@ -42,6 +42,9 @@ An opt-in retry policy is a candidate for a later version ([ROADMAP.md](../../ROADMAP.md)). If added, it must be off by default and must never retry a write unless the caller explicitly accepts duplication risk. +> [ADR-0009](0009-opt-in-retry-policy.md) added that policy under both +> conditions. The default is still one call, one request. + ## Consequences **Good:** diff --git a/docs/adr/0009-opt-in-retry-policy.md b/docs/adr/0009-opt-in-retry-policy.md new file mode 100644 index 0000000..ab75880 --- /dev/null +++ b/docs/adr/0009-opt-in-retry-policy.md @@ -0,0 +1,103 @@ +# ADR-0009: Opt-in retry policy, with a per-attempt timeout + +**Status:** Accepted +**Date:** 2026-07-25 +**Amends:** [ADR-0006](0006-disable-automatic-retries-in-v0.1.0.md) + +## Context + +ADR-0006 shipped v0.1.0 with no retries, because a client that silently retries +a write can duplicate points and corrupt aggregates without the caller ever +learning that it happened. That reasoning still holds, and the default is not +changing. + +What has changed is the evidence about the cost. Every caller running against a +real network ends up writing the same backoff loop, and the loop they write +cannot see what the client already knows: the `Retry-After` header on a 429, +whether a connection ever opened, and whether a timeout was the one the caller +configured or one the server imposed. Rebuilding that outside the client means +reconstructing information the client threw away. + +ADR-0006 left two conditions on any future policy: off by default, and never +retry a write unless the caller explicitly accepts the duplication risk. It also +noted the interface design as the real work, and named one question it did not +answer: what `timeoutMs` means once there is more than one attempt. + +## Decision + +Add an opt-in `retry` option on the client. Absent, behaviour is exactly what +v0.1.0 shipped: one call, one request. + +**`timeoutMs` stays the budget for a single attempt.** Each attempt gets the +full amount. A caller who needs a ceiling on the whole sequence sets +`retry.maxElapsedMs`, which stops the client from starting an attempt it knows +cannot finish inside the budget. + +The alternative — reinterpreting `timeoutMs` as a deadline across all attempts — +was rejected. It would silently redefine an existing option, so the same code +would behave differently after an upgrade, and the redefinition would be +invisible until the day a request was slow. It would also make each attempt's +budget shrink as attempts accumulate, so the last attempt, the one made under +the worst conditions, would be given the least time to succeed. That is exactly +backwards. A per-attempt budget also means "a single request must not hang for +more than X", which is what a caller writing `timeoutMs` is actually saying. + +**What is retried.** `ping`, `query`, and `queryTable` are retried. Writes are +retried only with `retry.retryWrites`, which is the explicit acceptance of +duplication risk that ADR-0006 required. `execute` is never retried, whatever +the configuration. + +ADR-0006 rejected "retry reads, never writes" on the grounds that `POST +/api/v1/sql` carries both `SELECT` and `INSERT` and the client cannot tell them +apart without parsing SQL. That is still true, and this ADR does not solve it by +parsing. It solves it by using the method the caller chose as the declaration: +`query` and `queryTable` return rows and are treated as reads, `execute` exists +for statements whose point is their effect and is treated as unsafe to repeat. +A caller who sends an `INSERT` through `query` has miscategorised it, and the +documentation says so plainly at both methods. + +**Which failures are retried.** Timeouts, network failures, 429, and 5xx other +than 501. Everything else — rejected credentials, malformed SQL, a payload too +large, a caller abort — is final, because the server will decide it the same way +next time. + +**Backoff.** Exponential from `initialMs`, capped at `maxMs`, with full jitter +on by default so a fleet of clients that failed together does not return in +lockstep. A `Retry-After` from the server overrides the computed delay, since +the server knows when it will be ready, but is still capped by `maxMs` so a +mistaken or hostile header cannot park the caller indefinitely. + +**Cancellation.** An `AbortSignal` ends the sequence immediately, including +during a backoff sleep. A caller who cancels does not wait out a delay first. + +## Consequences + +**Good:** + +- The duplicate-write failure mode ADR-0006 exists to prevent stays off by default and behind an explicit flag when enabled. +- Callers stop rewriting the same loop, and the loop they no longer write is the one that could not see `Retry-After`. +- `timeoutMs` keeps its meaning, so upgrading changes nothing for code that does not set `retry`. +- The retry decision lives next to the typed errors that classify failures, which is where the information already is. + +**Costs:** + +- `query` being retryable is a judgement about intent, not a fact the client can verify. A caller who sends mutating SQL through `query` and enables retries can duplicate work. Documented at the method, but documentation is weaker than a guarantee. +- More configuration surface, and more behaviour that must be explained before someone can predict what a failing call will do. +- A retried request occupies a connection longer than a failed one, so an overloaded server sees load fall more slowly than it would without retries, jitter notwithstanding. + +## Alternatives considered + +**Leave retries to the caller.** The status quo, and still workable. Rejected +because the argument for building it was never that the loop is hard to write; +it is that the caller cannot see what the client discarded. + +**Retry `execute` too.** Rejected. `execute` exists precisely for statements +that change something, and the client cannot tell whether a failed attempt took +effect before the connection dropped. + +**Infer idempotency by parsing SQL.** Rejected for the same reason ADR-0006 +rejected it: parsing SQL to guess intent is out of scope and would be wrong at +exactly the moments it mattered. + +**Retry on any 5xx including 501.** Rejected. 501 means the server will never +implement the endpoint, so waiting cannot change the answer. diff --git a/docs/adr/README.md b/docs/adr/README.md index 0f75a2a..e67c339 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -7,16 +7,17 @@ constraint and an accident. ## Index -| ADR | Title | Status | -| --------------------------------------------------- | ----------------------------------- | -------- | -| [0001](0001-use-cnosdb-http-api.md) | Use the CnosDB HTTP API | Accepted | -| [0002](0002-use-native-fetch.md) | Use native fetch | Accepted | -| [0003](0003-publish-esm-and-commonjs.md) | Publish ESM and CommonJS | Accepted | -| [0004](0004-use-changesets.md) | Use Changesets | Accepted | -| [0005](0005-use-issue-first-github-flow.md) | Use issue-first GitHub Flow | Accepted | -| [0006](0006-disable-automatic-retries-in-v0.1.0.md) | Disable automatic retries in v0.1.0 | Accepted | -| [0007](0007-use-unofficial-independent-branding.md) | Use unofficial independent branding | Accepted | -| [0008](0008-group-source-into-domain-folders.md) | Group source into domain folders | Accepted | +| ADR | Title | Status | +| --------------------------------------------------- | ----------------------------------- | -------------------------------------------------------- | +| [0001](0001-use-cnosdb-http-api.md) | Use the CnosDB HTTP API | Accepted | +| [0002](0002-use-native-fetch.md) | Use native fetch | Accepted | +| [0003](0003-publish-esm-and-commonjs.md) | Publish ESM and CommonJS | Accepted | +| [0004](0004-use-changesets.md) | Use Changesets | Accepted | +| [0005](0005-use-issue-first-github-flow.md) | Use issue-first GitHub Flow | Accepted | +| [0006](0006-disable-automatic-retries-in-v0.1.0.md) | Disable automatic retries in v0.1.0 | Accepted, amended by [0009](0009-opt-in-retry-policy.md) | +| [0007](0007-use-unofficial-independent-branding.md) | Use unofficial independent branding | Accepted | +| [0008](0008-group-source-into-domain-folders.md) | Group source into domain folders | Accepted | +| [0009](0009-opt-in-retry-policy.md) | Opt-in retry policy | Accepted | ## When to write one @@ -29,6 +30,7 @@ legal positioning. Do not write one for ordinary implementation choices. ```text Title Status Proposed | Accepted | Superseded by ADR-XXXX | Deprecated +Amends ADR-XXXX, when this narrows or extends an earlier decision Date Context The forces at play, and what makes this hard. Decision What we are doing, stated plainly. diff --git a/docs/architecture.md b/docs/architecture.md index 2a7c595..375fef1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -25,7 +25,7 @@ Within each folder: | `types/` | `common`, `client-options`, `request-options`, `point`, `ping` | | `errors/` | `base`, `http-status`, `transport`, `response`, `from-status` | | `line-protocol/` | `escape`, `field`, `timestamp`, `serialize` | -| `http/` | `url`, `auth`, `body`, `guards`, `transport` | +| `http/` | `url`, `auth`, `body`, `guards`, `retry`, `transport` | | `client/` | `defaults`, `validate`, `controls`, `client` | ### The barrel rule @@ -163,16 +163,22 @@ Each of these has an ADR in [adr/](adr/): - Dual ESM and CommonJS output ([0003](adr/0003-publish-esm-and-commonjs.md)). - Changesets for releases ([0004](adr/0004-use-changesets.md)). - Issue-first GitHub Flow ([0005](adr/0005-use-issue-first-github-flow.md)). -- No automatic retries ([0006](adr/0006-disable-automatic-retries-in-v0.1.0.md)). +- No automatic retries by default ([0006](adr/0006-disable-automatic-retries-in-v0.1.0.md), amended by [0009](adr/0009-opt-in-retry-policy.md)). - Unofficial, independent branding ([0007](adr/0007-use-unofficial-independent-branding.md)). +- Opt-in retry policy with a per-attempt timeout ([0009](adr/0009-opt-in-retry-policy.md)). -### Why no retries +### Why retries are off by default A retry is a correctness decision, not a convenience. `POST /api/v1/write` is not safely idempotent from the client's point of view: a timeout may mean the write never landed, or that it landed and the response was lost. Retrying -silently duplicates data in the second case. So v0.1.0 retries nothing and -instead exposes typed errors precise enough for the caller to decide. +silently duplicates data in the second case. + +So the client retries nothing unless asked. A caller who sets `retry` gets +reads retried; writes stay excluded until `retryWrites` says otherwise. The +decision of what counts as retryable lives in `http/retry.ts`, next to the +typed errors that classify each failure, and the transport consults it only for +requests the client itself has marked repeatable. ### Dependency policy diff --git a/src/client/client.ts b/src/client/client.ts index 2413b6c..449f595 100644 --- a/src/client/client.ts +++ b/src/client/client.ts @@ -3,6 +3,7 @@ import { createAuthorizationHeader, normalizeBaseUrl, normalizeHeaders, + normalizeRetry, Transport, } from "../http/index.js"; import { serializePoints } from "../line-protocol/index.js"; @@ -60,6 +61,7 @@ export class CnosDBClient { readonly #tenant: string; readonly #precision: TimePrecision; readonly #compression: Compression; + readonly #retryWrites: boolean; constructor(options: CnosDBClientOptions) { // Runtime guards throughout the constructor protect JavaScript callers, @@ -99,6 +101,9 @@ export class CnosDBClient { ); } + const retry = normalizeRetry(options.retry); + this.#retryWrites = retry?.retryWrites ?? false; + this.#database = database; this.#tenant = tenant; this.#precision = precision; @@ -111,6 +116,7 @@ export class CnosDBClient { ), timeoutMs, headers: normalizeHeaders(options.headers, "headers"), + ...(retry === undefined ? {} : { retry }), // Bind so that a supplied global `fetch` keeps its expected receiver. fetch: fetchImpl.bind(globalThis), }); @@ -126,6 +132,7 @@ export class CnosDBClient { method: "GET", path: PING_PATH, accept: "application/json", + retryable: true, ...requestControls(options), }); @@ -155,6 +162,11 @@ export class CnosDBClient { * * Statements that produce no rows (for example DDL) resolve to `undefined` * cast to `T`; prefer {@link CnosDBClient.execute} for those. + * + * This method is retried when a retry policy is configured, on the + * assumption that a statement sent through `query` is a read. Send anything + * that changes data through {@link CnosDBClient.execute} instead, which is + * never retried. */ async query( statement: string, @@ -168,6 +180,7 @@ export class CnosDBClient { body: sql, contentType: "text/plain; charset=utf-8", accept: "application/json", + retryable: true, ...requestControls(options), }); return result as T; @@ -197,6 +210,7 @@ export class CnosDBClient { body: sql, contentType: "text/plain; charset=utf-8", accept: "application/csv", + retryable: true, ...requestControls(options), }); @@ -210,6 +224,10 @@ export class CnosDBClient { /** * Executes a SQL statement whose result rows are not needed, such as DDL. * Any 2xx response counts as success and the body is discarded. + * + * Never retried, even with a retry policy configured: this method exists for + * statements that change something, and the client cannot tell whether a + * failed attempt took effect before the connection broke. */ async execute(statement: string, options: QueryOptions = {}): Promise { const sql = requireStatement(statement); @@ -227,7 +245,8 @@ export class CnosDBClient { /** * Writes a raw Line Protocol payload. * - * The payload is sent verbatim: it is neither validated, split, nor retried. + * The payload is sent verbatim: it is neither validated nor split, and it + * is retried only when the retry policy sets `retryWrites`. */ async writeLineProtocol( data: string, @@ -275,6 +294,7 @@ export class CnosDBClient { contentType: "text/plain; charset=utf-8", accept: "application/json", compression: this.#resolveCompression(options), + retryable: this.#retryWrites, ...requestControls(options), }); } diff --git a/src/http/index.ts b/src/http/index.ts index 923acd6..a047d00 100644 --- a/src/http/index.ts +++ b/src/http/index.ts @@ -3,5 +3,13 @@ export { createAuthorizationHeader } from "./auth.js"; export { normalizeHeaders, RESERVED_HEADERS } from "./headers.js"; export { gzipBody } from "./compress.js"; export { MAX_RESPONSE_BODY_CHARS, truncate } from "./body.js"; +export { + delayFor, + isRetryable, + normalizeRetry, + parseRetryAfter, + sleep, +} from "./retry.js"; +export type { ResolvedRetry } from "./retry.js"; export { Transport } from "./transport.js"; export type { TransportOptions, TransportRequest } from "./transport.js"; diff --git a/src/http/retry.ts b/src/http/retry.ts new file mode 100644 index 0000000..0b3251f --- /dev/null +++ b/src/http/retry.ts @@ -0,0 +1,214 @@ +import { + CnosDBNetworkError, + CnosDBRateLimitError, + CnosDBServerError, + CnosDBTimeoutError, +} from "../errors/index.js"; +import type { RetryOptions } from "../types/index.js"; + +/** A validated retry policy with every default resolved. @internal */ +export interface ResolvedRetry { + readonly attempts: number; + readonly initialMs: number; + readonly maxMs: number; + readonly jitter: boolean; + readonly retryWrites: boolean; + readonly maxElapsedMs: number | undefined; +} + +const DEFAULT_INITIAL_MS = 100; +const DEFAULT_MAX_MS = 2_000; + +/** + * Validates a caller-supplied policy and fills in defaults. Returns + * `undefined` when retries are not configured, which is the default. + * + * @internal + */ +export function normalizeRetry( + retry: RetryOptions | undefined, +): ResolvedRetry | undefined { + if (retry === undefined) return undefined; + if (typeof retry !== "object") { + throw new TypeError( + `CnosDB client option \`retry\` must be an object; received ${String(retry)}.`, + ); + } + + const attempts = requirePositiveInteger("retry.attempts", retry.attempts); + const backoff = retry.backoff ?? {}; + const initialMs = optionalPositiveInteger( + "retry.backoff.initialMs", + backoff.initialMs, + DEFAULT_INITIAL_MS, + ); + const maxMs = optionalPositiveInteger( + "retry.backoff.maxMs", + backoff.maxMs, + DEFAULT_MAX_MS, + ); + if (maxMs < initialMs) { + throw new TypeError( + `CnosDB client option \`retry.backoff.maxMs\` (${String(maxMs)}) must ` + + `not be smaller than \`retry.backoff.initialMs\` (${String(initialMs)}).`, + ); + } + + const maxElapsedMs = + retry.maxElapsedMs === undefined + ? undefined + : requirePositiveInteger("retry.maxElapsedMs", retry.maxElapsedMs); + + return { + attempts, + initialMs, + maxMs, + jitter: retry.backoff?.jitter ?? true, + retryWrites: retry.retryWrites ?? false, + maxElapsedMs, + }; +} + +/** + * Whether a failure could plausibly succeed if tried again. + * + * The list is deliberately short. Anything the server has decided about the + * request itself — bad credentials, a malformed statement, a payload that is + * too large — will be decided the same way next time, and a caller abort is + * an instruction, not a failure. + * + * @internal + */ +export function isRetryable(error: unknown): boolean { + if (error instanceof CnosDBTimeoutError) return true; + if (error instanceof CnosDBNetworkError) return true; + if (error instanceof CnosDBRateLimitError) return true; + if (error instanceof CnosDBServerError) { + // 501 means the server will never implement it, so waiting cannot help. + return error.status !== 501; + } + return false; +} + +/** + * Delay before attempt number `attempt`, where the first retry is attempt 2. + * + * A `Retry-After` from the server wins over the computed backoff, because the + * server knows when it will be ready and we do not. It is still capped by + * `maxMs` so a hostile or mistaken header cannot park the caller for an hour. + * + * @internal + */ +export function delayFor( + policy: ResolvedRetry, + attempt: number, + retryAfterMs: number | undefined, +): number { + const exponential = Math.min( + policy.maxMs, + policy.initialMs * 2 ** Math.max(0, attempt - 2), + ); + if (retryAfterMs !== undefined) { + return Math.min(policy.maxMs, retryAfterMs); + } + return policy.jitter ? Math.random() * exponential : exponential; +} + +/** + * Parses a `Retry-After` header, which is either a delay in seconds or an + * HTTP date. Returns `undefined` for anything else, including a date in the + * past, so that a nonsense value falls back to normal backoff. + * + * @internal + */ +export function parseRetryAfter( + header: string | null, + now: number = Date.now(), +): number | undefined { + if (header === null) return undefined; + const value = header.trim(); + if (value.length === 0) return undefined; + + if (/^\d+$/.test(value)) { + return Number(value) * 1_000; + } + + // An HTTP date always begins with an abbreviated weekday. Requiring that + // keeps `Date.parse`'s generosity from reading "-5" as a year. + if (!/^[A-Za-z]{3},/.test(value)) return undefined; + + const date = Date.parse(value); + if (Number.isNaN(date)) return undefined; + return Math.max(0, date - now); +} + +/** + * Retry-After values are carried out of band rather than on the error, so the + * error classes stay a description of what went wrong rather than a channel + * for transport bookkeeping. + */ +const retryAfterByError = new WeakMap(); + +/** @internal */ +export function rememberRetryAfter(error: unknown, delayMs: number): void { + if (typeof error === "object" && error !== null) { + retryAfterByError.set(error, delayMs); + } +} + +/** @internal */ +export function retryAfterFor(error: unknown): number | undefined { + if (typeof error === "object" && error !== null) { + return retryAfterByError.get(error); + } + return undefined; +} + +/** + * Waits, unless the caller aborts first — in which case the abort reason is + * thrown so the caller's cancellation is not swallowed by a sleeping client. + * + * @internal + */ +export async function sleep(ms: number, signal?: AbortSignal): Promise { + if (signal?.aborted === true) { + throw signal.reason ?? new Error("The caller aborted the request."); + } + await new Promise((resolve, reject) => { + const timer = setTimeout(() => { + signal?.removeEventListener("abort", onAbort); + resolve(); + }, ms); + // A pending backoff must never be the reason a process stays alive. + timer.unref(); + + function onAbort(): void { + clearTimeout(timer); + reject( + (signal?.reason as Error | undefined) ?? + new Error("The caller aborted the request."), + ); + } + + signal?.addEventListener("abort", onAbort, { once: true }); + }); +} + +function requirePositiveInteger(name: string, value: unknown): number { + if (typeof value !== "number" || !Number.isInteger(value) || value < 1) { + throw new TypeError( + `CnosDB client option \`${name}\` must be an integer of at least 1; ` + + `received ${String(value)}.`, + ); + } + return value; +} + +function optionalPositiveInteger( + name: string, + value: unknown, + fallback: number, +): number { + if (value === undefined) return fallback; + return requirePositiveInteger(name, value); +} diff --git a/src/http/transport.ts b/src/http/transport.ts index c9b51b7..f9c773d 100644 --- a/src/http/transport.ts +++ b/src/http/transport.ts @@ -9,6 +9,15 @@ import type { Compression, FetchLike } from "../types/index.js"; import { readBodySafely, truncate } from "./body.js"; import { gzipBody } from "./compress.js"; import { isAbortError, isCnosDBError, validateTimeout } from "./guards.js"; +import { + delayFor, + isRetryable, + parseRetryAfter, + rememberRetryAfter, + retryAfterFor, + sleep, + type ResolvedRetry, +} from "./retry.js"; /** @internal */ export interface TransportOptions { @@ -18,6 +27,8 @@ export interface TransportOptions { readonly fetch: FetchLike; /** Already normalized by {@link normalizeHeaders}. */ readonly headers?: Readonly>; + /** Already validated by {@link normalizeRetry}. Absent means no retries. */ + readonly retry?: ResolvedRetry; } /** @internal */ @@ -35,6 +46,12 @@ export interface TransportRequest { readonly headers?: Readonly>; /** Compression for this request's body. Defaults to `"none"`. */ readonly compression?: Compression; + /** + * Whether this request may be sent more than once. Only requests marked + * here are eligible for the configured retry policy; everything else is + * sent exactly once regardless of configuration. + */ + readonly retryable?: boolean; } /** @@ -51,6 +68,7 @@ export class Transport { readonly #timeoutMs: number; readonly #fetch: FetchLike; readonly #headers: Readonly>; + readonly #retry: ResolvedRetry | undefined; constructor(options: TransportOptions) { this.#baseUrl = options.baseUrl; @@ -58,13 +76,49 @@ export class Transport { this.#timeoutMs = options.timeoutMs; this.#fetch = options.fetch; this.#headers = options.headers ?? {}; + this.#retry = options.retry; } /** * Performs a request and returns the raw response together with the request * context needed for diagnostics. Non-2xx statuses are converted to errors. + * + * When a retry policy is configured and the request is marked retryable, a + * failure that could plausibly succeed later is tried again. The last + * failure is what propagates, so the caller sees the reason it gave up + * rather than the reason it first stumbled. */ async request(request: TransportRequest): Promise { + const policy = + request.retryable === true && this.#retry !== undefined + ? this.#retry + : undefined; + if (policy === undefined) { + return this.#attempt(request); + } + + const startedAt = Date.now(); + for (let attempt = 1; ; attempt += 1) { + try { + return await this.#attempt(request); + } catch (error) { + if (attempt >= policy.attempts || !isRetryable(error)) throw error; + + const delay = delayFor(policy, attempt + 1, retryAfterFor(error)); + if ( + policy.maxElapsedMs !== undefined && + Date.now() - startedAt + delay >= policy.maxElapsedMs + ) { + // Sleeping past the ceiling only to fail there wastes the caller's + // time, so stop while the budget still means something. + throw error; + } + await sleep(delay, request.signal); + } + } + } + + async #attempt(request: TransportRequest): Promise { const url = new URL(request.path, this.#baseUrl); for (const [key, value] of Object.entries(request.searchParams ?? {})) { url.searchParams.set(key, value); @@ -141,10 +195,17 @@ export class Transport { if (!response.ok) { const responseBody = await readBodySafely(response); - throw createErrorForStatus(response.status, { + const failure = createErrorForStatus(response.status, { ...context, ...(responseBody === undefined ? {} : { responseBody }), }); + const retryAfterMs = parseRetryAfter( + response.headers.get("retry-after"), + ); + if (retryAfterMs !== undefined) { + rememberRetryAfter(failure, retryAfterMs); + } + throw failure; } return response; diff --git a/src/index.ts b/src/index.ts index 73a6519..a38d199 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,6 +13,7 @@ export { } from "./errors/index.js"; export type { CnosDBErrorOptions } from "./errors/index.js"; export type { + BackoffOptions, CnosDBClientOptions, Compression, FetchLike, @@ -22,6 +23,7 @@ export type { QueryOptions, QueryTable, RequestOptions, + RetryOptions, TimePrecision, WriteOptions, } from "./types/index.js"; diff --git a/src/types/client-options.ts b/src/types/client-options.ts index 65144f3..b63b775 100644 --- a/src/types/client-options.ts +++ b/src/types/client-options.ts @@ -1,4 +1,5 @@ import type { Compression, FetchLike, TimePrecision } from "./common.js"; +import type { RetryOptions } from "./retry.js"; /** * Construction options for {@link CnosDBClient}. @@ -35,9 +36,23 @@ export interface CnosDBClientOptions { /** * Default request timeout in milliseconds. Defaults to `10_000`. + * + * This is the budget for a single attempt. When `retry` is configured, each + * attempt gets the full budget; bound the total with `retry.maxElapsedMs`. */ readonly timeoutMs?: number; + /** + * Retry policy. Unset by default, so a failed request fails once and the + * caller decides what to do. + * + * When set, `ping`, `query`, and `queryTable` are retried on failures that + * could plausibly succeed later. Writes are retried only with + * `retry.retryWrites`, and `execute` is never retried because it exists for + * statements that change something. + */ + readonly retry?: RetryOptions; + /** * Default write precision. Defaults to `"ms"`, which matches JavaScript * `Date` resolution. diff --git a/src/types/index.ts b/src/types/index.ts index f0d9a0e..0bb40a6 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -6,5 +6,6 @@ export type { WriteOptions, } from "./request-options.js"; export type { PingResult } from "./ping.js"; +export type { BackoffOptions, RetryOptions } from "./retry.js"; export type { QueryTable } from "./query-table.js"; export type { Point, PointFieldValue } from "./point.js"; diff --git a/src/types/retry.ts b/src/types/retry.ts new file mode 100644 index 0000000..0386cb8 --- /dev/null +++ b/src/types/retry.ts @@ -0,0 +1,64 @@ +/** + * Backoff schedule between retry attempts. + * + * The delay doubles from `initialMs` and is capped at `maxMs`. With `jitter` + * on, the actual wait is a random point between zero and that delay, so a + * fleet of clients that all failed at the same moment does not come back in + * lockstep and knock the server over again. + */ +export interface BackoffOptions { + /** First delay, in milliseconds. Defaults to `100`. */ + readonly initialMs?: number; + + /** Upper bound on any single delay, in milliseconds. Defaults to `2_000`. */ + readonly maxMs?: number; + + /** + * Spread the delay randomly across the interval. Defaults to `true`. + * + * Turn it off only when you need a deterministic schedule, such as in a + * test; synchronized retries are a real failure mode in production. + */ + readonly jitter?: boolean; +} + +/** + * Retry policy. Retries are off unless this is supplied, and nothing about + * the policy is inferred from the environment. + * + * Only failures that could plausibly succeed on a second attempt are retried: + * a connection that never completed, a timeout, HTTP 429, and 5xx other than + * 501. A rejected password, a malformed statement, or a caller abort are + * final, and retrying them would only waste the caller's time. + * + * `timeoutMs` remains the budget for a single attempt, not for the whole + * sequence. Use `maxElapsedMs` when you need a bound on the total. + */ +export interface RetryOptions { + /** + * Total attempts including the first, so `1` disables retrying. Must be an + * integer of at least 1. + */ + readonly attempts: number; + + /** Delay schedule between attempts. */ + readonly backoff?: BackoffOptions; + + /** + * Retry writes as well. Defaults to `false`. + * + * A write is not idempotent and CnosDB does not deduplicate, so a retried + * write whose first attempt actually landed will duplicate points. Only + * enable this when your points carry timestamps and tags that make a repeat + * write harmless, which for Line Protocol usually means an overwrite of the + * same series and time rather than a new row. + */ + readonly retryWrites?: boolean; + + /** + * Wall-clock ceiling for the whole sequence, in milliseconds. When set, a + * further attempt is not started once this much time has passed since the + * first one began. Unset by default, so `attempts` alone bounds the work. + */ + readonly maxElapsedMs?: number; +} diff --git a/tests/integration/client.integration.test.ts b/tests/integration/client.integration.test.ts index d36a3b8..b3d995b 100644 --- a/tests/integration/client.integration.test.ts +++ b/tests/integration/client.integration.test.ts @@ -6,7 +6,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { CnosDBClient } from "../../src/client/index.js"; import { CnosDBRequestError } from "../../src/errors/index.js"; import { splitPoints } from "../../src/line-protocol/index.js"; -import type { PingResult } from "../../src/types/index.js"; +import type { FetchLike, PingResult } from "../../src/types/index.js"; import { captureError } from "../helpers.js"; /** @@ -305,6 +305,72 @@ describe("splitPoints", () => { }); }); +describe("retries", () => { + /** + * Wraps the real fetch so the first `failures` calls fail the way a dropped + * connection does. Everything after that reaches the live server, which is + * the point: it proves a retried request still succeeds for real. + */ + function flakyFetch(failures: number): { + fetch: FetchLike; + calls: () => number; + } { + let seen = 0; + const fetch: FetchLike = (input, init) => { + seen += 1; + if (seen <= failures) { + return Promise.reject(new TypeError("fetch failed")); + } + return globalThis.fetch(input, init); + }; + return { fetch, calls: () => seen }; + } + + it("recovers from a transient failure and returns real rows", async () => { + const { fetch, calls } = flakyFetch(2); + const flaky = new CnosDBClient({ + url: baseUrl, + username: USERNAME, + password: PASSWORD, + timeoutMs: 30_000, + fetch, + retry: { attempts: 3, backoff: { initialMs: 10, maxMs: 50 } }, + }); + + const result = await flaky.ping(); + expect(result.status).toBe("healthy"); + expect(calls()).toBe(3); + }); + + it("gives up after the configured attempts", async () => { + const { fetch, calls } = flakyFetch(Number.POSITIVE_INFINITY); + const doomed = new CnosDBClient({ + url: baseUrl, + fetch, + retry: { attempts: 2, backoff: { initialMs: 10, maxMs: 20 } }, + }); + + await captureError(doomed.ping()); + expect(calls()).toBe(2); + }); + + it("does not retry a write unless the caller opts in", async () => { + const { fetch, calls } = flakyFetch(1); + const noWriteRetry = new CnosDBClient({ + url: baseUrl, + username: USERNAME, + password: PASSWORD, + fetch, + retry: { attempts: 3, backoff: { initialMs: 10, maxMs: 20 } }, + }); + + await captureError( + noWriteRetry.writeLineProtocol("retry_guard v=1", { database }), + ); + expect(calls()).toBe(1); + }); +}); + describe("queryTable", () => { const table = "table_shape"; diff --git a/tests/unit/client/retry.test.ts b/tests/unit/client/retry.test.ts new file mode 100644 index 0000000..25bdcd9 --- /dev/null +++ b/tests/unit/client/retry.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from "vitest"; + +import { CnosDBClient } from "../../../src/client/index.js"; +import { CnosDBServerError } from "../../../src/errors/index.js"; +import type { FetchLike, RetryOptions } from "../../../src/types/index.js"; +import { captureError } from "../../helpers.js"; + +const RETRY: RetryOptions = { + attempts: 3, + backoff: { initialMs: 1, maxMs: 2, jitter: false }, +}; + +/** Fails every call with 503, counting how many times it was asked. */ +function countingFetch(): { fetch: FetchLike; count: () => number } { + let calls = 0; + const fetch: FetchLike = () => { + calls += 1; + return Promise.resolve(new Response("{}", { status: 503 })); + }; + return { fetch, count: () => calls }; +} + +function makeClient( + fetch: FetchLike, + retry: RetryOptions | "none" = RETRY, +): CnosDBClient { + return new CnosDBClient({ + url: "http://localhost:8902", + fetch, + ...(retry === "none" ? {} : { retry }), + }); +} + +describe("client retry policy", () => { + it("is off by default", async () => { + const { fetch, count } = countingFetch(); + await captureError(makeClient(fetch, "none").ping()); + expect(count()).toBe(1); + }); + + it.each([ + ["ping", (c: CnosDBClient) => c.ping()], + ["query", (c: CnosDBClient) => c.query("SELECT 1")], + ["queryTable", (c: CnosDBClient) => c.queryTable("SELECT 1")], + ])("retries %s", async (_label, call) => { + const { fetch, count } = countingFetch(); + const error = await captureError(call(makeClient(fetch))); + expect(error).toBeInstanceOf(CnosDBServerError); + expect(count()).toBe(3); + }); + + it("never retries execute, which exists for statements with effects", async () => { + const { fetch, count } = countingFetch(); + await captureError(makeClient(fetch).execute("DROP DATABASE d")); + expect(count()).toBe(1); + }); + + it.each([ + ["writeLineProtocol", (c: CnosDBClient) => c.writeLineProtocol("m v=1")], + [ + "writePoints", + (c: CnosDBClient) => + c.writePoints({ measurement: "m", fields: { v: 1 } }), + ], + ])("does not retry %s without retryWrites", async (_label, call) => { + const { fetch, count } = countingFetch(); + await captureError(call(makeClient(fetch))); + expect(count()).toBe(1); + }); + + it("retries writes once the caller accepts the duplication risk", async () => { + const { fetch, count } = countingFetch(); + const client = makeClient(fetch, { ...RETRY, retryWrites: true }); + await captureError(client.writeLineProtocol("m v=1")); + expect(count()).toBe(3); + }); + + it("rejects an invalid policy at construction, not at first use", () => { + expect( + () => + new CnosDBClient({ + url: "http://localhost:8902", + retry: { attempts: 0 }, + }), + ).toThrow(/retry.attempts/); + }); +}); diff --git a/tests/unit/http/helpers.ts b/tests/unit/http/helpers.ts index 3109df7..d77b89c 100644 --- a/tests/unit/http/helpers.ts +++ b/tests/unit/http/helpers.ts @@ -1,4 +1,5 @@ import { normalizeBaseUrl } from "../../../src/http/url.js"; +import type { ResolvedRetry } from "../../../src/http/retry.js"; import { Transport } from "../../../src/http/transport.js"; import type { FetchLike } from "../../../src/types/index.js"; import { toUrl } from "../../helpers.js"; @@ -39,6 +40,7 @@ export function makeTransport( authorization?: string | undefined; timeoutMs?: number; headers?: Record; + retry?: ResolvedRetry; } = {}, ): Transport { return new Transport({ @@ -47,6 +49,7 @@ export function makeTransport( timeoutMs: overrides.timeoutMs ?? 10_000, fetch: fetchImpl, ...(overrides.headers === undefined ? {} : { headers: overrides.headers }), + ...(overrides.retry === undefined ? {} : { retry: overrides.retry }), }); } diff --git a/tests/unit/http/retry.test.ts b/tests/unit/http/retry.test.ts new file mode 100644 index 0000000..0e752b9 --- /dev/null +++ b/tests/unit/http/retry.test.ts @@ -0,0 +1,188 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + CnosDBAuthenticationError, + CnosDBNetworkError, + CnosDBRateLimitError, + CnosDBRequestError, + CnosDBServerError, + CnosDBTimeoutError, +} from "../../../src/errors/index.js"; +import { + delayFor, + isRetryable, + normalizeRetry, + parseRetryAfter, + sleep, + type ResolvedRetry, +} from "../../../src/http/retry.js"; +import { captureError } from "../../helpers.js"; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function policy(overrides: Partial = {}): ResolvedRetry { + return { + attempts: 3, + initialMs: 100, + maxMs: 2_000, + jitter: false, + retryWrites: false, + maxElapsedMs: undefined, + ...overrides, + }; +} + +describe("normalizeRetry", () => { + it("returns undefined when retries are not configured", () => { + expect(normalizeRetry(undefined)).toBeUndefined(); + }); + + it("fills in the documented defaults", () => { + expect(normalizeRetry({ attempts: 3 })).toStrictEqual({ + attempts: 3, + initialMs: 100, + maxMs: 2_000, + jitter: true, + retryWrites: false, + maxElapsedMs: undefined, + }); + }); + + it("keeps every supplied value", () => { + expect( + normalizeRetry({ + attempts: 5, + backoff: { initialMs: 25, maxMs: 400, jitter: false }, + retryWrites: true, + maxElapsedMs: 5_000, + }), + ).toStrictEqual({ + attempts: 5, + initialMs: 25, + maxMs: 400, + jitter: false, + retryWrites: true, + maxElapsedMs: 5_000, + }); + }); + + it("accepts one attempt, which simply disables retrying", () => { + expect(normalizeRetry({ attempts: 1 })?.attempts).toBe(1); + }); + + it.each([0, -1, 1.5, Number.NaN, "3", null])( + "rejects attempts of %s", + (attempts) => { + expect(() => + normalizeRetry({ attempts } as unknown as { attempts: number }), + ).toThrow(TypeError); + }, + ); + + it("rejects a maximum delay below the initial one", () => { + expect(() => + normalizeRetry({ attempts: 2, backoff: { initialMs: 500, maxMs: 100 } }), + ).toThrow(/must not be smaller/); + }); + + it("rejects a non-object policy", () => { + expect(() => normalizeRetry(3 as unknown as { attempts: number })).toThrow( + TypeError, + ); + }); +}); + +describe("isRetryable", () => { + it.each([ + ["a timeout", new CnosDBTimeoutError("timed out", { timeoutMs: 1 })], + ["a network failure", new CnosDBNetworkError("connection refused")], + ["a rate limit", new CnosDBRateLimitError("slow down", { status: 429 })], + ["a server error", new CnosDBServerError("boom", { status: 503 })], + ])("retries %s", (_label, error) => { + expect(isRetryable(error)).toBe(true); + }); + + it.each([ + [ + "rejected credentials", + new CnosDBAuthenticationError("nope", { status: 422 }), + ], + ["a rejected request", new CnosDBRequestError("bad sql", { status: 400 })], + ["an unimplemented endpoint", new CnosDBServerError("no", { status: 501 })], + ["something that is not an error", "boom"], + ])("does not retry %s", (_label, error) => { + expect(isRetryable(error)).toBe(false); + }); +}); + +describe("delayFor", () => { + it("doubles from the initial delay without jitter", () => { + const p = policy({ initialMs: 100, maxMs: 10_000 }); + expect(delayFor(p, 2, undefined)).toBe(100); + expect(delayFor(p, 3, undefined)).toBe(200); + expect(delayFor(p, 4, undefined)).toBe(400); + }); + + it("caps the delay at the configured maximum", () => { + const p = policy({ initialMs: 100, maxMs: 250 }); + expect(delayFor(p, 9, undefined)).toBe(250); + }); + + it("spreads the delay across the interval when jitter is on", () => { + vi.spyOn(Math, "random").mockReturnValue(0.25); + expect(delayFor(policy({ jitter: true }), 3, undefined)).toBe(50); + }); + + it("prefers the server's Retry-After over the computed backoff", () => { + expect(delayFor(policy(), 2, 750)).toBe(750); + }); + + it("caps Retry-After too, so a wild value cannot park the caller", () => { + expect(delayFor(policy({ maxMs: 2_000 }), 2, 3_600_000)).toBe(2_000); + }); +}); + +describe("parseRetryAfter", () => { + it("reads a delay in seconds", () => { + expect(parseRetryAfter("5")).toBe(5_000); + }); + + it("reads an HTTP date as a delay from now", () => { + const now = Date.parse("2026-01-01T00:00:00Z"); + expect(parseRetryAfter("Thu, 01 Jan 2026 00:00:30 GMT", now)).toBe(30_000); + }); + + it("treats a date in the past as no delay", () => { + const now = Date.parse("2026-01-01T00:01:00Z"); + expect(parseRetryAfter("Thu, 01 Jan 2026 00:00:00 GMT", now)).toBe(0); + }); + + it.each([null, "", " ", "soon", "-5", "5.5"])("ignores %s", (header) => { + expect(parseRetryAfter(header)).toBeUndefined(); + }); +}); + +describe("sleep", () => { + it("resolves after the delay", async () => { + const started = Date.now(); + await sleep(20); + expect(Date.now() - started).toBeGreaterThanOrEqual(15); + }); + + it("rejects immediately when the signal is already aborted", async () => { + const controller = new AbortController(); + controller.abort(new Error("gone")); + const error = await captureError(sleep(10_000, controller.signal)); + expect(error.message).toBe("gone"); + }); + + it("rejects with the abort reason when the signal fires mid-wait", async () => { + const controller = new AbortController(); + const promise = sleep(10_000, controller.signal); + controller.abort(new Error("caller changed its mind")); + const error = await captureError(promise); + expect(error.message).toBe("caller changed its mind"); + }); +}); diff --git a/tests/unit/http/transport-retry.test.ts b/tests/unit/http/transport-retry.test.ts new file mode 100644 index 0000000..dc63e5b --- /dev/null +++ b/tests/unit/http/transport-retry.test.ts @@ -0,0 +1,199 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + CnosDBNetworkError, + CnosDBRequestError, + CnosDBServerError, +} from "../../../src/errors/index.js"; +import type { ResolvedRetry } from "../../../src/http/retry.js"; +import type { FetchLike } from "../../../src/types/index.js"; +import { captureError } from "../../helpers.js"; +import { makeTransport } from "./helpers.js"; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const FAST: ResolvedRetry = { + attempts: 3, + initialMs: 1, + maxMs: 2, + jitter: false, + retryWrites: false, + maxElapsedMs: undefined, +}; + +/** A fetch that replays the given responses or errors, one per call. */ +function scriptedFetch(steps: (Response | Error)[]): { + fetch: FetchLike; + count: () => number; +} { + let index = 0; + const fetch: FetchLike = () => { + const step = steps[Math.min(index, steps.length - 1)]!; + index += 1; + return step instanceof Error + ? Promise.reject(step) + : Promise.resolve(step.clone()); + }; + return { fetch, count: () => index }; +} + +function ok(): Response { + return new Response("{}", { + status: 200, + headers: { "content-type": "application/json" }, + }); +} + +function failure( + status: number, + headers: Record = {}, +): Response { + return new Response("{}", { status, headers }); +} + +describe("Transport retries", () => { + it("sends once when no policy is configured", async () => { + const { fetch, count } = scriptedFetch([failure(503), ok()]); + const error = await captureError( + makeTransport(fetch).requestJson({ + method: "GET", + path: "api/v1/ping", + retryable: true, + }), + ); + expect(error).toBeInstanceOf(CnosDBServerError); + expect(count()).toBe(1); + }); + + it("sends once when the request is not marked retryable", async () => { + const { fetch, count } = scriptedFetch([failure(503), ok()]); + await captureError( + makeTransport(fetch, { retry: FAST }).requestJson({ + method: "POST", + path: "api/v1/sql", + }), + ); + expect(count()).toBe(1); + }); + + it("retries a server error and returns the eventual success", async () => { + const { fetch, count } = scriptedFetch([failure(503), failure(500), ok()]); + const result = await makeTransport(fetch, { retry: FAST }).requestJson({ + method: "GET", + path: "api/v1/ping", + retryable: true, + }); + expect(result).toStrictEqual({}); + expect(count()).toBe(3); + }); + + it("retries a network failure", async () => { + const { fetch, count } = scriptedFetch([ + new TypeError("fetch failed"), + ok(), + ]); + await makeTransport(fetch, { retry: FAST }).requestJson({ + method: "GET", + path: "api/v1/ping", + retryable: true, + }); + expect(count()).toBe(2); + }); + + it("gives up after the configured number of attempts", async () => { + const { fetch, count } = scriptedFetch([failure(503)]); + const error = await captureError( + makeTransport(fetch, { retry: FAST }).requestJson({ + method: "GET", + path: "api/v1/ping", + retryable: true, + }), + ); + expect(error).toBeInstanceOf(CnosDBServerError); + expect(count()).toBe(3); + }); + + it("does not retry a failure the server will repeat", async () => { + const { fetch, count } = scriptedFetch([failure(400)]); + const error = await captureError( + makeTransport(fetch, { retry: FAST }).requestJson({ + method: "GET", + path: "api/v1/ping", + retryable: true, + }), + ); + expect(error).toBeInstanceOf(CnosDBRequestError); + expect(count()).toBe(1); + }); + + it("waits as long as Retry-After asks", async () => { + const { fetch } = scriptedFetch([ + failure(429, { "retry-after": "1" }), + ok(), + ]); + const transport = makeTransport(fetch, { + retry: { ...FAST, maxMs: 60 }, + }); + const started = Date.now(); + await transport.requestJson({ + method: "GET", + path: "api/v1/ping", + retryable: true, + }); + // Capped at maxMs, but longer than the 1 ms backoff it would have used. + expect(Date.now() - started).toBeGreaterThanOrEqual(50); + }); + + it("stops retrying once the elapsed ceiling is in reach", async () => { + const { fetch, count } = scriptedFetch([failure(503)]); + const error = await captureError( + makeTransport(fetch, { + retry: { + ...FAST, + attempts: 10, + initialMs: 50, + maxMs: 50, + maxElapsedMs: 60, + }, + }).requestJson({ method: "GET", path: "api/v1/ping", retryable: true }), + ); + expect(error).toBeInstanceOf(CnosDBServerError); + expect(count()).toBeLessThanOrEqual(2); + }); + + it("abandons the sequence when the caller aborts during a backoff", async () => { + const { fetch, count } = scriptedFetch([failure(503)]); + const controller = new AbortController(); + const promise = makeTransport(fetch, { + retry: { ...FAST, initialMs: 5_000, maxMs: 5_000 }, + }).requestJson({ + method: "GET", + path: "api/v1/ping", + retryable: true, + signal: controller.signal, + }); + setTimeout(() => { + controller.abort(new Error("caller gave up")); + }, 10); + const error = await captureError(promise); + expect(error.message).toBe("caller gave up"); + expect(count()).toBe(1); + }); + + it("propagates the final failure, not the first", async () => { + const { fetch } = scriptedFetch([ + failure(503), + new TypeError("fetch failed"), + ]); + const error = await captureError( + makeTransport(fetch, { retry: { ...FAST, attempts: 2 } }).requestJson({ + method: "GET", + path: "api/v1/ping", + retryable: true, + }), + ); + expect(error).toBeInstanceOf(CnosDBNetworkError); + }); +}); diff --git a/tests/unit/http/transport.test.ts b/tests/unit/http/transport.test.ts index 2a4952d..e943180 100644 --- a/tests/unit/http/transport.test.ts +++ b/tests/unit/http/transport.test.ts @@ -203,6 +203,7 @@ describe("Transport status mapping", () => { const broken = { ok: false, status: 500, + headers: new Headers(), text: () => Promise.reject(new Error("stream failure")), } as unknown as Response; const { fetch } = recordingFetch(() => broken); diff --git a/tests/unit/public-api.test.ts b/tests/unit/public-api.test.ts index 3d59f72..d5817a7 100644 --- a/tests/unit/public-api.test.ts +++ b/tests/unit/public-api.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import * as publicApi from "../../src/index.js"; import type { + BackoffOptions, CnosDBClientOptions, CnosDBErrorOptions, Compression, @@ -12,6 +13,7 @@ import type { QueryOptions, QueryTable, RequestOptions, + RetryOptions, SplitOptions, TimePrecision, WriteOptions, @@ -77,10 +79,13 @@ describe("public API surface", () => { const precision: TimePrecision = "ms"; const compression: Compression = "gzip"; const fetchLike: FetchLike = () => Promise.resolve(new Response()); + const backoff: BackoffOptions = { initialMs: 10, maxMs: 20, jitter: false }; + const retry: RetryOptions = { attempts: 2, backoff, retryWrites: false }; const clientOptions: CnosDBClientOptions = { url: "http://localhost:8902", precision, compression, + retry, headers: { "x-api-key": "k" }, fetch: fetchLike, }; @@ -105,6 +110,7 @@ describe("public API surface", () => { expect(writeOptions.precision).toBe("ms"); expect(table.columns).toEqual(["a"]); expect(splitOptions.maxBytes).toBe(1_000); + expect(retry.attempts).toBe(2); expect(point.measurement).toBe("m"); expect(ping.status).toBe("healthy"); expect(errorOptions.cause).toBeInstanceOf(Error);