mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-01 21:02:12 +03:00
Mistral's quota-exhausted response is a bare 401 with a contentless
{"detail":"Unauthorized"} body — byte-identical to a genuinely revoked
key. classifyFailure() in the connection-test route always resolved
this to upstream_auth_error ("Invalid API key"), hiding the real
quota-exhaustion cause and misleading operators into rotating a still-
valid key.
classifyFailure() now accepts an optional `provider` and, for a bare
Mistral 401 with no explicit auth signal in the message (no "invalid
api key" / "token invalid" / "revoked" / "access denied" text), returns
`upstream_ambiguous_auth_or_quota` instead. A Mistral 401 that DOES
carry an explicit auth signal, and any non-Mistral 401, are unaffected
and still classify as upstream_auth_error (baseline preserved).
The new branching logic lives in a new module
(mistralAmbiguousAuth.ts) rather than inline in route.ts, keeping that
frozen file's line count within its file-size-baseline.json budget.
TDD: tests/unit/provider-test-mistral-401-classify.test.ts reproduces
the bug (RED against unfixed classifyFailure), proves the fix (GREEN),
and pins the baseline non-Mistral-401 behavior per the owner's
explicit requirement.
This commit is contained in:
committed by
GitHub
parent
a9eb25b93c
commit
69bbcafcb4
1
changelog.d/fixes/7638-mistral-401-classify.md
Normal file
1
changelog.d/fixes/7638-mistral-401-classify.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(providers): classify Mistral ambiguous 401 (quota vs revoked key) instead of asserting hard auth failure (#7638)
|
||||
57
src/app/api/providers/[id]/test/mistralAmbiguousAuth.ts
Normal file
57
src/app/api/providers/[id]/test/mistralAmbiguousAuth.ts
Normal file
@@ -0,0 +1,57 @@
|
||||
/**
|
||||
* #7638: Mistral's quota-exhausted response is `401 {"detail":"Unauthorized"}` — byte-identical
|
||||
* to a genuinely revoked key. Unlike other providers, a bare Mistral 401 with no auth-specific
|
||||
* signal in the body cannot be trusted as a hard auth failure; the generic 401/403 branch in
|
||||
* classifyFailure() delegates here so it can return an ambiguous diagnosis instead of asserting
|
||||
* "Invalid API key" outright.
|
||||
* A message that DOES carry an explicit auth signal (e.g. "Invalid API key") still falls
|
||||
* through to the normal `upstream_auth_error` result — only the contentless case is ambiguous.
|
||||
*/
|
||||
export interface AuthOr401Diagnosis {
|
||||
type: string;
|
||||
source: string;
|
||||
message: string | null;
|
||||
code: string | null;
|
||||
}
|
||||
|
||||
/** Param type for classifyFailure() in route.ts — extracted here to keep that frozen file's LOC flat. */
|
||||
export interface ClassifyFailureArgs {
|
||||
error: string;
|
||||
statusCode?: number | null;
|
||||
refreshFailed?: boolean;
|
||||
unsupported?: boolean;
|
||||
provider?: string;
|
||||
}
|
||||
|
||||
function isMistralAmbiguous401(provider: string | undefined, normalized: string): boolean {
|
||||
if (provider !== "mistral") return false;
|
||||
const hasAuthSignal =
|
||||
normalized.includes("invalid api key") ||
|
||||
normalized.includes("token invalid") ||
|
||||
normalized.includes("revoked") ||
|
||||
normalized.includes("access denied");
|
||||
return !hasAuthSignal;
|
||||
}
|
||||
|
||||
/** Decides the diagnosis for a 401/403 status: ambiguous (Mistral-only) or the generic auth error. */
|
||||
export function classifyAmbiguousOrAuthError(
|
||||
provider: string | undefined,
|
||||
normalized: string,
|
||||
message: string,
|
||||
numericStatus: number
|
||||
): AuthOr401Diagnosis {
|
||||
if (numericStatus === 401 && isMistralAmbiguous401(provider, normalized)) {
|
||||
return {
|
||||
type: "upstream_ambiguous_auth_or_quota",
|
||||
source: "upstream",
|
||||
message: message || null,
|
||||
code: String(numericStatus),
|
||||
};
|
||||
}
|
||||
return {
|
||||
type: "upstream_auth_error",
|
||||
source: "upstream",
|
||||
message: message || null,
|
||||
code: String(numericStatus),
|
||||
};
|
||||
}
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
} from "@/lib/oauth/gitlab";
|
||||
import { providerAllowsOptionalApiKey } from "@/shared/constants/providers";
|
||||
import { removeConnectionHealth } from "@omniroute/open-sse/services/apiKeyRotator.ts";
|
||||
import { classifyAmbiguousOrAuthError, type ClassifyFailureArgs } from "./mistralAmbiguousAuth";
|
||||
|
||||
// Bound the OAuth probe so a hung upstream can't block the connection-test queue
|
||||
// forever (#1449). Mirrors the 30s timeout the API-key path uses via validateProviderApiKey.
|
||||
@@ -188,12 +189,8 @@ export function classifyFailure({
|
||||
statusCode = null,
|
||||
refreshFailed = false,
|
||||
unsupported = false,
|
||||
}: {
|
||||
error: string;
|
||||
statusCode?: number | null;
|
||||
refreshFailed?: boolean;
|
||||
unsupported?: boolean;
|
||||
}) {
|
||||
provider,
|
||||
}: ClassifyFailureArgs) {
|
||||
const message = toSafeMessage(error, "Connection test failed");
|
||||
const normalized = message.toLowerCase();
|
||||
const numericStatus = Number.isFinite(statusCode) ? Number(statusCode) : null;
|
||||
@@ -214,7 +211,7 @@ export function classifyFailure({
|
||||
}
|
||||
|
||||
if (numericStatus === 401 || numericStatus === 403) {
|
||||
return makeDiagnosis("upstream_auth_error", "upstream", message, String(numericStatus));
|
||||
return classifyAmbiguousOrAuthError(provider, normalized, message, numericStatus);
|
||||
}
|
||||
|
||||
if (numericStatus === 429) {
|
||||
@@ -722,14 +719,14 @@ async function testApiKeyConnection(connection: any) {
|
||||
return {
|
||||
valid: false,
|
||||
error,
|
||||
diagnosis: classifyFailure({ error, unsupported: true }),
|
||||
diagnosis: classifyFailure({ error, unsupported: true, provider: connection.provider }),
|
||||
};
|
||||
}
|
||||
|
||||
const error = result.valid ? null : result.error || "Invalid API key";
|
||||
const diagnosis = result.valid
|
||||
? makeDiagnosis("ok", "upstream", null, null)
|
||||
: classifyFailure({ error });
|
||||
: classifyFailure({ error, statusCode: result.statusCode, provider: connection.provider });
|
||||
|
||||
return {
|
||||
valid: !!result.valid,
|
||||
@@ -813,7 +810,7 @@ export async function testSingleConnection(connectionId: string, validationModel
|
||||
result.diagnosis ||
|
||||
(result.valid
|
||||
? makeDiagnosis("ok", "local", null, null)
|
||||
: classifyFailure({ error: result.error, statusCode: result.statusCode }));
|
||||
: classifyFailure({ error: result.error, statusCode: result.statusCode, provider }));
|
||||
|
||||
const updateData: Record<string, any> = {
|
||||
testStatus: result.valid ? "active" : "error",
|
||||
|
||||
27
tests/unit/provider-test-mistral-401-classify.test.ts
Normal file
27
tests/unit/provider-test-mistral-401-classify.test.ts
Normal file
@@ -0,0 +1,27 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { classifyFailure } = await import("../../src/app/api/providers/[id]/test/route.ts");
|
||||
|
||||
test("#7638: a Mistral ambiguous 401 (contentless body) should not be asserted as a hard auth_failed", () => {
|
||||
const d = classifyFailure({
|
||||
error: '{"detail":"Unauthorized"}',
|
||||
statusCode: 401,
|
||||
provider: "mistral",
|
||||
});
|
||||
assert.notEqual(d.type, "upstream_auth_error");
|
||||
});
|
||||
|
||||
test("#7638 baseline: a non-Mistral bare 401 still correctly classifies as upstream_auth_error", () => {
|
||||
const d = classifyFailure({ error: '{"detail":"Unauthorized"}', statusCode: 401 });
|
||||
assert.equal(d.type, "upstream_auth_error");
|
||||
});
|
||||
|
||||
test("#7638: a Mistral 401 with an explicit auth-signal message still classifies as upstream_auth_error", () => {
|
||||
const d = classifyFailure({
|
||||
error: "Invalid API key",
|
||||
statusCode: 401,
|
||||
provider: "mistral",
|
||||
});
|
||||
assert.equal(d.type, "upstream_auth_error");
|
||||
});
|
||||
Reference in New Issue
Block a user