From 43f729bcdf7695aaf7a6587670b3233b9ebb0ccd Mon Sep 17 00:00:00 2001 From: Sergio-LPA Date: Fri, 11 Sep 2026 18:42:14 +0100 Subject: [PATCH] fix(adapter-hermes-gateway): range-check retry timestamps and classify headerless 429s from the body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to the provider-quota backoff change, from review: - A finite but out-of-range retry value (e.g. `Retry-After: 9999999999999999`, or the same number in a terminal "retry after Ns" hint) built an Invalid Date whose toISOString() threw. On the terminal-event path that exception escaped result mapping and rejected execute() instead of returning a handled failure. Serialise through a range guard: the header degrades to "no retry-after", the terminal hint degrades to the 60s fallback cool-down. - The direct HTTP 429 path fed the synthetic "Hermes gateway HTTP 429" message to the quota detector, so every headerless 429 — including the gateway merely throttling requests — was promoted to provider_quota and routed through provider-quota recovery. The detector now only sees the response body (plain text, or error/error.message/message/detail/text); a headerless 429 without an upstream quota signature stays transient_upstream with no retryNotBefore. Tests cover the oversized header and hint, the body extractor shapes, and execute() on headerless/gateway-throttling/upstream-quota/retry-after 429s. Co-Authored-By: Claude Fable 5.1 --- .../hermes/src/gateway/server/execute.test.ts | 117 ++++++++++++++++++ .../hermes/src/gateway/server/execute.ts | 54 ++++++-- 2 files changed, 163 insertions(+), 8 deletions(-) diff --git a/packages/adapters/hermes/src/gateway/server/execute.test.ts b/packages/adapters/hermes/src/gateway/server/execute.test.ts index a22bb5aabb..e5348609b1 100644 --- a/packages/adapters/hermes/src/gateway/server/execute.test.ts +++ b/packages/adapters/hermes/src/gateway/server/execute.test.ts @@ -777,6 +777,27 @@ describe("mapFinalResultForTest", () => { expect(result.retryNotBefore).toBe(new Date(now + 60 * 1000).toISOString()); }); + it("degrades an out-of-range retry hint in a terminal failed event to the fallback cool-down instead of throwing", () => { + const now = new Date("2026-09-04T00:00:00Z").getTime(); + vi.spyOn(Date, "now").mockReturnValue(now); + const result = mapFinalResultForTest({ + terminal: { + runId: "run-quota-absurd", + status: "failed", + payload: { + status: "failed", + error: "Codex provider quota exhausted (429); retry after 9999999999999999s.", + }, + }, + outputChunks: [], + sessionKey: "session-key", + strategy: "issue", + }); + expect(result.errorCode).toBe("hermes_gateway_rate_limited"); + expect(result.errorFamily).toBe("provider_quota"); + expect(result.retryNotBefore).toBe(new Date(now + 60 * 1000).toISOString()); + }); + it("leaves unrelated terminal failures untouched", () => { const result = mapFinalResultForTest({ terminal: { @@ -815,6 +836,39 @@ describe("parseHermesRetryAfterHeader", () => { expect(parseHermesRetryAfterHeader(" ")).toBeNull(); expect(parseHermesRetryAfterHeader("not-a-date")).toBeNull(); }); + + it("returns null instead of throwing for finite values beyond the Date range", () => { + const now = new Date("2026-09-04T00:00:00Z").getTime(); + expect(() => parseHermesRetryAfterHeader("9999999999999999", now)).not.toThrow(); + expect(parseHermesRetryAfterHeader("9999999999999999", now)).toBeNull(); + expect(parseHermesRetryAfterHeader("99999999999999999999999", now)).toBeNull(); + }); +}); + +describe("extractQuotaSignalFromBody", () => { + const { extractQuotaSignalFromBody } = __providerQuotaInternals; + + it("reads the upstream message from the shapes the gateway uses", () => { + expect(extractQuotaSignalFromBody("HTTP 429: The usage limit has been reached")).toBe( + "HTTP 429: The usage limit has been reached", + ); + expect(extractQuotaSignalFromBody({ text: "quota exhausted (429)" })).toBe("quota exhausted (429)"); + expect(extractQuotaSignalFromBody({ error: "quota exhausted (429)" })).toBe("quota exhausted (429)"); + expect(extractQuotaSignalFromBody({ error: { message: "quota exhausted (429)" } })).toBe( + "quota exhausted (429)", + ); + expect(extractQuotaSignalFromBody({ message: "quota exhausted (429)" })).toBe("quota exhausted (429)"); + expect(extractQuotaSignalFromBody({ detail: "quota exhausted (429)" })).toBe("quota exhausted (429)"); + }); + + it("returns null for empty or unrelated bodies", () => { + expect(extractQuotaSignalFromBody(null)).toBeNull(); + expect(extractQuotaSignalFromBody(undefined)).toBeNull(); + expect(extractQuotaSignalFromBody("")).toBeNull(); + expect(extractQuotaSignalFromBody({})).toBeNull(); + expect(extractQuotaSignalFromBody([])).toBeNull(); + expect(extractQuotaSignalFromBody({ error: 42 })).toBeNull(); + }); }); describe("detectProviderQuotaExhaustion", () => { @@ -826,6 +880,17 @@ describe("detectProviderQuotaExhaustion", () => { expect(detectProviderQuotaExhaustion("random failure")).toBeNull(); }); + it("degrades an out-of-range retry hint to the fallback cool-down instead of throwing", () => { + const now = new Date("2026-09-04T00:00:00Z").getTime(); + const message = "Codex provider quota exhausted (429); retry after 9999999999999999s."; + expect(() => detectProviderQuotaExhaustion(message, now)).not.toThrow(); + expect(detectProviderQuotaExhaustion(message, now)).toEqual({ + errorCode: "hermes_gateway_rate_limited", + errorFamily: "provider_quota", + retryNotBefore: new Date(now + 60 * 1000).toISOString(), + }); + }); + it("extracts an explicit retry-after from the Codex quota message", () => { const now = new Date("2026-09-04T00:00:00Z").getTime(); const result = detectProviderQuotaExhaustion( @@ -839,3 +904,55 @@ describe("detectProviderQuotaExhaustion", () => { }); }); }); + +describe("execute: direct HTTP 429 classification", () => { + const now = new Date("2026-09-04T00:00:00Z").getTime(); + + function run429(init: { body?: string; headers?: Record }) { + vi.spyOn(Date, "now").mockReturnValue(now); + const fetchMock = vi.fn(async () => + new Response(init.body ?? "", { + status: 429, + headers: { "content-type": "application/json", ...(init.headers ?? {}) }, + }), + ); + vi.stubGlobal("fetch", fetchMock); + return execute(makeCtx({ apiBaseUrl: "http://127.0.0.1:8642", apiKey: "secret-key", timeoutSec: 5 })); + } + + it("keeps a headerless 429 with no upstream quota signal as transient gateway throttling", async () => { + const result = await run429({}); + expect(result.exitCode).toBe(1); + expect(result.errorCode).toBe("hermes_gateway_rate_limited"); + expect(result.errorFamily).toBe("transient_upstream"); + expect(result.retryNotBefore).toBeNull(); + }); + + it("keeps a headerless 429 whose body only describes gateway throttling as transient_upstream", async () => { + const result = await run429({ body: JSON.stringify({ error: "Too many requests to the gateway, slow down" }) }); + expect(result.errorFamily).toBe("transient_upstream"); + expect(result.retryNotBefore).toBeNull(); + }); + + it("promotes a headerless 429 to provider_quota when the body carries the upstream quota signature", async () => { + const result = await run429({ + body: JSON.stringify({ error: "Codex provider quota exhausted (429); retry after 120s. Credentials still valid." }), + }); + expect(result.errorCode).toBe("hermes_gateway_rate_limited"); + expect(result.errorFamily).toBe("provider_quota"); + expect(result.retryNotBefore).toBe(new Date(now + 120 * 1000).toISOString()); + }); + + it("honours the retry-after header on a 429 and leaves the family as transient_upstream", async () => { + const result = await run429({ headers: { "retry-after": "30" } }); + expect(result.errorFamily).toBe("transient_upstream"); + expect(result.retryNotBefore).toBe(new Date(now + 30 * 1000).toISOString()); + }); + + it("does not reject execute() when the retry-after header is beyond the Date range", async () => { + const result = await run429({ headers: { "retry-after": "9999999999999999" } }); + expect(result.exitCode).toBe(1); + expect(result.errorFamily).toBe("transient_upstream"); + expect(result.retryNotBefore).toBeNull(); + }); +}); diff --git a/packages/adapters/hermes/src/gateway/server/execute.ts b/packages/adapters/hermes/src/gateway/server/execute.ts index 03e81267e7..fdc02c2258 100644 --- a/packages/adapters/hermes/src/gateway/server/execute.ts +++ b/packages/adapters/hermes/src/gateway/server/execute.ts @@ -374,6 +374,18 @@ const RETRY_AFTER_HINT_RE = /retry[-_\s]?after[:\s]+(\d+)\s*s?\b/i; // can shorten it, but long enough to break the immediate hot-loop. const HERMES_GATEWAY_QUOTA_FALLBACK_SEC = 60; +// ECMAScript Dates only cover ±8.64e15 ms around the epoch. A finite but +// out-of-range value (e.g. "Retry-After: 9999999999999999") builds an Invalid +// Date whose toISOString() throws, and on the terminal-event path that +// exception would escape result mapping and reject execute() instead of +// yielding a handled failure. Serialise through this guard instead. +const MAX_DATE_MS = 8_640_000_000_000_000; + +function toSafeIsoTimestamp(ms: number): string | null { + if (!Number.isFinite(ms) || Math.abs(ms) > MAX_DATE_MS) return null; + return new Date(ms).toISOString(); +} + function parseHermesRetryAfterHeader(raw: string | null | undefined, now = Date.now()): string | null { if (raw === null || raw === undefined) return null; const value = String(raw).trim(); @@ -382,12 +394,30 @@ function parseHermesRetryAfterHeader(raw: string | null | undefined, now = Date. if (/^\d+$/.test(value)) { const seconds = Number.parseInt(value, 10); if (!Number.isFinite(seconds) || seconds < 0) return null; - return new Date(now + seconds * 1000).toISOString(); + return toSafeIsoTimestamp(now + seconds * 1000); } // HTTP-date form const parsed = Date.parse(value); if (!Number.isFinite(parsed)) return null; - return new Date(parsed).toISOString(); + return toSafeIsoTimestamp(parsed); +} + +// The direct HTTP 429 path only sees the synthetic "Hermes gateway HTTP 429" +// message, which says nothing about *who* is throttling. The upstream quota +// signal, when present, lives in the response body — either as a plain-text +// body (wrapped as { text }) or as an { error | message | detail } record. +function extractQuotaSignalFromBody(body: unknown): string | null { + if (typeof body === "string") return nonEmpty(body); + const record = asRecord(body); + if (!record) return null; + const nestedError = asRecord(record.error); + return ( + nonEmpty(record.error) ?? + nonEmpty(nestedError?.message) ?? + nonEmpty(record.message) ?? + nonEmpty(record.detail) ?? + nonEmpty(record.text) + ); } function detectProviderQuotaExhaustion( @@ -411,13 +441,18 @@ function detectProviderQuotaExhaustion( if (Number.isFinite(parsed) && parsed > 0) seconds = parsed; } if (seconds === null) seconds = HERMES_GATEWAY_QUOTA_FALLBACK_SEC; - const retryNotBefore = new Date(now + seconds * 1000).toISOString(); + // An absurd retry hint must not turn into an exception (or a null backoff + // that re-enables the hot-loop): degrade to the fallback cool-down. + const retryNotBefore = + toSafeIsoTimestamp(now + seconds * 1000) ?? + toSafeIsoTimestamp(now + HERMES_GATEWAY_QUOTA_FALLBACK_SEC * 1000); return { errorCode: "hermes_gateway_rate_limited", errorFamily: "provider_quota", retryNotBefore }; } export const __providerQuotaInternals = { parseHermesRetryAfterHeader, detectProviderQuotaExhaustion, + extractQuotaSignalFromBody, }; function fetchFailureMessage(err: unknown): string { @@ -840,16 +875,19 @@ function errorResult(err: unknown, redactText: TextRedactor = sanitizeSensitiveT const hermesError = err as HermesHttpError; const code = hermesError.code ?? "hermes_gateway_protocol_error"; const classified = hermesError.status ? classifyHttpError(hermesError.status) : null; - const rawMessage = err instanceof Error ? err.message : String(err); const errorMessage = code === "hermes_gateway_auth_failed" ? `${redactErrorMessage(err, redactText)}. Check adapterConfig.apiKey matches the Hermes API_SERVER_KEY for the running gateway.` : redactErrorMessage(err, redactText); - // On real HTTP 429s, upgrade the family to provider_quota (more specific than - // transient_upstream) when the message or body signals it, and synthesise - // retryNotBefore from the message if the header was missing. + // On real HTTP 429s without a retry-after header, upgrade the family to + // provider_quota (more specific than transient_upstream) only when the + // response *body* carries an upstream quota signature, and synthesise + // retryNotBefore from it. The synthetic "Hermes gateway HTTP 429" message + // must not be consulted: it matches the broad 429 matcher for every + // headerless 429, including the gateway merely throttling requests, which + // has to stay transient_upstream. const quotaOverride = hermesError.status === 429 && !hermesError.retryNotBefore - ? detectProviderQuotaExhaustion(rawMessage) + ? detectProviderQuotaExhaustion(extractQuotaSignalFromBody(hermesError.body)) : null; return { exitCode: 1,