fix(sse): retry once on STREAM_EARLY_EOF for single-model requests (#3758) (#3817)

This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-14 01:39:05 -03:00
committed by GitHub
parent ef324cd00e
commit 826c533a59
4 changed files with 171 additions and 7 deletions

View File

@@ -9,6 +9,7 @@
### 🐛 Fixed
- **fix(intelligence): run pricing + models.dev sync from the live startup path** — like the Arena ELO sync (v3.8.24), the external **pricing sync** (`PRICING_SYNC_ENABLED`) and the **models.dev capability sync** (Settings → AI toggle) were only initialized from `server-init.ts`, which the Next standalone runtime never executes — and models.dev had no caller at all. Their toggles were inert in production. Both are now initialized from `instrumentation-node.ts` (self-gated, opt-in preserved, non-blocking, never fatal). (thanks @diegosouzapw)
- **fix(sse): retry once on an early stream close (`STREAM_EARLY_EOF`) for single-model requests** — flaky OpenAI-compatible upstreams (e.g. NVIDIA NIM with minimax-m3 / qwen3.5 / glm-5.1) intermittently send HTTP 200 then close the SSE with zero useful frames, surfacing as a 502 "Stream ended before producing useful content". Only Antigravity got an early-close retry; every other provider returned the 502 immediately on the non-combo single-model path. A bounded one-retry (early-close only — not readiness-timeout — and without marking the account unavailable) now generalizes it. (The separate qwen-web validation SSRF part of the same report was already fixed in v3.8.24, [#3767](https://github.com/diegosouzapw/OmniRoute/pull/3767).) ([#3758](https://github.com/diegosouzapw/OmniRoute/issues/3758) — thanks @Svatosalav)
- **fix(models): preserve eye-hidden models across auto-sync / import** — hiding models via the visibility (eye) toggle to keep only a combo's models was undone on every model import or auto-sync, which re-showed all of them. The sync re-import treated "hidden" identically to "deleted" and dropped both; a distinct `isDeleted` marker now separates the trash/delete path (still dropped on re-import, #3199) from the eye toggle (preserved as listed-but-hidden), and eye-hidden models are no longer re-aliased into the routable catalog on sync. ([#3782](https://github.com/diegosouzapw/OmniRoute/issues/3782) — thanks @xenstar)
- **fix(providers): correct the lmarena cookie hint (`session``arena-auth-prod-v1`)** — the lmarena credential hint asked for a cookie named `session`, but lmarena.ai's real auth cookie is `arena-auth-prod-v1`, so users who pasted only `session=…` hit validation failures. The credential name, placeholder and storage keys now use the correct name (the legacy `session` key is retained for back-compat with already-saved credentials). ([#3810](https://github.com/diegosouzapw/OmniRoute/issues/3810) — thanks @xspylol)

View File

@@ -53,6 +53,7 @@ import {
handleNoCredentials,
safeResolveProxy,
safeLogEvents,
shouldRetryStreamEarlyEof,
withSessionHeader,
} from "./chatHelpers";
import { connectionHasExtraKeys } from "@omniroute/open-sse/services/apiKeyRotator.ts";
@@ -98,10 +99,7 @@ import {
resolveCooldownAwareRetrySettings,
waitForCooldownAwareRetry,
} from "../services/cooldownAwareRetry";
import {
constrainConnectionsToQuota,
resolveQuotaKeyScope,
} from "../../lib/quota/quotaKey";
import { constrainConnectionsToQuota, resolveQuotaKeyScope } from "../../lib/quota/quotaKey";
registerCodexQuotaFetcher();
@@ -772,7 +770,14 @@ async function handleSingleModelChat(
});
}
const { provider: resolvedProvider, model, sourceFormat, targetFormat, extendedContext, apiFormat } = resolved;
const {
provider: resolvedProvider,
model,
sourceFormat,
targetFormat,
extendedContext,
apiFormat,
} = resolved;
// Prefer the combo target's providerId when available — the model string's
// provider prefix may differ from the credential provider ID (e.g. model
// "xiaomi/mimo-v2-flash" resolves to provider "xiaomi" but the combo target
@@ -877,6 +882,10 @@ async function handleSingleModelChat(
let requestRetryLastError = null;
let requestRetryLastStatus = null;
let requestRetryLastCooldownMs = 0;
// Bug #3758: per-request counter bounding the early-close (STREAM_EARLY_EOF)
// re-attempt to exactly one for the whole request. Declared outside both retry
// loops so it can never reset and loop.
let streamEarlyEofRetries = 0;
requestAttemptLoop: while (true) {
const excludedConnectionIds = new Set<string>();
@@ -1097,6 +1106,27 @@ async function handleSingleModelChat(
(result.errorType === "stream_timeout" || result.errorType === "stream_early_eof") &&
!isAntigravityStreamReadinessFailure
) {
// Bug #3758: flaky OpenAI-compatible upstreams (e.g. NVIDIA NIM) sometimes
// send HTTP 200 then close the SSE early with zero useful frames
// (STREAM_EARLY_EOF). That is a transient upstream glitch, not a bad key — so
// allow exactly ONE bounded same-connection re-attempt before surfacing the
// 502. Do NOT retry STREAM_READINESS_TIMEOUT (a slow-but-alive upstream;
// retrying would only double latency) and do NOT mark the account unavailable
// for the early close.
if (
shouldRetryStreamEarlyEof(result.errorCode, streamEarlyEofRetries) &&
!hasForcedConnection
) {
streamEarlyEofRetries += 1;
log.warn(
"STREAM",
`${provider}/${model} closed the stream early before useful content — retrying once (attempt ${streamEarlyEofRetries})`
);
// Plain re-attempt of the same request: no markAccountUnavailable, no
// excludedConnectionIds mutation (an early close is not a bad connection).
continue;
}
// Stream readiness timeout is an upstream stall after an HTTP response was received,
// not an account/quota failure. Do NOT mark the account unavailable here.
return result.response;
@@ -1357,8 +1387,11 @@ async function handleSingleModelChat(
model,
providerProfile,
{
persistUnavailableState:
!(isCombo && result.status === 429 && (failureKind === "rate_limit" || failureKind === "transient")),
persistUnavailableState: !(
isCombo &&
result.status === 429 &&
(failureKind === "rate_limit" || failureKind === "transient")
),
isCombo,
}
);

View File

@@ -582,6 +582,35 @@ export function handleNoCredentials(
);
}
/**
* Bug #3758 (Problem A): NVIDIA NIM (and other flaky OpenAI-compatible upstreams)
* intermittently send HTTP 200, then close the SSE early with zero useful frames.
* The readiness gate surfaces this as `STREAM_EARLY_EOF` / HTTP 502. On the
* single-model (non-combo) path that 502 used to be returned immediately for every
* provider except `antigravity`, so a transient upstream hang-up looked like a hard
* failure to the caller (e.g. the test-chat scenario).
*
* This decides whether a single-model request should re-attempt after an early
* close. It deliberately:
* - retries ONLY on `STREAM_EARLY_EOF` (the strong "upstream hung up after 200"
* signal) — NOT on `STREAM_READINESS_TIMEOUT` / `stream_timeout`, which is a
* slow-but-alive upstream where retrying would only double latency; and
* - is bounded to exactly ONE retry via the per-request `attempt` counter, so it
* can never loop (the second consecutive early close surfaces the 502).
*
* Pure function (no side effects): the caller performs a plain same-connection
* re-attempt and must NOT mark the account unavailable for an early close — it is a
* transient upstream glitch, not a bad key.
*/
export const STREAM_EARLY_EOF_MAX_RETRIES = 1;
export function shouldRetryStreamEarlyEof(
errorCode: string | null | undefined,
attempt: number
): boolean {
return errorCode === "STREAM_EARLY_EOF" && attempt < STREAM_EARLY_EOF_MAX_RETRIES;
}
/**
* Proxy-resolution failure policy. Default: fail-closed (rethrow) so a request
* with an assigned-but-unresolvable proxy never silently egresses on the real IP.

View File

@@ -0,0 +1,101 @@
import test from "node:test";
import assert from "node:assert/strict";
// Bug #3758 (Problem A): NVIDIA NIM models intermittently send HTTP 200 then close
// the SSE early with zero useful frames (flaky upstream early-close). The readiness
// gate correctly surfaces STREAM_EARLY_EOF / HTTP 502, but on the single-model path
// only `antigravity` got an early-close retry — every other OpenAI-compatible
// provider returned the 502 immediately with no retry.
//
// The fix generalizes the early-close handling: on `STREAM_EARLY_EOF` for ANY
// provider on the single-model path, allow exactly ONE bounded re-attempt before
// surfacing the 502. The decision lives in the pure helper
// `shouldRetryStreamEarlyEof(errorCode, attempt)` so it can be unit-tested in
// isolation and can NEVER loop.
const { shouldRetryStreamEarlyEof } = await import("../../src/sse/handlers/chatHelpers.ts");
test("shouldRetryStreamEarlyEof: retries once on the first STREAM_EARLY_EOF (attempt 0)", () => {
// Attempt 1 returned 200 then closed the SSE early → STREAM_EARLY_EOF.
// A non-antigravity provider (e.g. nvidia) must RETRY ONCE, not return the 502.
assert.equal(shouldRetryStreamEarlyEof("STREAM_EARLY_EOF", 0), true);
});
test("shouldRetryStreamEarlyEof: does NOT retry a second consecutive early-close (bounded)", () => {
// Two consecutive early-closes → the second one returns the 502 (exactly one retry, no loop).
assert.equal(shouldRetryStreamEarlyEof("STREAM_EARLY_EOF", 1), false);
assert.equal(shouldRetryStreamEarlyEof("STREAM_EARLY_EOF", 2), false);
assert.equal(shouldRetryStreamEarlyEof("STREAM_EARLY_EOF", 99), false);
});
test("shouldRetryStreamEarlyEof: does NOT retry a stream readiness TIMEOUT (preserves latency)", () => {
// A slow-but-alive upstream (STREAM_READINESS_TIMEOUT / stream_timeout) must NOT
// be retried — retrying would double latency for a request that is still warming up.
assert.equal(shouldRetryStreamEarlyEof("STREAM_READINESS_TIMEOUT", 0), false);
assert.equal(shouldRetryStreamEarlyEof("stream_timeout", 0), false);
});
test("shouldRetryStreamEarlyEof: ignores unrelated/empty error codes", () => {
assert.equal(shouldRetryStreamEarlyEof("", 0), false);
assert.equal(shouldRetryStreamEarlyEof(null, 0), false);
assert.equal(shouldRetryStreamEarlyEof(undefined, 0), false);
assert.equal(shouldRetryStreamEarlyEof("UPSTREAM_4XX", 0), false);
assert.equal(shouldRetryStreamEarlyEof("account_semaphore_capacity", 0), false);
});
// Behavioral end-to-end of the single-model path: stub executeChatWithBreaker so
// attempt 1 returns a STREAM_EARLY_EOF result and attempt 2 returns a successful
// stream. Assert the handler retries ONCE on the same connection, returns the
// successful stream, and never calls markAccountUnavailable for the early-close.
//
// Driving the full handler in a unit test is heavy (DB, auth, breaker), so this
// block mirrors the exact decision wiring the handler uses around the
// STREAM_EARLY_EOF branch, using the pure helper as the single source of truth.
test("single-model early-close: retries once then succeeds; double early-close surfaces 502; no markAccountUnavailable", () => {
// Simulated per-request early-EOF attempt counter, mirroring chat.ts.
function simulate(results: Array<{ errorCode: string; success?: boolean }>) {
let earlyEofAttempts = 0;
let markAccountUnavailableCalls = 0;
let i = 0;
while (true) {
const result = results[Math.min(i, results.length - 1)];
if (result.success) {
return { outcome: "success", earlyEofAttempts, markAccountUnavailableCalls };
}
// Non-antigravity early-close: never marks the account unavailable.
if (shouldRetryStreamEarlyEof(result.errorCode, earlyEofAttempts)) {
earlyEofAttempts += 1;
i += 1;
continue;
}
// Falls through to the immediate 502 return.
return { outcome: "502", earlyEofAttempts, markAccountUnavailableCalls };
}
}
// attempt 1: early-close → retry; attempt 2: success
const recovered = simulate([
{ errorCode: "STREAM_EARLY_EOF" },
{ errorCode: "STREAM_EARLY_EOF", success: true },
]);
assert.equal(recovered.outcome, "success");
assert.equal(recovered.earlyEofAttempts, 1, "exactly one retry before success");
assert.equal(
recovered.markAccountUnavailableCalls,
0,
"early-close must not mark account unavailable"
);
// attempt 1 + attempt 2 both early-close → surfaces the 502 (bounded, no loop)
const exhausted = simulate([
{ errorCode: "STREAM_EARLY_EOF" },
{ errorCode: "STREAM_EARLY_EOF" },
{ errorCode: "STREAM_EARLY_EOF" },
]);
assert.equal(exhausted.outcome, "502");
assert.equal(exhausted.earlyEofAttempts, 1, "only one retry attempted before surfacing 502");
assert.equal(exhausted.markAccountUnavailableCalls, 0);
});