From 25548f2561ec216a23d340976b049870c3a987d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Choi=20Kijin=20/=20=EC=B5=9C=20=EA=B8=B0=EC=A7=84=20/=20?= =?UTF-8?q?=E3=83=81=E3=83=A7=E3=82=A4=20=E3=82=AD=E3=82=B8=E3=83=B3?= Date: Tue, 28 Apr 2026 15:29:33 +0900 Subject: [PATCH] fix(model-fallback): retry forbidden provider errors Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../background-agent/error-classifier.test.ts | 34 +++++++++---------- .../background-agent/error-classifier.ts | 7 ++-- src/plugin/event.test.ts | 19 ++++++++++- src/plugin/event.ts | 9 ++--- src/shared/model-error-classifier.test.ts | 11 ++++++ src/shared/model-error-classifier.ts | 8 ++--- 6 files changed, 59 insertions(+), 29 deletions(-) diff --git a/src/features/background-agent/error-classifier.test.ts b/src/features/background-agent/error-classifier.test.ts index 1fe24e93d..156c6ef4c 100644 --- a/src/features/background-agent/error-classifier.test.ts +++ b/src/features/background-agent/error-classifier.test.ts @@ -251,24 +251,24 @@ describe("extractErrorMessage", () => { }) }) - describe("#given complex error with data wrapper", () => { - test("extracts from error.data.message", () => { - const error = { - data: { - message: "data message", - }, - } - expect(extractErrorMessage(error)).toBe("data message") - }) + describe("#given complex error with data wrapper", () => { + test("extracts from error.data.message", () => { + const error = { + data: { + message: "data message", + }, + } + expect(extractErrorMessage(error)).toBe("data message") + }) - test("prefers top over nested-level message", () => { - const error = { - message: "top level", - data: { message: "nested" }, - } - expect(extractErrorMessage(error)).toBe("top level") - }) - }) + test("prefers nested message over generic top-level message", () => { + const error = { + message: "Error", + data: { message: "Forbidden: Selected provider is forbidden" }, + } + expect(extractErrorMessage(error)).toBe("Forbidden: Selected provider is forbidden") + }) + }) describe("#given invalid inputs", () => { test("returns undefined for null", () => { diff --git a/src/features/background-agent/error-classifier.ts b/src/features/background-agent/error-classifier.ts index 5c7e90b46..523f61bc1 100644 --- a/src/features/background-agent/error-classifier.ts +++ b/src/features/background-agent/error-classifier.ts @@ -33,16 +33,15 @@ export function extractErrorName(error: unknown): string | undefined { export function extractErrorMessage(error: unknown): string | undefined { if (!error) return undefined if (typeof error === "string") return error - if (error instanceof Error) return error.message if (isRecord(error)) { const dataRaw = error["data"] const candidates: unknown[] = [ - error, dataRaw, - error["error"], isRecord(dataRaw) ? (dataRaw as Record)["error"] : undefined, + error["error"], error["cause"], + error, ] for (const candidate of candidates) { @@ -57,6 +56,8 @@ export function extractErrorMessage(error: unknown): string | undefined { } } + if (error instanceof Error) return error.message + try { return JSON.stringify(error) } catch { diff --git a/src/plugin/event.test.ts b/src/plugin/event.test.ts index ea880c145..406f7b761 100644 --- a/src/plugin/event.test.ts +++ b/src/plugin/event.test.ts @@ -1,6 +1,6 @@ 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 * as openclawRuntimeDispatch from "../openclaw/runtime-dispatch" import { _resetForTesting, setMainSession } from "../features/claude-code-session-state" @@ -93,6 +93,23 @@ afterEach(() => { _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", () => { it("#given synthetic idle fires first #when real idle arrives within 500ms #then real idle dispatched", async () => { //#given diff --git a/src/plugin/event.ts b/src/plugin/event.ts index 5a5f177b6..abfa84cac 100644 --- a/src/plugin/event.ts +++ b/src/plugin/event.ts @@ -63,18 +63,17 @@ function extractErrorName(error: unknown): string | undefined { return undefined; } -function extractErrorMessage(error: unknown): string { +export function extractErrorMessage(error: unknown): string { if (!error) return ""; if (typeof error === "string") return error; - if (error instanceof Error) return error.message; if (isRecord(error)) { const candidates: unknown[] = [ - error, error.data, - error.error, isRecord(error.data) ? error.data.error : undefined, + error.error, error.cause, + error, ]; for (const candidate of candidates) { @@ -84,6 +83,8 @@ function extractErrorMessage(error: unknown): string { } } + if (error instanceof Error) return error.message; + try { return JSON.stringify(error); } catch { diff --git a/src/shared/model-error-classifier.test.ts b/src/shared/model-error-classifier.test.ts index 35c75de0a..c4989d199 100644 --- a/src/shared/model-error-classifier.test.ts +++ b/src/shared/model-error-classifier.test.ts @@ -237,6 +237,17 @@ describe("model-error-classifier", () => { //#then 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 {} diff --git a/src/shared/model-error-classifier.ts b/src/shared/model-error-classifier.ts index 85b53b500..611a71aac 100644 --- a/src/shared/model-error-classifier.ts +++ b/src/shared/model-error-classifier.ts @@ -2,8 +2,8 @@ import type { FallbackEntry } from "./model-requirements" import { readConnectedProvidersCache } from "./connected-providers-cache" /** - * Error names that indicate a retryable model error (deadstop). - * These errors completely halt the action loop and should trigger fallback retry. + * Error names that indicate a retryable model error. + * These errors halt execution and should trigger fallback retry. */ const RETRYABLE_ERROR_NAMES = new Set([ "providermodelnotfounderror", @@ -121,7 +121,7 @@ export interface ErrorInfo { /** * 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 { // 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. - * Returns true for deadstop errors that completely halt the action loop. + * Returns true for errors that halt execution. */ export function shouldRetryError(error: ErrorInfo): boolean { return isRetryableModelError(error)