From 30c75e5a87fc0fad2a38da2da71552dad433fb24 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 26 Sep 2026 18:59:38 -0700 Subject: [PATCH] fix: cap Retry-After, reject duplicate indices, trim baseURL slash Resolve apiStyle before the empty-docs short-circuit so a bad style fails on every call. Refs CL-9449. --- README.md | 4 +++- src/request.ts | 15 ++++++++++++--- src/rerank.test.ts | 43 +++++++++++++++++++++++++++++++++++++++++++ src/rerank.ts | 30 ++++++++++++++++++++---------- 4 files changed, 78 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 53081e5..1450f67 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,9 @@ const registry = createRerankAdapterRegistry({ - A failed request throws `RerankRequestError`. Its `reason` says what went wrong, and `url` says where. Network errors, timeouts, rate limits and server errors are retried first under the retry policy. - A bad config throws arktype's `TraversalError` before any request is sent. -- With non-empty `docs`, a missing `model` for `cohere` or `voyage` throws `TraversalError`, and an `apiStyle` the registry does not know throws an `Error` that names it. +- An `apiStyle` the registry does not know throws an `Error` that names it. With non-empty `docs`, a missing `model` for `cohere` or `voyage` throws `TraversalError`. +- A reply with an out-of-range or repeated index throws `RerankRequestError` with a `protocol_mismatch` reason. +- `Retry-After` is honored up to 60 seconds. - A custom `parseResponse` should throw `ProtocolMismatchError` from `@intx/inference` so the failure becomes a `RerankRequestError`. Anything else it throws is rethrown as is. An empty `docs` list returns `[]` without a request. To keep search working when the reranker is down, catch the error and keep your original order. diff --git a/src/request.ts b/src/request.ts index 70713eb..17e5ec9 100644 --- a/src/request.ts +++ b/src/request.ts @@ -77,10 +77,19 @@ type Attempt = | { ok: true; body: unknown } | { ok: false; error: InferenceError }; +const MAX_RETRY_AFTER_MS = 60_000; + +const clamp = (ms: number): number => + Math.min(MAX_RETRY_AFTER_MS, Math.max(0, ms)); + /** * `Retry-After` in seconds or as an HTTP date; undefined when absent. * - * Both branches clamp at zero. A server may name an instant that has already + * Both branches clamp to `[0, MAX_RETRY_AFTER_MS]`. The ceiling keeps a hostile + * or far-future value from overflowing the timer, which fires immediately on + * anything past 2^31-1 ms. + * + * The floor exists because a server may name an instant that has already * passed, and `Retry-After: -5` is not unheard of; a negative delay would flow * into the retry policy as though it were a pacing hint. * @@ -93,10 +102,10 @@ export const extractRetryAfterMs: RetryAfterExtractor = (headers) => { if (header === undefined || header === "") return undefined; const seconds = Number(header); - if (Number.isFinite(seconds)) return Math.max(0, seconds * 1_000); + if (Number.isFinite(seconds)) return clamp(seconds * 1_000); const at = Date.parse(header); - return Number.isNaN(at) ? undefined : Math.max(0, at - Date.now()); + return Number.isNaN(at) ? undefined : clamp(at - Date.now()); }; /** Aborts on the caller's signal or the per-attempt timeout, whichever first. */ diff --git a/src/rerank.test.ts b/src/rerank.test.ts index c64ff6e..7c43af9 100644 --- a/src/rerank.test.ts +++ b/src/rerank.test.ts @@ -200,6 +200,24 @@ describe("errors", () => { }); }); + test("a duplicate index is a protocol mismatch", async () => { + setup(); + reply( + 200, + JSON.stringify([ + { index: 0, score: 0.5 }, + { index: 0, score: 0.9 }, + ]), + ); + + const error = await rerank({ baseURL: BASE_URL, apiStyle: "tei" }); + + expect(error).toBeInstanceOf(RerankRequestError); + expect(error).toMatchObject({ + reason: { message: "rerank response repeats index 0" }, + }); + }); + test.each(["cohere", "voyage"])( "%s without a model fails before posting", async (apiStyle) => { @@ -247,6 +265,22 @@ test("empty input returns nothing without a request", async () => { expect(harness.scenario.matchedRequests()).toHaveLength(0); }); +test("empty input still rejects an unknown apiStyle", async () => { + setup(); + const error = await rerank({ baseURL: BASE_URL, apiStyle: "nope" }, []); + expect(error).toHaveProperty( + "message", + expect.stringContaining('Unknown rerank API style "nope"'), + ); +}); + +test("a trailing slash on baseURL is trimmed", async () => { + setup(); + reply(200, "[]"); + await rerank({ baseURL: `${BASE_URL}/`, apiStyle: "tei" }); + expect(harness.scenario.matchedRequests()[0]?.url).toBe(`${BASE_URL}/rerank`); +}); + test("a registry is not mutable through the map it was built from", () => { const source: Record = { tei: rerankAdapterRegistry.resolve("tei"), @@ -279,6 +313,15 @@ describe("retry-after parsing", () => { expect(extractRetryAfterMs(new Headers({ "retry-after": "-5" }))).toBe(0); }); + test.each(["999999999", "Fri, 01 Jan 2099 00:00:00 GMT"])( + "caps %s at 60 seconds", + (value) => { + expect(extractRetryAfterMs(new Headers({ "retry-after": value }))).toBe( + 60_000, + ); + }, + ); + test("treats a blank header as absent", () => { expect( extractRetryAfterMs(new Headers({ "retry-after": " " })), diff --git a/src/rerank.ts b/src/rerank.ts index 9fa991d..d7d0e33 100644 --- a/src/rerank.ts +++ b/src/rerank.ts @@ -51,7 +51,10 @@ export type RerankOptions = { /** * Rerank a candidate set, returning `{id, score}` sorted descending. * - * Empty input short-circuits without a request. Every protocol addresses + * Empty input short-circuits without a request, after the config and + * `apiStyle` are validated. A trailing slash on `baseURL` is trimmed. A reply + * that repeats an index is a protocol mismatch rather than silently deduped: + * no supported provider does it, so it signals a broken server. Every protocol addresses * documents by their position in the request array and TEI's reply is * explicitly unordered, so results are mapped back through `docs[index]` rather * than by reply position. @@ -63,13 +66,14 @@ export async function rerankDocuments( options: RerankOptions = {}, ): Promise { const parsed = RerankConfigSchema.assert(config); + const adapter = (options.registry ?? rerankAdapterRegistry).resolve( + parsed.apiStyle, + ); if (docs.length === 0) return []; - const registry = options.registry ?? rerankAdapterRegistry; - const adapter = registry.resolve(parsed.apiStyle); const request = adapter.buildRequest(query, docs, { - baseURL: parsed.baseURL, + baseURL: parsed.baseURL.replace(/\/+$/, ""), model: parsed.model, apiKey: parsed.apiKey, }); @@ -96,18 +100,24 @@ export async function rerankDocuments( ); } + const mismatch = (message: string): RerankRequestError => + new RerankRequestError( + classifyProtocolMismatch(message, body), + request.url, + ); + const seen = new Set(); return scored .map(({ index, score }) => { const doc = docs[index]; if (doc === undefined) { - throw new RerankRequestError( - classifyProtocolMismatch( - `rerank response index ${index} out of bounds for ${docs.length} documents`, - body, - ), - request.url, + throw mismatch( + `rerank response index ${index} out of bounds for ${docs.length} documents`, ); } + if (seen.has(index)) { + throw mismatch(`rerank response repeats index ${index}`); + } + seen.add(index); return { id: doc.id, score }; }) .sort((a, b) => b.score - a.score);