mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 19:02:17 +03:00
fix(sse): trust the adapter's explicit status over message patterns
CONTRACT CHANGE. classifyChatGptSessionError put message-pattern matching ahead of an explicit numeric status, so an adapter error carrying status 503 whose prose mentioned signing in was classified 401 session_expired — marking a healthy account's credentials expired and pulling it out of rotation. The vendor documents the field as "Authoritative upstream/proxy status when known; avoids message-based classification" (vendor/codex-chatgpt-web/types.ts). Explicit status now runs third (after ChatGptSessionInputError and TimeoutError, both of which are stronger signals), uses the event's own code when present, and attaches fallbackHint: "connection_cooldown" for 503/429 so a genuine outage cools one connection instead of tripping the whole-provider breaker. Message matching is unchanged and still essential — errors thrown by the executor itself carry no status. ChatGptSessionStreamOpen's error arm now carries fallbackHint, so the executor uses the bridge's classification directly instead of re-classifying the sanitized message. The test "a matching message wins over an explicit upstream status" is deliberately inverted and renamed; its comment records why.
This commit is contained in:
@@ -313,13 +313,13 @@ export class ChatGptSessionExecutor extends BaseExecutor {
|
||||
void run();
|
||||
const opened = await openChatGptSessionStream(events, meta);
|
||||
if (opened.kind === "error") {
|
||||
// `opened.status`/`opened.code` are authoritative: the bridge classified the real
|
||||
// adapter event, which carries its own `status`/`code` and its own `name` (a Playwright
|
||||
// TimeoutError is classified by name, and its message alone would fall through to a
|
||||
// breaker-tripping 502). `ChatGptSessionStreamOpen` has no `fallbackHint` field, so the
|
||||
// message is re-examined for that one value and nothing else.
|
||||
const hint = classifyChatGptSessionError(new Error(opened.message)).fallbackHint;
|
||||
return wrapped(errorResponse(opened.status, opened.message, opened.code, hint), input.body);
|
||||
// Every field of the verdict comes from the bridge's classification of the real adapter
|
||||
// event — status, code and fallbackHint alike. Re-classifying the sanitized message here
|
||||
// would discard the event's own `status`/`code`/`name`.
|
||||
return wrapped(
|
||||
errorResponse(opened.status, opened.message, opened.code, opened.fallbackHint),
|
||||
input.body
|
||||
);
|
||||
}
|
||||
return wrapped(
|
||||
new Response(opened.stream, { status: 200, headers: SSE_HEADERS }),
|
||||
|
||||
@@ -68,6 +68,22 @@ export function classifyChatGptSessionError(error: unknown): ChatGptSessionError
|
||||
return { status: 400, code: "browser_ui_timeout" };
|
||||
}
|
||||
|
||||
// An explicit numeric status is the adapter's own authoritative verdict. The vendor documents
|
||||
// it that way on `AdapterEvent.error.status` ("Authoritative upstream/proxy status when known;
|
||||
// avoids message-based classification"), and trusting it first is what stops a 503 whose prose
|
||||
// happens to mention signing in from being reported as an expired session — which would mark a
|
||||
// healthy account's credentials dead and pull it out of rotation. Message matching still runs
|
||||
// below, because failures thrown by this executor itself carry no status at all.
|
||||
if (typeof like.status === "number" && like.status >= 400) {
|
||||
const status = like.status;
|
||||
return {
|
||||
status,
|
||||
code: like.code ?? "turn_failed",
|
||||
// 503/429 must cool this one connection down rather than trip the whole-provider breaker.
|
||||
...(status === 503 || status === 429 ? { fallbackHint: "connection_cooldown" as const } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
if (BROWSER_UNAVAILABLE.test(like.message)) {
|
||||
return { status: 503, code: "browser_unavailable", fallbackHint: "connection_cooldown" };
|
||||
}
|
||||
@@ -86,8 +102,5 @@ export function classifyChatGptSessionError(error: unknown): ChatGptSessionError
|
||||
if (UI_TIMEOUT.test(like.message)) {
|
||||
return { status: 400, code: "browser_ui_timeout" };
|
||||
}
|
||||
if (typeof like.status === "number" && like.status >= 400) {
|
||||
return { status: like.status, code: like.code ?? "turn_failed" };
|
||||
}
|
||||
return { status: 502, code: "turn_failed" };
|
||||
}
|
||||
|
||||
@@ -80,13 +80,72 @@ test("an explicit upstream status is used when no message pattern matches", () =
|
||||
assert.equal(result.status, 502);
|
||||
});
|
||||
|
||||
// Message-pattern matching intentionally runs before the explicit-status branch, so a
|
||||
// recognised message wins even when the error also carries an explicit upstream status.
|
||||
test("a matching message wins over an explicit upstream status", () => {
|
||||
// CONTRACT CHANGE (deliberate inversion of the previous expectation). This test used to assert
|
||||
// that a recognised message beat an explicit upstream status; the precedence is now the other way
|
||||
// round. The vendor documents `AdapterEvent.error.status` as "Authoritative upstream/proxy status
|
||||
// when known; avoids message-based classification", and honouring that is what stops a 503 whose
|
||||
// prose happens to mention signing in from being reported as an expired session — which would
|
||||
// mark a HEALTHY account's credentials dead and pull it out of rotation. Message matching still
|
||||
// runs, just as the fallback for failures that carry no status at all (everything this executor
|
||||
// throws itself).
|
||||
test("an explicit upstream status wins over a matching message", () => {
|
||||
const result = classifyChatGptSessionError({
|
||||
message: "ChatGPT reported a usage limit",
|
||||
status: 500,
|
||||
});
|
||||
assert.equal(result.status, 500);
|
||||
assert.equal(result.code, "turn_failed");
|
||||
});
|
||||
|
||||
test("a session-expired message cannot downgrade an explicit 503 to a 401", () => {
|
||||
const result = classifyChatGptSessionError({
|
||||
message: "Please sign in to continue",
|
||||
status: 503,
|
||||
});
|
||||
assert.equal(result.status, 503);
|
||||
assert.notEqual(result.code, "session_expired");
|
||||
});
|
||||
|
||||
test("an explicit 503 carries the connection cooldown hint", () => {
|
||||
const result = classifyChatGptSessionError({ message: "upstream unavailable", status: 503 });
|
||||
assert.equal(result.status, 503);
|
||||
assert.equal(result.fallbackHint, "connection_cooldown");
|
||||
});
|
||||
|
||||
test("an explicit 429 carries the connection cooldown hint", () => {
|
||||
const result = classifyChatGptSessionError({ message: "slow down", status: 429 });
|
||||
assert.equal(result.status, 429);
|
||||
assert.equal(result.fallbackHint, "connection_cooldown");
|
||||
});
|
||||
|
||||
test("an explicit 400 carries no cooldown hint", () => {
|
||||
const result = classifyChatGptSessionError({ message: "bad request", status: 400 });
|
||||
assert.equal(result.status, 400);
|
||||
assert.equal(result.fallbackHint, undefined);
|
||||
});
|
||||
|
||||
test("an explicit status keeps the event's own code when it carries one", () => {
|
||||
const result = classifyChatGptSessionError({
|
||||
message: "anything",
|
||||
status: 503,
|
||||
code: "upstream_unavailable",
|
||||
});
|
||||
assert.equal(result.code, "upstream_unavailable");
|
||||
});
|
||||
|
||||
test("a TimeoutError still outranks an explicit upstream status", () => {
|
||||
const timeout = new Error("locator.waitForSelector: Timeout 30000ms exceeded") as Error & {
|
||||
status?: number;
|
||||
};
|
||||
timeout.name = "TimeoutError";
|
||||
timeout.status = 503;
|
||||
const result = classifyChatGptSessionError(timeout);
|
||||
assert.equal(result.status, 400);
|
||||
assert.equal(result.code, "browser_ui_timeout");
|
||||
});
|
||||
|
||||
test("message patterns still classify failures that carry no status", () => {
|
||||
const result = classifyChatGptSessionError(new Error("ChatGPT reported a usage limit"));
|
||||
assert.equal(result.status, 429);
|
||||
assert.equal(result.code, "rate_limited");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user