fix(adapter-hermes-gateway): range-check retry timestamps and classify headerless 429s from the body
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 <noreply@anthropic.com>
This commit is contained in:
parent
411f9535ed
commit
43f729bcdf
|
|
@ -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<string, string> }) {
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue