mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
feat(resilience): add useUpstream429BreakerHints toggle per #2100 follow-up
rdself flagged in #2116 that the per-failure-kind breaker cooldown lacks a user-controlled switch and should default conservatively for reverse-proxy / CLI-backed providers where forwarded 429 metadata is unreliable. This PR adds the toggle, the per-provider default policy, and surfaces it in the Resilience settings UI. Why --- A provider routed through cliproxyapi / lm-studio / vllm / etc may produce 429 metadata that the upstream layer fabricated. Letting that drive circuit-breaker cooldown duration is unsafe by default. Direct cloud providers (openai, anthropic, groq, cerebras, mistral, gemini, etc.) are the safe ones to trust. What ---- - New helper src/shared/utils/providerHints.ts: defaultUseUpstream429BreakerHints(providerId) returns false for the UPSTREAM_PROXY_PROVIDERS / SELF_HOSTED_CHAT_PROVIDER_IDS / isLocalProvider / isClaudeCodeCompatibleProvider sets; true for everything else. resolveUseUpstream429BreakerHints() picks user override when set, otherwise the per-provider default. - Schema: connectionCooldownProfileSchema gains useUpstream429BreakerHints: z.boolean().nullable().optional(). null is the explicit unset sentinel. - Settings layer: ConnectionCooldownProfileSettings adds useUpstream429BreakerHints?: boolean. Normalize preserves undefined (no toBoolean coercion) and treats null as "delete the key". The unset state survives all partial-merge round-trips. JSON serialization omits the key when undefined. - API route: PATCH /api/resilience detects useUpstream429BreakerHints transitions (stored override change in either oauth or apikey profile) and calls resetAllCircuitBreakers() so the registry stops serving cached options. - UI: ConnectionCooldownCard renderProfile gains a tri-state \<select\> with options Default (per provider) / Always on / Always off. Read-only rendering mirrors. Save handler converts undefined → null before PATCH so JSON.stringify does not drop the key. - Three wire-up call sites pass cooldownByKind + classifyError to getCircuitBreaker only when useHints === true: * open-sse/services/accountFallback.ts (configureProviderBreaker) * src/sse/handlers/chat.ts (~L516) * src/sse/handlers/chatHelpers.ts (~L151) - classify429.ts gains classify429FromError(err) adapter that maps the common HTTP-error shapes (axios-style err.response.status, low-level err.status, message fallback) to FailureKind so callers can use it directly as the breaker's classifyError option. Backwards compatibility ----------------------- Default behaviour is byte-identical when neither schema field nor user override is touched, since useUpstream429BreakerHints stays undefined and the helper returns the same per-provider policy that the breaker would have applied with cooldownByKind unset. Existing code paths that never construct a breaker with this option see no change. Tests ----- 39 tests pass across 4 unit suites: - tests/unit/provider-hints.test.ts (6 tests): per-provider defaults for cloud / cliproxyapi / self-hosted / claude-code prefix; user override truth-table in both directions including the v1 regression test where proxy-with-override-true must resolve to true. - tests/unit/resilience-settings-upstream429-breaker.test.ts (7 tests): defaults absent; explicit boolean stored; null sentinel deletes the key; key absent from JSON after delete; partial-merge omitting key leaves existing value; toBoolean coercion explicitly avoided; mixed-provider round-trip preserves disjoint per-profile settings. - tests/unit/classify429.test.ts (carried from #2116, still passes). - tests/unit/circuit-breaker-failure-kind.test.ts (carried, still passes). Iteration history (codex audits) -------------------------------- This plan went through 5 codex review rounds: - v1 → 5 concerns (default-vs-gate logic, naming, missed third call site, accountFallback layer separation, compat surface scope) - v2 → HIGH: normalization could swallow per-provider default; 4 minor - v3 → HIGH: binary BooleanField could not represent the unset state - v4 → HIGH: PATCH partial-merge semantics — omitted key ≠ unset - v5 → APPROVED with the null sentinel pattern Audit trail and grep output for getCircuitBreaker call sites are in the PR description. Open questions for the maintainer (see PR body) ----------------------------------------------- 1. Hard gate vs default for proxy/CLI providers (v5 ships default-off, user can override). 2. Cooldown values when toggle on are hardcoded for v1. 3. Reset granularity: resetAllCircuitBreakers() is coarse-grained but matches existing patterns; per-profile reset is a follow-up. 4. The 3 getCircuitBreaker-with-options call sites duplicate logic — a separate DRY refactor PR is suggested.
This commit is contained in:
Hernan Inverso
@@ -22,6 +22,12 @@ import {
|
||||
getCircuitBreaker,
|
||||
STATE,
|
||||
} from "../../src/shared/utils/circuitBreaker";
|
||||
import {
|
||||
classify429FromError,
|
||||
type FailureKind,
|
||||
} from "../../src/shared/utils/classify429";
|
||||
import { resolveUseUpstream429BreakerHints } from "../../src/shared/utils/providerHints";
|
||||
|
||||
|
||||
type ProviderProfile = {
|
||||
baseCooldownMs: number;
|
||||
@@ -532,9 +538,24 @@ function configureProviderBreaker(
|
||||
if (!provider) return null;
|
||||
|
||||
const resolvedProfile = { ...getProviderProfile(provider), ...(profile ?? {}) };
|
||||
// Issue #2100 follow-up: resolve useUpstream429BreakerHints from the
|
||||
// provider profile (stored override) or fall back to per-provider default.
|
||||
// Stored value type is `boolean | undefined` — never `null` after PATCH.
|
||||
const userValue = (resolvedProfile as { useUpstream429BreakerHints?: boolean })
|
||||
.useUpstream429BreakerHints;
|
||||
const useHints = resolveUseUpstream429BreakerHints(provider, userValue);
|
||||
return getCircuitBreaker(provider, {
|
||||
failureThreshold: resolvedProfile.failureThreshold ?? resolvedProfile.circuitBreakerThreshold,
|
||||
resetTimeout: resolvedProfile.resetTimeoutMs ?? resolvedProfile.circuitBreakerReset,
|
||||
...(useHints
|
||||
? {
|
||||
cooldownByKind: {
|
||||
rate_limit: 60_000,
|
||||
quota_exhausted: 3_600_000,
|
||||
} satisfies Partial<Record<FailureKind, number>>,
|
||||
classifyError: classify429FromError,
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,9 @@ type RequestQueueSettings = {
|
||||
type ConnectionCooldownProfileSettings = {
|
||||
baseCooldownMs: number;
|
||||
useUpstreamRetryHints: boolean;
|
||||
// Issue #2100 follow-up. Optional / undefined when unset; the per-provider
|
||||
// default in src/shared/utils/providerHints.ts resolves at runtime.
|
||||
useUpstream429BreakerHints?: boolean;
|
||||
maxBackoffSteps: number;
|
||||
};
|
||||
|
||||
@@ -360,6 +363,48 @@ function ConnectionCooldownCard({
|
||||
}))
|
||||
}
|
||||
/>
|
||||
<div className="flex flex-col gap-1">
|
||||
<label className="flex items-center justify-between gap-2 text-sm">
|
||||
<span className="text-text-muted">
|
||||
Use upstream 429 hints for breaker cooldown
|
||||
</span>
|
||||
<select
|
||||
className="rounded border border-border-default bg-surface-1 px-2 py-1 text-sm font-mono"
|
||||
value={
|
||||
current.useUpstream429BreakerHints === true
|
||||
? "on"
|
||||
: current.useUpstream429BreakerHints === false
|
||||
? "off"
|
||||
: "default"
|
||||
}
|
||||
onChange={(e) => {
|
||||
const v = e.target.value;
|
||||
const next: boolean | undefined =
|
||||
v === "on" ? true : v === "off" ? false : undefined;
|
||||
setDraft((prev) => {
|
||||
const profile = { ...prev[key] };
|
||||
if (next === undefined) {
|
||||
delete (profile as { useUpstream429BreakerHints?: boolean }).useUpstream429BreakerHints;
|
||||
} else {
|
||||
(profile as { useUpstream429BreakerHints?: boolean }).useUpstream429BreakerHints = next;
|
||||
}
|
||||
return { ...prev, [key]: profile };
|
||||
});
|
||||
}}
|
||||
>
|
||||
<option value="default">Default (per provider)</option>
|
||||
<option value="on">Always on</option>
|
||||
<option value="off">Always off</option>
|
||||
</select>
|
||||
</label>
|
||||
<p className="text-xs text-text-muted">
|
||||
Apply Retry-After / quota-exhausted signals from 429 responses to
|
||||
circuit-breaker cooldown duration. Default uses a per-provider
|
||||
policy: direct cloud providers default on; reverse-proxy /
|
||||
self-hosted / CLI-backed providers default off. Independent of
|
||||
"Use upstream retry hints".
|
||||
</p>
|
||||
</div>
|
||||
<NumberField
|
||||
label="Max backoff steps"
|
||||
value={current.maxBackoffSteps}
|
||||
@@ -381,6 +426,16 @@ function ConnectionCooldownCard({
|
||||
{current.useUpstreamRetryHints ? "Yes" : "No"}
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex items-center justify-between text-sm">
|
||||
<span className="text-text-muted">Use upstream 429 hints (breaker)</span>
|
||||
<span className="font-mono text-text-main">
|
||||
{current.useUpstream429BreakerHints === true
|
||||
? "Yes"
|
||||
: current.useUpstream429BreakerHints === false
|
||||
? "No"
|
||||
: "Default"}
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex items-center justify-between text-sm">
|
||||
<span className="text-text-muted">Max backoff steps</span>
|
||||
<span className="font-mono text-text-main">{current.maxBackoffSteps}</span>
|
||||
@@ -414,7 +469,26 @@ function ConnectionCooldownCard({
|
||||
setEditing(false);
|
||||
}}
|
||||
onSave={async () => {
|
||||
await onSave(draft);
|
||||
// Build PATCH-ready payload: convert undefined useUpstream429BreakerHints
|
||||
// to explicit null sentinel so the server treats it as unset (not as
|
||||
// partial-merge "leave unchanged"). JSON.stringify drops undefined keys.
|
||||
const payload = {
|
||||
oauth: {
|
||||
...draft.oauth,
|
||||
useUpstream429BreakerHints:
|
||||
draft.oauth.useUpstream429BreakerHints === undefined
|
||||
? (null as unknown as boolean | undefined)
|
||||
: draft.oauth.useUpstream429BreakerHints,
|
||||
},
|
||||
apikey: {
|
||||
...draft.apikey,
|
||||
useUpstream429BreakerHints:
|
||||
draft.apikey.useUpstream429BreakerHints === undefined
|
||||
? (null as unknown as boolean | undefined)
|
||||
: draft.apikey.useUpstream429BreakerHints,
|
||||
},
|
||||
};
|
||||
await onSave(payload as typeof draft);
|
||||
setEditing(false);
|
||||
}}
|
||||
/>
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from "@/lib/resilience/settings";
|
||||
import { updateResilienceSchema } from "@/shared/validation/schemas";
|
||||
import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
|
||||
import { resetAllCircuitBreakers } from "@/shared/utils/circuitBreaker";
|
||||
|
||||
type JsonRecord = Record<string, unknown>;
|
||||
|
||||
@@ -196,6 +197,20 @@ export async function PATCH(request) {
|
||||
});
|
||||
await syncRuntimeSettings(nextResilience);
|
||||
|
||||
// Issue #2100 follow-up: detect transitions in useUpstream429BreakerHints
|
||||
// and reset breakers so the registry stops serving cached options.
|
||||
// Compared on STORED override transition (boolean | undefined) so that
|
||||
// `null` (PATCH input) → undefined (stored) is correctly detected as
|
||||
// "unset request" when the previous stored value was a boolean.
|
||||
const breakerHintsChanged =
|
||||
currentResilience.connectionCooldown.oauth.useUpstream429BreakerHints !==
|
||||
nextResilience.connectionCooldown.oauth.useUpstream429BreakerHints ||
|
||||
currentResilience.connectionCooldown.apikey.useUpstream429BreakerHints !==
|
||||
nextResilience.connectionCooldown.apikey.useUpstream429BreakerHints;
|
||||
if (breakerHintsChanged) {
|
||||
resetAllCircuitBreakers();
|
||||
}
|
||||
|
||||
return NextResponse.json({
|
||||
ok: true,
|
||||
requestQueue: nextResilience.requestQueue,
|
||||
|
||||
@@ -14,6 +14,17 @@ export interface RequestQueueSettings {
|
||||
export interface ConnectionCooldownProfileSettings {
|
||||
baseCooldownMs: number;
|
||||
useUpstreamRetryHints: boolean;
|
||||
/**
|
||||
* Issue #2100 follow-up: opt-in toggle for upstream 429 hint trust at the
|
||||
* circuit-breaker cooldown layer (independent of `useUpstreamRetryHints`
|
||||
* which controls retry scheduling).
|
||||
*
|
||||
* Stored shape is intentionally optional / `boolean | undefined`: when
|
||||
* unset, the per-provider default from `providerHints.ts` applies.
|
||||
* Normalize/merge MUST preserve `undefined` — do not coerce via
|
||||
* `toBoolean(value, fallback)`.
|
||||
*/
|
||||
useUpstream429BreakerHints?: boolean;
|
||||
maxBackoffSteps: number;
|
||||
}
|
||||
|
||||
@@ -155,7 +166,29 @@ function normalizeConnectionCooldownProfile(
|
||||
fallback: ConnectionCooldownProfileSettings
|
||||
): ConnectionCooldownProfileSettings {
|
||||
const record = asRecord(next);
|
||||
return {
|
||||
// useUpstream429BreakerHints uses a 3-state input contract:
|
||||
// - boolean → user override, store as-is
|
||||
// - null → explicit unset sentinel, drop key so the per-provider
|
||||
// default in `providerHints.ts` resolves at runtime
|
||||
// - omitted → leave existing fallback value unchanged (partial-merge)
|
||||
// Never coerce via `toBoolean(value, fallback)` because that would
|
||||
// collapse the unset state.
|
||||
const hasHintsKey = Object.prototype.hasOwnProperty.call(
|
||||
record,
|
||||
"useUpstream429BreakerHints",
|
||||
);
|
||||
const rawHints = record.useUpstream429BreakerHints;
|
||||
let useUpstream429BreakerHints: boolean | undefined;
|
||||
if (!hasHintsKey) {
|
||||
useUpstream429BreakerHints = fallback.useUpstream429BreakerHints;
|
||||
} else if (rawHints === null) {
|
||||
useUpstream429BreakerHints = undefined;
|
||||
} else if (typeof rawHints === "boolean") {
|
||||
useUpstream429BreakerHints = rawHints;
|
||||
} else {
|
||||
useUpstream429BreakerHints = fallback.useUpstream429BreakerHints;
|
||||
}
|
||||
const out: ConnectionCooldownProfileSettings = {
|
||||
baseCooldownMs: toInteger(record.baseCooldownMs, fallback.baseCooldownMs, {
|
||||
min: 0,
|
||||
max: 24 * 60 * 60 * 1000,
|
||||
@@ -166,6 +199,11 @@ function normalizeConnectionCooldownProfile(
|
||||
max: 32,
|
||||
}),
|
||||
};
|
||||
// Only attach the key when defined — preserves omission across round-trips.
|
||||
if (useUpstream429BreakerHints !== undefined) {
|
||||
out.useUpstream429BreakerHints = useUpstream429BreakerHints;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function normalizeLegacyConnectionCooldownProfile(
|
||||
|
||||
@@ -162,3 +162,63 @@ export function retryAfterFromResponse(response: {
|
||||
}): number | null {
|
||||
return parseRetryAfter(getHeader(response.headers, "retry-after"));
|
||||
}
|
||||
|
||||
/**
|
||||
* Adapter that takes an error thrown by an HTTP client (fetch wrapper, axios,
|
||||
* upstream SDK, etc.) and produces a {@link FailureKind} suitable for the
|
||||
* `classifyError` option of the circuit breaker.
|
||||
*
|
||||
* Recognises the common error shapes:
|
||||
* - `err.status` + `err.headers` + `err.body` (low-level fetch wrapper)
|
||||
* - `err.response.status` + `err.response.headers` + `err.response.data` (axios-style)
|
||||
* - `err.message` (last-resort body for keyword scan)
|
||||
*
|
||||
* Returns `undefined` when the error doesn't carry enough information to
|
||||
* classify, so the breaker can decide what to do without a kind tag.
|
||||
*
|
||||
* Companion to issue #2100 follow-up.
|
||||
*/
|
||||
export function classify429FromError(err: unknown): FailureKind | undefined {
|
||||
if (err === null || typeof err !== "object") return undefined;
|
||||
const e = err as Record<string, unknown>;
|
||||
|
||||
let status: number | undefined;
|
||||
let headers: Record<string, string> | undefined;
|
||||
let body: unknown;
|
||||
|
||||
if (typeof e.status === "number") {
|
||||
status = e.status;
|
||||
}
|
||||
if (typeof e.statusCode === "number" && status === undefined) {
|
||||
status = e.statusCode;
|
||||
}
|
||||
|
||||
if (e.response && typeof e.response === "object") {
|
||||
const resp = e.response as Record<string, unknown>;
|
||||
if (typeof resp.status === "number" && status === undefined) {
|
||||
status = resp.status;
|
||||
}
|
||||
if (resp.headers && typeof resp.headers === "object") {
|
||||
headers = resp.headers as Record<string, string>;
|
||||
}
|
||||
if (resp.data !== undefined) {
|
||||
body = resp.data;
|
||||
} else if (typeof resp.body !== "undefined") {
|
||||
body = resp.body;
|
||||
}
|
||||
}
|
||||
|
||||
if (headers === undefined && e.headers && typeof e.headers === "object") {
|
||||
headers = e.headers as Record<string, string>;
|
||||
}
|
||||
if (body === undefined) {
|
||||
if (typeof e.body !== "undefined") {
|
||||
body = e.body;
|
||||
} else if (typeof e.message === "string") {
|
||||
body = e.message;
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof status !== "number") return undefined;
|
||||
return classify429({ status, headers, body });
|
||||
}
|
||||
|
||||
73
src/shared/utils/providerHints.ts
Normal file
73
src/shared/utils/providerHints.ts
Normal file
@@ -0,0 +1,73 @@
|
||||
/**
|
||||
* Per-provider default policy for upstream 429 hint trust.
|
||||
*
|
||||
* @see Issue #2100 follow-up — surface a user-overridable per-profile toggle
|
||||
* that decides whether the circuit breaker uses upstream 429 body / Retry-After
|
||||
* hints (`classify429`, `cooldownByKind`) to differentiate rate-limit from
|
||||
* quota-exhausted failure cooldowns.
|
||||
*
|
||||
* This helper returns the **default** answer for a given provider. The actual
|
||||
* runtime decision is the user override (if any) OR this default. See
|
||||
* `accountFallback.ts` / `chat.ts` / `chatHelpers.ts` for the resolution
|
||||
* call sites:
|
||||
*
|
||||
* ```ts
|
||||
* const userValue = providerProfile.useUpstream429BreakerHints; // boolean | undefined
|
||||
* const useHints = userValue !== undefined
|
||||
* ? userValue
|
||||
* : defaultUseUpstream429BreakerHints(provider);
|
||||
* ```
|
||||
*
|
||||
* Default policy: direct cloud providers default `true` because their 429
|
||||
* bodies and `Retry-After` headers are authoritative. Reverse-proxy /
|
||||
* self-hosted / CLI-backed providers default `false` because forwarded 429
|
||||
* metadata is often unreliable or fabricated by the proxy.
|
||||
*
|
||||
* @module shared/utils/providerHints
|
||||
*/
|
||||
|
||||
import {
|
||||
UPSTREAM_PROXY_PROVIDERS,
|
||||
SELF_HOSTED_CHAT_PROVIDER_IDS,
|
||||
isLocalProvider,
|
||||
isClaudeCodeCompatibleProvider,
|
||||
} from "../constants/providers";
|
||||
|
||||
/**
|
||||
* Conservative per-provider default for `useUpstream429BreakerHints`.
|
||||
*
|
||||
* Returns `false` for any provider whose 429 metadata may be forwarded by
|
||||
* an intermediary (proxy, self-hosted runtime, CLI wrapper). Returns `true`
|
||||
* for direct cloud providers where the upstream response is authoritative.
|
||||
*/
|
||||
export function defaultUseUpstream429BreakerHints(providerId: string): boolean {
|
||||
if (Object.prototype.hasOwnProperty.call(UPSTREAM_PROXY_PROVIDERS, providerId)) {
|
||||
return false;
|
||||
}
|
||||
if (isLocalProvider(providerId)) {
|
||||
return false;
|
||||
}
|
||||
if (SELF_HOSTED_CHAT_PROVIDER_IDS.has(providerId)) {
|
||||
return false;
|
||||
}
|
||||
if (isClaudeCodeCompatibleProvider(providerId)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the effective `useHints` decision: the user override wins if set,
|
||||
* otherwise fall back to the per-provider default.
|
||||
*
|
||||
* `undefined` means "not user-set" and triggers the default lookup.
|
||||
*/
|
||||
export function resolveUseUpstream429BreakerHints(
|
||||
providerId: string,
|
||||
userValue: boolean | undefined,
|
||||
): boolean {
|
||||
if (userValue !== undefined) {
|
||||
return userValue;
|
||||
}
|
||||
return defaultUseUpstream429BreakerHints(providerId);
|
||||
}
|
||||
@@ -775,6 +775,11 @@ const connectionCooldownProfileSchema = z
|
||||
.object({
|
||||
baseCooldownMs: z.number().int().min(0).optional(),
|
||||
useUpstreamRetryHints: z.boolean().optional(),
|
||||
// Issue #2100 follow-up: per-profile toggle for upstream 429 hint trust.
|
||||
// `null` is an explicit unset sentinel — PATCH handler deletes the key
|
||||
// from stored settings so the per-provider default resolves at runtime.
|
||||
// `undefined` (key omitted) means "leave existing value unchanged".
|
||||
useUpstream429BreakerHints: z.boolean().nullable().optional(),
|
||||
maxBackoffSteps: z.number().int().min(0).optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
@@ -45,6 +45,11 @@ import {
|
||||
} from "./chatHelpers";
|
||||
|
||||
// Pipeline integration — wired modules
|
||||
import {
|
||||
classify429FromError,
|
||||
type FailureKind,
|
||||
} from "@/shared/utils/classify429";
|
||||
import { resolveUseUpstream429BreakerHints } from "@/shared/utils/providerHints";
|
||||
import { getCircuitBreaker } from "../../shared/utils/circuitBreaker";
|
||||
import { markAccountExhaustedFrom429 } from "../../domain/quotaCache";
|
||||
import { RequestTelemetry, recordTelemetry } from "../../shared/utils/requestTelemetry";
|
||||
@@ -627,11 +632,25 @@ async function handleSingleModelChat(
|
||||
});
|
||||
if (gate) return gate;
|
||||
|
||||
// Issue #2100 follow-up: opt-in upstream 429 hint trust per provider.
|
||||
const useHints429 = resolveUseUpstream429BreakerHints(
|
||||
provider,
|
||||
(providerProfile as { useUpstream429BreakerHints?: boolean }).useUpstream429BreakerHints,
|
||||
);
|
||||
const breaker = getCircuitBreaker(provider, {
|
||||
failureThreshold: providerProfile.failureThreshold,
|
||||
resetTimeout: providerProfile.resetTimeoutMs,
|
||||
onStateChange: (name: string, from: string, to: string) =>
|
||||
log.info("CIRCUIT", `${name}: ${from} → ${to}`),
|
||||
...(useHints429
|
||||
? {
|
||||
cooldownByKind: {
|
||||
rate_limit: 60_000,
|
||||
quota_exhausted: 3_600_000,
|
||||
} satisfies Partial<Record<FailureKind, number>>,
|
||||
classifyError: classify429FromError,
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
|
||||
const userAgent = request?.headers?.get("user-agent") || "";
|
||||
|
||||
@@ -25,6 +25,12 @@ import {
|
||||
} from "@omniroute/open-sse/utils/proxyFetch.ts";
|
||||
import { resolveProxyForConnection } from "@/lib/localDb";
|
||||
import { CircuitBreakerOpenError, getCircuitBreaker } from "../../shared/utils/circuitBreaker";
|
||||
import {
|
||||
classify429FromError,
|
||||
type FailureKind,
|
||||
} from "../../shared/utils/classify429";
|
||||
import { resolveUseUpstream429BreakerHints } from "../../shared/utils/providerHints";
|
||||
|
||||
import { logProxyEvent } from "../../lib/proxyLogger";
|
||||
import { logTranslationEvent } from "../../lib/translatorEvents";
|
||||
import { getRuntimeProviderProfile } from "@omniroute/open-sse/services/accountFallback.ts";
|
||||
@@ -238,11 +244,25 @@ export async function checkPipelineGates(
|
||||
) {
|
||||
const bypassReason = options.bypassReason || "pipeline override";
|
||||
const providerProfile = options.providerProfile ?? (await getRuntimeProviderProfile(provider));
|
||||
// Issue #2100 follow-up: opt-in upstream 429 hint trust per provider.
|
||||
const useHints429 = resolveUseUpstream429BreakerHints(
|
||||
provider,
|
||||
(providerProfile as { useUpstream429BreakerHints?: boolean }).useUpstream429BreakerHints,
|
||||
);
|
||||
const breaker = getCircuitBreaker(provider, {
|
||||
failureThreshold: providerProfile.failureThreshold ?? providerProfile.circuitBreakerThreshold,
|
||||
resetTimeout: providerProfile.resetTimeoutMs ?? providerProfile.circuitBreakerReset,
|
||||
onStateChange: (name: string, from: string, to: string) =>
|
||||
log.info("CIRCUIT", `${name}: ${from} → ${to}`),
|
||||
...(useHints429
|
||||
? {
|
||||
cooldownByKind: {
|
||||
rate_limit: 60_000,
|
||||
quota_exhausted: 3_600_000,
|
||||
} satisfies Partial<Record<FailureKind, number>>,
|
||||
classifyError: classify429FromError,
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
if (options.ignoreCircuitBreaker && !breaker.canExecute()) {
|
||||
log.info("CIRCUIT", `Bypassing OPEN circuit breaker for ${provider} (${bypassReason})`);
|
||||
|
||||
58
tests/unit/provider-hints.test.ts
Normal file
58
tests/unit/provider-hints.test.ts
Normal file
@@ -0,0 +1,58 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
defaultUseUpstream429BreakerHints,
|
||||
resolveUseUpstream429BreakerHints,
|
||||
} from "../../src/shared/utils/providerHints.ts";
|
||||
|
||||
test("defaultUseUpstream429BreakerHints: direct cloud providers default true", () => {
|
||||
for (const id of ["openai", "anthropic", "groq", "cerebras", "mistral", "google"]) {
|
||||
assert.equal(
|
||||
defaultUseUpstream429BreakerHints(id),
|
||||
true,
|
||||
`expected true for ${id}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("defaultUseUpstream429BreakerHints: cliproxyapi defaults false", () => {
|
||||
assert.equal(defaultUseUpstream429BreakerHints("cliproxyapi"), false);
|
||||
});
|
||||
|
||||
test("defaultUseUpstream429BreakerHints: self-hosted chat providers default false", () => {
|
||||
for (const id of ["lm-studio", "vllm", "lemonade", "llamafile", "triton", "xinference", "oobabooga"]) {
|
||||
assert.equal(
|
||||
defaultUseUpstream429BreakerHints(id),
|
||||
false,
|
||||
`expected false for ${id}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("defaultUseUpstream429BreakerHints: claude-code-* prefix defaults false", () => {
|
||||
for (const id of ["anthropic-compatible-cc-direct", "anthropic-compatible-cc-bedrock", "anthropic-compatible-cc-vertex"]) {
|
||||
assert.equal(
|
||||
defaultUseUpstream429BreakerHints(id),
|
||||
false,
|
||||
`expected false for ${id}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("resolveUseUpstream429BreakerHints: user override wins (both directions)", () => {
|
||||
// Cloud provider with user override OFF → false
|
||||
assert.equal(resolveUseUpstream429BreakerHints("openai", false), false);
|
||||
// Cloud provider with user override ON → true (no-op vs default)
|
||||
assert.equal(resolveUseUpstream429BreakerHints("openai", true), true);
|
||||
// Proxy provider with user override ON → true (user explicitly trusted it)
|
||||
assert.equal(resolveUseUpstream429BreakerHints("cliproxyapi", true), true);
|
||||
// Proxy provider with user override OFF → false (no-op vs default)
|
||||
assert.equal(resolveUseUpstream429BreakerHints("cliproxyapi", false), false);
|
||||
});
|
||||
|
||||
test("resolveUseUpstream429BreakerHints: undefined falls back to per-provider default", () => {
|
||||
// Critical: this is the v3 regression-test for the v1 default-vs-gate bug.
|
||||
assert.equal(resolveUseUpstream429BreakerHints("openai", undefined), true);
|
||||
assert.equal(resolveUseUpstream429BreakerHints("cliproxyapi", undefined), false);
|
||||
assert.equal(resolveUseUpstream429BreakerHints("lm-studio", undefined), false);
|
||||
});
|
||||
94
tests/unit/resilience-settings-upstream429-breaker.test.ts
Normal file
94
tests/unit/resilience-settings-upstream429-breaker.test.ts
Normal file
@@ -0,0 +1,94 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
DEFAULT_RESILIENCE_SETTINGS,
|
||||
mergeResilienceSettings,
|
||||
resolveResilienceSettings,
|
||||
type ResilienceSettings,
|
||||
} from "../../src/lib/resilience/settings.ts";
|
||||
|
||||
function cloneDefaults(): ResilienceSettings {
|
||||
// structuredClone is enough for the plain-object settings shape.
|
||||
return structuredClone(DEFAULT_RESILIENCE_SETTINGS);
|
||||
}
|
||||
|
||||
test("defaults: useUpstream429BreakerHints omitted (undefined) on both profiles", () => {
|
||||
const settings = cloneDefaults();
|
||||
assert.equal(settings.connectionCooldown.oauth.useUpstream429BreakerHints, undefined);
|
||||
assert.equal(settings.connectionCooldown.apikey.useUpstream429BreakerHints, undefined);
|
||||
});
|
||||
|
||||
test("mergeResilienceSettings: explicit true is stored", () => {
|
||||
const current = cloneDefaults();
|
||||
const next = mergeResilienceSettings(current, {
|
||||
connectionCooldown: { oauth: { useUpstream429BreakerHints: true } },
|
||||
});
|
||||
assert.equal(next.connectionCooldown.oauth.useUpstream429BreakerHints, true);
|
||||
// apikey unchanged
|
||||
assert.equal(next.connectionCooldown.apikey.useUpstream429BreakerHints, undefined);
|
||||
});
|
||||
|
||||
test("mergeResilienceSettings: explicit false is stored", () => {
|
||||
const current = cloneDefaults();
|
||||
const next = mergeResilienceSettings(current, {
|
||||
connectionCooldown: { apikey: { useUpstream429BreakerHints: false } },
|
||||
});
|
||||
assert.equal(next.connectionCooldown.apikey.useUpstream429BreakerHints, false);
|
||||
});
|
||||
|
||||
test("mergeResilienceSettings: null sentinel deletes the field (back to undefined)", () => {
|
||||
// Start with explicit false on oauth
|
||||
const start = mergeResilienceSettings(cloneDefaults(), {
|
||||
connectionCooldown: { oauth: { useUpstream429BreakerHints: false } },
|
||||
});
|
||||
assert.equal(start.connectionCooldown.oauth.useUpstream429BreakerHints, false);
|
||||
// PATCH with null should reset to undefined
|
||||
const next = mergeResilienceSettings(start, {
|
||||
connectionCooldown: {
|
||||
oauth: { useUpstream429BreakerHints: null as unknown as boolean },
|
||||
},
|
||||
});
|
||||
assert.equal(next.connectionCooldown.oauth.useUpstream429BreakerHints, undefined);
|
||||
// Key should not appear in JSON
|
||||
const serialized = JSON.parse(JSON.stringify(next.connectionCooldown.oauth));
|
||||
assert.equal(
|
||||
"useUpstream429BreakerHints" in serialized,
|
||||
false,
|
||||
"key should be absent in serialized JSON",
|
||||
);
|
||||
});
|
||||
|
||||
test("mergeResilienceSettings: omitted key (partial-merge) leaves existing value", () => {
|
||||
// Start with explicit true on apikey
|
||||
const start = mergeResilienceSettings(cloneDefaults(), {
|
||||
connectionCooldown: { apikey: { useUpstream429BreakerHints: true } },
|
||||
});
|
||||
// PATCH oauth only — apikey must keep its value
|
||||
const next = mergeResilienceSettings(start, {
|
||||
connectionCooldown: { oauth: { baseCooldownMs: 5000 } },
|
||||
});
|
||||
assert.equal(next.connectionCooldown.apikey.useUpstream429BreakerHints, true);
|
||||
});
|
||||
|
||||
test("resolveResilienceSettings: omitted field in record stays undefined (no toBoolean coercion)", () => {
|
||||
const record = {
|
||||
connectionCooldown: {
|
||||
oauth: { baseCooldownMs: 1000, useUpstreamRetryHints: true, maxBackoffSteps: 5 },
|
||||
apikey: { baseCooldownMs: 2000, useUpstreamRetryHints: false, maxBackoffSteps: 3 },
|
||||
},
|
||||
};
|
||||
const resolved = resolveResilienceSettings(record as Parameters<typeof resolveResilienceSettings>[0]);
|
||||
assert.equal(resolved.connectionCooldown.oauth.useUpstream429BreakerHints, undefined);
|
||||
assert.equal(resolved.connectionCooldown.apikey.useUpstream429BreakerHints, undefined);
|
||||
});
|
||||
|
||||
test("mixed-provider round-trip: oauth=false + apikey=true survives merge", () => {
|
||||
const next = mergeResilienceSettings(cloneDefaults(), {
|
||||
connectionCooldown: {
|
||||
oauth: { useUpstream429BreakerHints: false },
|
||||
apikey: { useUpstream429BreakerHints: true },
|
||||
},
|
||||
});
|
||||
assert.equal(next.connectionCooldown.oauth.useUpstream429BreakerHints, false);
|
||||
assert.equal(next.connectionCooldown.apikey.useUpstream429BreakerHints, true);
|
||||
});
|
||||
Reference in New Issue
Block a user