Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
47 changes: 40 additions & 7 deletions src/sse/handlers/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
handleNoCredentials,
safeResolveProxy,
safeLogEvents,
shouldRetryStreamEarlyEof,
withSessionHeader,
} from "./chatHelpers";
import { connectionHasExtraKeys } from "@omniroute/open-sse/services/apiKeyRotator.ts";
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
);
Expand Down
29 changes: 29 additions & 0 deletions src/sse/handlers/chatHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
101 changes: 101 additions & 0 deletions tests/unit/stream-early-eof-retry-3758.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});