mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-23 07:32:20 +03:00
fix(relay): normalize bifrost errors, remap credential 404, fix analytics (#10797)
Merged — the 5 pre-existing tests that broke from this PR's intentional 404→401 remap (single-model no-credentials) are now realigned to the new contract. Thanks!
This commit is contained in:
committed by
GitHub
parent
3810a6c52c
commit
bc6129bcb2
@@ -10,7 +10,11 @@ import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors";
|
||||
import { handleChat } from "@/sse/handlers/chat";
|
||||
import { createInjectionGuard } from "@/middleware/promptInjectionGuard";
|
||||
import { getRelayTokenByHash, checkRateLimit, recordRelayUsage } from "@/lib/db/relayProxies";
|
||||
import { buildErrorBody } from "@omniroute/open-sse/utils/error";
|
||||
import {
|
||||
buildErrorBody,
|
||||
parseUpstreamError,
|
||||
sanitizeErrorMessage,
|
||||
} from "@omniroute/open-sse/utils/error";
|
||||
import {
|
||||
checkIpRateLimit,
|
||||
extractToken,
|
||||
@@ -29,6 +33,7 @@ import {
|
||||
import { getProviderPluginManifestEntryForModel } from "@omniroute/open-sse/config/providerPluginManifestRegistry.ts";
|
||||
import { getProviderPluginManifestHeader } from "@omniroute/open-sse/config/providerPluginManifestUrl.ts";
|
||||
import { finalizeReadableStream } from "./streamFinalizer";
|
||||
import { stripStaleEncodingHeaders } from "@omniroute/open-sse/utils/upstreamResponseHeaders.ts";
|
||||
import {
|
||||
clearBifrostFailure,
|
||||
getActiveBifrostCooldown,
|
||||
@@ -108,6 +113,32 @@ async function forwardToBifrost(
|
||||
headers.set("Content-Type", upstream.headers.get("Content-Type") ?? "application/json");
|
||||
}
|
||||
|
||||
// Issue #1: Bifrost (or the upstream behind it) may return plain text or HTML
|
||||
// on a non-OK status (e.g. 502 from a sidecar, "invalid character 'd'" style
|
||||
// proxy errors). Forwarding `upstream.body` raw leaks non-JSON into a client
|
||||
// that expects OpenAI-shaped JSON, producing client-side parse failures.
|
||||
// Normalize any non-OK response through parseUpstreamError + buildErrorBody so
|
||||
// the client always receives a valid JSON error. (Hard rule #12.)
|
||||
if (!upstream.ok) {
|
||||
const parsed = await parseUpstreamError(upstream, null);
|
||||
const errorBody = buildErrorBody(
|
||||
parsed.statusCode,
|
||||
sanitizeErrorMessage(parsed.message),
|
||||
parsed.responseBody
|
||||
);
|
||||
const errorHeaders = stripStaleEncodingHeaders(headers);
|
||||
errorHeaders.set("Content-Type", "application/json");
|
||||
if (parsed.retryAfterMs && parsed.retryAfterMs > 0) {
|
||||
errorHeaders.set("Retry-After", String(Math.ceil(parsed.retryAfterMs / 1000)));
|
||||
}
|
||||
clearTimeout(tid);
|
||||
recordUsage(token.id, request, startTime, clientIp, userAgent, "error", parsed.statusCode);
|
||||
return new Response(JSON.stringify(errorBody), {
|
||||
status: parsed.statusCode,
|
||||
headers: errorHeaders,
|
||||
});
|
||||
}
|
||||
|
||||
if (wantsStream && upstream.body) {
|
||||
const stream = finalizeReadableStream(upstream.body, (error) => {
|
||||
clearTimeout(tid);
|
||||
@@ -144,7 +175,8 @@ async function forwardToBifrost(
|
||||
startTime,
|
||||
clientIp,
|
||||
userAgent,
|
||||
upstream.status < 500 ? "success" : "error",
|
||||
// upstream.ok is guaranteed true here (the !upstream.ok branch above returns early).
|
||||
"success",
|
||||
upstream.status
|
||||
);
|
||||
|
||||
|
||||
@@ -1682,7 +1682,8 @@ async function handleSingleModelChat(
|
||||
model,
|
||||
lastError,
|
||||
lastStatus,
|
||||
candidateAliases
|
||||
candidateAliases,
|
||||
isCombo
|
||||
);
|
||||
const lastFailedConnectionId =
|
||||
excludedConnectionIds.size > 0
|
||||
|
||||
@@ -630,7 +630,8 @@ export function handleNoCredentials(
|
||||
model: string,
|
||||
lastError: string | null,
|
||||
lastStatus: number | null,
|
||||
candidateAliases?: readonly string[]
|
||||
candidateAliases?: readonly string[],
|
||||
isCombo: boolean = false
|
||||
) {
|
||||
if (credentials?.allRateLimited) {
|
||||
const errorMsg = lastError || credentials.lastError || "Unavailable";
|
||||
@@ -705,7 +706,7 @@ export function handleNoCredentials(
|
||||
log.warn("AUTH", `No active credentials for provider: ${provider}`);
|
||||
// #FIX: surface the candidate aliases (from resolveModelOrError) so the
|
||||
// operator can pick a working provider/model prefix instead of guessing.
|
||||
// Without this, "No active credentials for provider: kiro" leaves the
|
||||
// Without this, "No active credentials for provider: byNara" leaves the
|
||||
// user staring at a wall — most bugs in this area are actually "wrong
|
||||
// provider was picked", not "the provider is broken".
|
||||
const hint =
|
||||
@@ -715,6 +716,26 @@ export function handleNoCredentials(
|
||||
.map((a) => `${a}/${model}`)
|
||||
.join(", ")}.`
|
||||
: "";
|
||||
|
||||
// Issue #2: for single-model (non-combo) requests, a 404 leaks a misleading
|
||||
// "No active credentials" status to a direct API client (e.g. OpenCode) that
|
||||
// then mis-files it as "resource not found" instead of an auth/credential
|
||||
// failure. The 404 is only meaningful as a combo fall-through signal, so
|
||||
// remap it to an explicit error status for single-model traffic: a 401 when
|
||||
// the provider exists but has no usable credentials, else 503 when the
|
||||
// provider itself is unknown/unreachable. Combo routing keeps the 404 so it
|
||||
// can still skip past a disabled-credentials leg.
|
||||
if (!isCombo) {
|
||||
const singleModelStatus =
|
||||
provider && String(provider).trim().length > 0
|
||||
? HTTP_STATUS.UNAUTHORIZED
|
||||
: HTTP_STATUS.SERVICE_UNAVAILABLE;
|
||||
return errorResponse(
|
||||
singleModelStatus,
|
||||
`No active credentials for provider: ${provider}.${hint}`
|
||||
);
|
||||
}
|
||||
|
||||
return errorResponse(
|
||||
HTTP_STATUS.NOT_FOUND,
|
||||
`No active credentials for provider: ${provider}.${hint}`
|
||||
|
||||
Reference in New Issue
Block a user