fix(resilience): treat an unreachable-proxy ECONNREFUSED as a circuit-breaker event so combo fails over instead of hitting the 503 max-retry limit (#8376) (#8403)

changelog.d/fixes/8376-econnrefused-breaker.md

@@ -0,0 +1 @@
+- fix(resilience): treat an unreachable-proxy ECONNREFUSED as a circuit-breaker event so combo fails over instead of hitting the 503 max-retry limit (#8376)

open-sse/handlers/chatCore.ts

@@ -3214,17 +3214,29 @@ export async function handleChatCore({
     // via isLocalStreamLifecycleError so those map to 499 instead of falling
     // through to the 502 provider-failure default.
     const isRequestAborted = isLocalStreamLifecycleError(error);
+    // #8376: an unreachable upstream proxy (ECONNREFUSED/ECONNRESET/...) is tagged by
+    // proxyFetch.ts (tagProxyUnreachable) with `.errorCode = "proxy_unreachable"` before
+    // it reaches this catch. Classify it explicitly to 502 instead of falling through
+    // the generic `error.status` branch (a raw connect-refused error has no `.status` at
+    // all, so it used to collapse into an ordinary 502/504 the provider-breaker predicate
+    // can't tell apart from a per-model 5xx).
+    const isProxyUnreachableFailure =
+      !isRequestAborted && (error as { errorCode?: unknown })?.errorCode === "proxy_unreachable";
     const failureStatus = isRequestAborted
       ? 499
-      : error.name === "TimeoutError" || error.name === "BodyTimeoutError"
-        ? HTTP_STATUS.GATEWAY_TIMEOUT
-        : error.status && typeof error.status === "number"
-          ? error.status
-          : HTTP_STATUS.BAD_GATEWAY;
+      : isProxyUnreachableFailure
+        ? HTTP_STATUS.BAD_GATEWAY
+        : error.name === "TimeoutError" || error.name === "BodyTimeoutError"
+          ? HTTP_STATUS.GATEWAY_TIMEOUT
+          : error.status && typeof error.status === "number"
+            ? error.status
+            : HTTP_STATUS.BAD_GATEWAY;
     const failureMessage = isRequestAborted
       ? "Request aborted"
       : formatProviderError(error, provider, model, failureStatus);
-    const upstreamErrorCode = getUpstreamErrorIdentifier(error);
+    const upstreamErrorCode = isProxyUnreachableFailure
+      ? "proxy_unreachable"
+      : getUpstreamErrorIdentifier(error);
     // Tag our own deadline timeouts (fetch-start TimeoutError / body BodyTimeoutError,
     // both surfaced as a 504) as "upstream_timeout" so the cooldown layer can tell a
     // slow-but-not-failed request apart from a real provider 5xx. (Antigravity already

open-sse/services/combo.ts

@@ -2381,12 +2381,11 @@ export async function handleComboChat({
             return { ok: false, response: result };
           }
 
-          // Trigger shared provider circuit breaker for 5xx errors and connection failures.
-          // If the next target in the combo is on the same provider, don't mark the provider
-          // as failed — different models on the same provider may still succeed.
-          // G-02: when fallbackResult.skipProviderBreaker is set (embedded service supervisor
-          // outage signalled via X-Omni-Fallback-Hint: connection_cooldown) apply connection
-          // cooldown only — do NOT trip the whole-provider breaker.
+          // Trigger shared provider circuit breaker for 5xx errors and connection failures. If the
+          // next target is on the same provider, don't mark it failed (a different model may still
+          // succeed) — #8376: EXCEPT a proxy-unreachable failure, which poisons every model alike.
+          // G-02: when fallbackResult.skipProviderBreaker is set (embedded service supervisor outage
+          // signalled via X-Omni-Fallback-Hint: connection_cooldown) apply cooldown only — never trip.
           const nextTarget = orderedTargets[i + 1];
           const sameProviderNext =
             typeof nextTarget?.provider === "string" && nextTarget.provider === provider;
@@ -2398,6 +2397,7 @@ export async function handleComboChat({
               skipProviderBreaker: fallbackResult.skipProviderBreaker,
               requestScopedFailure,
               error: errorText,
+              isProxyUnreachable: structuredError?.code === "proxy_unreachable",
             })
           ) {
             recordProviderFailure(provider, log, targetWithConnection.connectionId, profile);

open-sse/services/combo/comboPredicates.ts

@@ -140,7 +140,12 @@ const PROVIDER_BREAKER_FAILURE_STATUSES = new Set([408, 500, 502, 503, 504]);
  *   this intentionally differs from `isProviderFailureCode` (accountFallback.ts), which
  *   INCLUDES 429 for connection-cooldown purposes and must not be changed here.
  * - When the next combo target is on the SAME provider, don't trip the provider breaker:
- *   a different model on that provider may still succeed.
+ *   a different model on that provider may still succeed. #8376: EXCEPT when the failure
+ *   itself is a transport-level "proxy unreachable" event (`isProxyUnreachable`) — a dead
+ *   upstream proxy poisons every account on that provider identically, so a different
+ *   model on the same provider will fail the exact same way. Without this override a
+ *   homogeneous same-provider combo pool never trips the breaker and instead burns every
+ *   attempt against the same dead proxy until it hits the 503 max-retry limit.
  * - G-02 / #2743: when the fallback result carries `skipProviderBreaker` (an embedded
  *   service supervisor outage signalled via `X-Omni-Fallback-Hint: connection_cooldown`)
  *   apply connection cooldown ONLY — never trip the whole-provider breaker.
@@ -161,11 +166,14 @@ export function shouldRecordProviderBreakerFailure(args: {
   skipProviderBreaker?: boolean;
   requestScopedFailure?: boolean;
   error?: unknown;
+  /** #8376: transport-level "proxy unreachable" signal — overrides the `sameProviderNext`
+   * exemption only; every other AND-term still gates the trip. */
+  isProxyUnreachable?: boolean;
 }): boolean {
   return (
     !args.isStreamReadinessFailure &&
     PROVIDER_BREAKER_FAILURE_STATUSES.has(args.status) &&
-    !args.sameProviderNext &&
+    (!args.sameProviderNext || args.isProxyUnreachable === true) &&
     !args.skipProviderBreaker &&
     !args.requestScopedFailure &&
     !isLocalStreamLifecycleError(args.error)

open-sse/utils/proxyFetch.ts

@@ -22,6 +22,45 @@ function isTlsFingerprintEnabled() {
   return process.env.ENABLE_TLS_FINGERPRINT === "true";
 }
 
+// #8376: transport-level connect-failure codes that mean "the configured upstream
+// proxy (or the target itself, for direct egress) is unreachable" — as opposed to an
+// ordinary upstream HTTP error. Read `.code` first (stable across undici/node
+// versions); native fetch wraps the real socket error in `.cause`, so fall back to
+// `.cause.code` when the top-level error is a bare "fetch failed" TypeError.
+const PROXY_UNREACHABLE_ERROR_CODES = new Set([
+  "ECONNREFUSED",
+  "ECONNRESET",
+  "ETIMEDOUT",
+  "ENETUNREACH",
+  "EHOSTUNREACH",
+  "EPIPE",
+  "UND_ERR_CONNECT_TIMEOUT",
+]);
+
+function isProxyUnreachableError(err: unknown): boolean {
+  if (!err || typeof err !== "object") return false;
+  const code = (err as { code?: unknown }).code;
+  if (typeof code === "string" && PROXY_UNREACHABLE_ERROR_CODES.has(code)) return true;
+  const causeCode = (err as { cause?: { code?: unknown } }).cause?.code;
+  return typeof causeCode === "string" && PROXY_UNREACHABLE_ERROR_CODES.has(causeCode);
+}
+
+/**
+ * #8376: tag a connect-failure error with a stable `.code`/`.errorCode` BEFORE it is
+ * rethrown, so chatCore's catch block (and, through the response body, the combo
+ * provider-breaker predicate) can classify it as "proxy unreachable" instead of
+ * falling through to a generic 502 that never trips the whole-provider breaker on a
+ * homogeneous same-provider combo pool. No-op when the error isn't connect-shaped.
+ */
+function tagProxyUnreachable<T>(err: T): T {
+  if (isProxyUnreachableError(err)) {
+    const e = err as Error & { code?: string; errorCode?: string };
+    e.code = e.code || "PROXY_UNREACHABLE";
+    e.errorCode = "proxy_unreachable";
+  }
+  return err;
+}
+
 /** Per-request tracking of whether TLS fingerprint was used */
 type TlsFingerprintStore = { used: boolean };
 const tlsFingerprintContext = new AsyncLocalStorage<TlsFingerprintStore>();
@@ -530,7 +569,7 @@ async function patchedFetch(
             if (dispatcherError instanceof Error) {
               (dispatcherError as Error & { proxyFetchDetail?: string }).proxyFetchDetail = detail;
             }
-            throw dispatcherError;
+            throw tagProxyUnreachable(dispatcherError);
           }
 
           // All attempts exhausted — try proxy fallback before native fetch
@@ -573,7 +612,7 @@ async function patchedFetch(
             if (nativeError instanceof Error) {
               (nativeError as Error & { proxyFetchDetail?: string }).proxyFetchDetail = detail;
             }
-            throw nativeError;
+            throw tagProxyUnreachable(nativeError);
           }
         }
         throw dispatcherError;

tests/unit/8376-econnrefused-breaker.test.ts

@@ -0,0 +1,96 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { shouldRecordProviderBreakerFailure } from "../../open-sse/services/combo/comboPredicates.ts";
+
+// #8376 — an unreachable upstream proxy (ECONNREFUSED) on a homogeneous same-provider
+// combo pool must still trip the whole-provider circuit breaker so combo routing fails
+// over to a different provider, instead of burning MAX_GLOBAL_ATTEMPTS against the same
+// dead proxy and returning 503 "Maximum combo retry limit".
+
+test("#8376: proxy-unreachable failure on a homogeneous same-provider combo trips the breaker via isProxyUnreachable override", () => {
+  const result = shouldRecordProviderBreakerFailure({
+    isStreamReadinessFailure: false,
+    status: 502,
+    sameProviderNext: true,
+    skipProviderBreaker: false,
+    requestScopedFailure: false,
+    error: "connect ECONNREFUSED 127.0.0.1:8787",
+    isProxyUnreachable: true,
+  });
+  assert.equal(result, true);
+});
+
+test("#8376 control: without the override, the SAME same-provider failure still does not trip (proves the override is additive, not a blanket bypass)", () => {
+  const result = shouldRecordProviderBreakerFailure({
+    isStreamReadinessFailure: false,
+    status: 502,
+    sameProviderNext: true,
+    skipProviderBreaker: false,
+    requestScopedFailure: false,
+    error: "connect ECONNREFUSED 127.0.0.1:8787",
+    isProxyUnreachable: false,
+  });
+  assert.equal(result, false);
+});
+
+test("#8376: the override never bypasses the other AND-terms — a stream-readiness failure still does not trip even when isProxyUnreachable is true", () => {
+  const result = shouldRecordProviderBreakerFailure({
+    isStreamReadinessFailure: true,
+    status: 502,
+    sameProviderNext: true,
+    skipProviderBreaker: false,
+    requestScopedFailure: false,
+    error: "connect ECONNREFUSED 127.0.0.1:8787",
+    isProxyUnreachable: true,
+  });
+  assert.equal(result, false);
+});
+
+test("#8376: the override never bypasses skipProviderBreaker (embedded-service connection-cooldown-only hint) even when isProxyUnreachable is true", () => {
+  const result = shouldRecordProviderBreakerFailure({
+    isStreamReadinessFailure: false,
+    status: 502,
+    sameProviderNext: true,
+    skipProviderBreaker: true,
+    requestScopedFailure: false,
+    error: "connect ECONNREFUSED 127.0.0.1:8787",
+    isProxyUnreachable: true,
+  });
+  assert.equal(result, false);
+});
+
+test("#8376: a genuine same-provider 5xx (not proxy-unreachable) still does NOT trip the breaker — no over-widening", () => {
+  const result = shouldRecordProviderBreakerFailure({
+    isStreamReadinessFailure: false,
+    status: 502,
+    sameProviderNext: true,
+    skipProviderBreaker: false,
+    requestScopedFailure: false,
+    error: "upstream returned 502",
+  });
+  assert.equal(result, false);
+});
+
+test("#8376: a normal 200-derived non-breaker-status failure is unaffected by isProxyUnreachable being true (status gate still applies)", () => {
+  const result = shouldRecordProviderBreakerFailure({
+    isStreamReadinessFailure: false,
+    status: 200,
+    sameProviderNext: true,
+    skipProviderBreaker: false,
+    requestScopedFailure: false,
+    isProxyUnreachable: true,
+  });
+  assert.equal(result, false);
+});
+
+test("#8376: a normal 429 (rate limit) is unaffected by isProxyUnreachable being true (429 intentionally excluded from breaker statuses)", () => {
+  const result = shouldRecordProviderBreakerFailure({
+    isStreamReadinessFailure: false,
+    status: 429,
+    sameProviderNext: true,
+    skipProviderBreaker: false,
+    requestScopedFailure: false,
+    isProxyUnreachable: true,
+  });
+  assert.equal(result, false);
+});