fix(model-fallback): retry forbidden provider errors
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -251,24 +251,24 @@ describe("extractErrorMessage", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe("#given complex error with data wrapper", () => {
|
describe("#given complex error with data wrapper", () => {
|
||||||
test("extracts from error.data.message", () => {
|
test("extracts from error.data.message", () => {
|
||||||
const error = {
|
const error = {
|
||||||
data: {
|
data: {
|
||||||
message: "data message",
|
message: "data message",
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
expect(extractErrorMessage(error)).toBe("data message")
|
expect(extractErrorMessage(error)).toBe("data message")
|
||||||
})
|
})
|
||||||
|
|
||||||
test("prefers top over nested-level message", () => {
|
test("prefers nested message over generic top-level message", () => {
|
||||||
const error = {
|
const error = {
|
||||||
message: "top level",
|
message: "Error",
|
||||||
data: { message: "nested" },
|
data: { message: "Forbidden: Selected provider is forbidden" },
|
||||||
}
|
}
|
||||||
expect(extractErrorMessage(error)).toBe("top level")
|
expect(extractErrorMessage(error)).toBe("Forbidden: Selected provider is forbidden")
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe("#given invalid inputs", () => {
|
describe("#given invalid inputs", () => {
|
||||||
test("returns undefined for null", () => {
|
test("returns undefined for null", () => {
|
||||||
|
|||||||
@@ -33,16 +33,15 @@ export function extractErrorName(error: unknown): string | undefined {
|
|||||||
export function extractErrorMessage(error: unknown): string | undefined {
|
export function extractErrorMessage(error: unknown): string | undefined {
|
||||||
if (!error) return undefined
|
if (!error) return undefined
|
||||||
if (typeof error === "string") return error
|
if (typeof error === "string") return error
|
||||||
if (error instanceof Error) return error.message
|
|
||||||
|
|
||||||
if (isRecord(error)) {
|
if (isRecord(error)) {
|
||||||
const dataRaw = error["data"]
|
const dataRaw = error["data"]
|
||||||
const candidates: unknown[] = [
|
const candidates: unknown[] = [
|
||||||
error,
|
|
||||||
dataRaw,
|
dataRaw,
|
||||||
error["error"],
|
|
||||||
isRecord(dataRaw) ? (dataRaw as Record<string, unknown>)["error"] : undefined,
|
isRecord(dataRaw) ? (dataRaw as Record<string, unknown>)["error"] : undefined,
|
||||||
|
error["error"],
|
||||||
error["cause"],
|
error["cause"],
|
||||||
|
error,
|
||||||
]
|
]
|
||||||
|
|
||||||
for (const candidate of candidates) {
|
for (const candidate of candidates) {
|
||||||
@@ -57,6 +56,8 @@ export function extractErrorMessage(error: unknown): string | undefined {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (error instanceof Error) return error.message
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return JSON.stringify(error)
|
return JSON.stringify(error)
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { describe, it, expect, afterEach, mock, spyOn } from "bun:test"
|
import { describe, it, expect, afterEach, mock, spyOn } from "bun:test"
|
||||||
|
|
||||||
import { createEventHandler } from "./event"
|
import { createEventHandler, extractErrorMessage } from "./event"
|
||||||
import { createChatMessageHandler } from "./chat-message"
|
import { createChatMessageHandler } from "./chat-message"
|
||||||
import * as openclawRuntimeDispatch from "../openclaw/runtime-dispatch"
|
import * as openclawRuntimeDispatch from "../openclaw/runtime-dispatch"
|
||||||
import { _resetForTesting, setMainSession } from "../features/claude-code-session-state"
|
import { _resetForTesting, setMainSession } from "../features/claude-code-session-state"
|
||||||
@@ -93,6 +93,23 @@ afterEach(() => {
|
|||||||
_resetForTesting()
|
_resetForTesting()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("event error extraction", () => {
|
||||||
|
it("prefers nested APIError message over generic top-level message", async () => {
|
||||||
|
//#given
|
||||||
|
const error = {
|
||||||
|
name: "APIError",
|
||||||
|
message: "Error",
|
||||||
|
data: { message: "Forbidden: Selected provider is forbidden" },
|
||||||
|
}
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = extractErrorMessage(error)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toBe("Forbidden: Selected provider is forbidden")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("createEventHandler - idle deduplication", () => {
|
describe("createEventHandler - idle deduplication", () => {
|
||||||
it("#given synthetic idle fires first #when real idle arrives within 500ms #then real idle dispatched", async () => {
|
it("#given synthetic idle fires first #when real idle arrives within 500ms #then real idle dispatched", async () => {
|
||||||
//#given
|
//#given
|
||||||
|
|||||||
+5
-4
@@ -63,18 +63,17 @@ function extractErrorName(error: unknown): string | undefined {
|
|||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
function extractErrorMessage(error: unknown): string {
|
export function extractErrorMessage(error: unknown): string {
|
||||||
if (!error) return "";
|
if (!error) return "";
|
||||||
if (typeof error === "string") return error;
|
if (typeof error === "string") return error;
|
||||||
if (error instanceof Error) return error.message;
|
|
||||||
|
|
||||||
if (isRecord(error)) {
|
if (isRecord(error)) {
|
||||||
const candidates: unknown[] = [
|
const candidates: unknown[] = [
|
||||||
error,
|
|
||||||
error.data,
|
error.data,
|
||||||
error.error,
|
|
||||||
isRecord(error.data) ? error.data.error : undefined,
|
isRecord(error.data) ? error.data.error : undefined,
|
||||||
|
error.error,
|
||||||
error.cause,
|
error.cause,
|
||||||
|
error,
|
||||||
];
|
];
|
||||||
|
|
||||||
for (const candidate of candidates) {
|
for (const candidate of candidates) {
|
||||||
@@ -84,6 +83,8 @@ function extractErrorMessage(error: unknown): string {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (error instanceof Error) return error.message;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return JSON.stringify(error);
|
return JSON.stringify(error);
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
@@ -237,6 +237,17 @@ describe("model-error-classifier", () => {
|
|||||||
//#then
|
//#then
|
||||||
expect(result).toBe(true)
|
expect(result).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("treats forbidden provider message as retryable", () => {
|
||||||
|
//#given
|
||||||
|
const error = { message: "Forbidden: Selected provider is forbidden" }
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = shouldRetryError(error)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toBe(true)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
export {}
|
export {}
|
||||||
|
|||||||
@@ -2,8 +2,8 @@ import type { FallbackEntry } from "./model-requirements"
|
|||||||
import { readConnectedProvidersCache } from "./connected-providers-cache"
|
import { readConnectedProvidersCache } from "./connected-providers-cache"
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Error names that indicate a retryable model error (deadstop).
|
* Error names that indicate a retryable model error.
|
||||||
* These errors completely halt the action loop and should trigger fallback retry.
|
* These errors halt execution and should trigger fallback retry.
|
||||||
*/
|
*/
|
||||||
const RETRYABLE_ERROR_NAMES = new Set([
|
const RETRYABLE_ERROR_NAMES = new Set([
|
||||||
"providermodelnotfounderror",
|
"providermodelnotfounderror",
|
||||||
@@ -121,7 +121,7 @@ export interface ErrorInfo {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Determines if an error is a retryable model error.
|
* Determines if an error is a retryable model error.
|
||||||
* Returns true if the error is a known retryable type OR matches retryable message patterns.
|
* Returns true if it's a known retryable type OR matches retryable message patterns.
|
||||||
*/
|
*/
|
||||||
export function isRetryableModelError(error: ErrorInfo): boolean {
|
export function isRetryableModelError(error: ErrorInfo): boolean {
|
||||||
// If we have an error name, check against known lists
|
// If we have an error name, check against known lists
|
||||||
@@ -156,7 +156,7 @@ export function isRetryableModelError(error: ErrorInfo): boolean {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Determines if an error should trigger a fallback retry.
|
* Determines if an error should trigger a fallback retry.
|
||||||
* Returns true for deadstop errors that completely halt the action loop.
|
* Returns true for errors that halt execution.
|
||||||
*/
|
*/
|
||||||
export function shouldRetryError(error: ErrorInfo): boolean {
|
export function shouldRetryError(error: ErrorInfo): boolean {
|
||||||
return isRetryableModelError(error)
|
return isRetryableModelError(error)
|
||||||
|
|||||||
Reference in New Issue
Block a user