Merge pull request #527 from alibaba/fix/abort-error-contract

improve error handling

packages/core/src/PageAgentCore.ts

@@ -103,11 +103,14 @@ export class PageAgentCore extends EventTarget {
 		this.tools = new Map(tools)
 		this.pageController = config.pageController
 
-		// Listen to LLM retry events
 		this.#llm.addEventListener('retry', (e) => {
-			const { attempt, maxAttempts } = (e as CustomEvent).detail
+			const { attempt, maxAttempts, lastError } = (e as CustomEvent).detail
 			this.#emitActivity({ type: 'retrying', attempt, maxAttempts })
-			// Also push to history for panel rendering
+			this.history.push({
+				type: 'error',
+				message: String(lastError),
+				rawResponse: (lastError as InvokeError).rawResponse,
+			})
 			this.history.push({
 				type: 'retry',
 				message: `LLM retry attempt ${attempt} of ${maxAttempts}`,
@@ -116,19 +119,6 @@ export class PageAgentCore extends EventTarget {
 			})
 			this.#emitHistoryChange()
 		})
-		this.#llm.addEventListener('error', (e) => {
-			const error = (e as CustomEvent).detail.error as Error | InvokeError
-			if ((error as any)?.rawError?.name === 'AbortError') return
-			const message = String(error)
-			this.#emitActivity({ type: 'error', message })
-			// Also push to history for panel rendering
-			this.history.push({
-				type: 'error',
-				message,
-				rawResponse: (error as InvokeError).rawResponse,
-			})
-			this.#emitHistoryChange()
-		})
 
 		if (this.config.customTools) {
 			for (const [name, tool] of Object.entries(this.config.customTools)) {
@@ -312,9 +302,9 @@ export class PageAgentCore extends EventTarget {
 				}
 			} catch (error: unknown) {
 				console.groupEnd() // to prevent nested groups
-				const isAbortError = (error as any)?.rawError?.name === 'AbortError'
+				const isAbortError = (error as any)?.name === 'AbortError'
 
-				console.error('Task failed', error)
+				if (!isAbortError) console.error('Task failed', error)
 				const errorMessage = isAbortError ? 'Task stopped' : String(error)
 				this.#emitActivity({ type: 'error', message: errorMessage })
 				this.history.push({ type: 'error', message: errorMessage, rawResponse: error })
@@ -378,8 +368,7 @@ export class PageAgentCore extends EventTarget {
 			description: 'You MUST call this tool every step!',
 			inputSchema: macroToolSchema as z.ZodType<MacroToolInput>,
 			execute: async (input: MacroToolInput): Promise<MacroToolResult> => {
-				// abort
+				this.#abortController.signal.throwIfAborted()
-				if (this.#abortController.signal.aborted) throw new Error('AbortError')
 
 				console.log(chalk.blue.bold('MacroTool input'), input)
 				const action = input.action

packages/llms/src/OpenAIClient.ts

@@ -25,6 +25,8 @@ export class OpenAIClient implements LLMClient {
 		abortSignal?: AbortSignal,
 		options?: InvokeOptions
 	): Promise<InvokeResult> {
+		abortSignal?.throwIfAborted()
+
 		// 1. Convert tools to OpenAI format
 		const openaiTools = Object.entries(tools).map(([name, t]) => zodToOpenAITool(name, t))
 
@@ -70,17 +72,20 @@ export class OpenAIClient implements LLMClient {
 				signal: abortSignal,
 			})
 		} catch (error: unknown) {
-			const isAbortError = (error as any)?.name === 'AbortError'
+			if ((error as any)?.name === 'AbortError') throw error
-			const errorMessage = isAbortError ? 'Network request aborted' : 'Network request failed'
+			console.error(error)
-			if (!isAbortError) console.error(error)
+			throw new InvokeError(InvokeErrorTypes.NETWORK_ERROR, 'Network request failed', error)
-			throw new InvokeError(InvokeErrorTypes.NETWORK_ERROR, errorMessage, error)
 		}
 
 		// 3. Handle HTTP errors
 		if (!response.ok) {
-			const errorData = await response.json().catch()
+			let errorData: any
-			const errorMessage =
+			try {
-				(errorData as { error?: { message?: string } }).error?.message || response.statusText
+				errorData = await response.json()
+			} catch (error) {
+				if ((error as any)?.name === 'AbortError') throw error
+			}
+			const errorMessage = errorData?.error?.message || response.statusText
 
 			if (response.status === 401 || response.status === 403) {
 				throw new InvokeError(
@@ -111,11 +116,21 @@ export class OpenAIClient implements LLMClient {
 		}
 
 		// 4. Parse and validate response
-		const data = await response.json()
+		let data: any
+		try {
+			data = await response.json()
+		} catch (error) {
+			if ((error as any)?.name === 'AbortError') throw error
+			throw new InvokeError(
+				InvokeErrorTypes.INVALID_RESPONSE,
+				'Response body is not valid JSON',
+				error
+			)
+		}
 
 		const choice = data.choices?.[0]
 		if (!choice) {
-			throw new InvokeError(InvokeErrorTypes.UNKNOWN, 'No choices in response', data)
+			throw new InvokeError(InvokeErrorTypes.INVALID_SCHEMA, 'No choices in response', data)
 		}
 
 		// Check finish_reason
@@ -140,7 +155,7 @@ export class OpenAIClient implements LLMClient {
 				)
 			default:
 				throw new InvokeError(
-					InvokeErrorTypes.UNKNOWN,
+					InvokeErrorTypes.INVALID_SCHEMA,
 					`Unexpected finish_reason: ${choice.finish_reason}`,
 					undefined,
 					data
@@ -212,11 +227,12 @@ export class OpenAIClient implements LLMClient {
 		let toolResult: unknown
 		try {
 			toolResult = await tool.execute(toolInput)
-		} catch (e) {
+		} catch (error: unknown) {
+			if ((error as any)?.name === 'AbortError') throw error
 			throw new InvokeError(
 				InvokeErrorTypes.TOOL_EXECUTION_ERROR,
-				`Tool execution failed: ${(e as Error).message}`,
+				`Tool execution failed: ${(error as Error)?.message}`,
-				e,
+				error,
 				data
 			)
 		}

packages/llms/src/errors.ts

@@ -1,5 +1,5 @@
 /**
- * Error types and error handling for LLM invocations
+ * Error types and error handling for LLM invocations.
  */
 
 export const InvokeErrorTypes = {
@@ -10,6 +10,8 @@ export const InvokeErrorTypes = {
 	NO_TOOL_CALL: 'no_tool_call', // Model did not call tool
 	INVALID_TOOL_ARGS: 'invalid_tool_args', // Tool args don't match schema
 	TOOL_EXECUTION_ERROR: 'tool_execution_error', // Tool execution error
+	INVALID_RESPONSE: 'invalid_response', // Response body is not valid JSON
+	INVALID_SCHEMA: 'invalid_schema', // Response is valid JSON but doesn't match expected shape
 
 	UNKNOWN: 'unknown',
 
@@ -22,6 +24,18 @@ export const InvokeErrorTypes = {
 
 type InvokeErrorType = (typeof InvokeErrorTypes)[keyof typeof InvokeErrorTypes]
 
+const RETRYABLE_TYPES: readonly InvokeErrorType[] = [
+	InvokeErrorTypes.NETWORK_ERROR,
+	InvokeErrorTypes.RATE_LIMIT,
+	InvokeErrorTypes.SERVER_ERROR,
+	InvokeErrorTypes.NO_TOOL_CALL,
+	InvokeErrorTypes.INVALID_TOOL_ARGS,
+	InvokeErrorTypes.TOOL_EXECUTION_ERROR,
+	InvokeErrorTypes.INVALID_RESPONSE,
+	InvokeErrorTypes.INVALID_SCHEMA,
+	InvokeErrorTypes.UNKNOWN,
+]
+
 export class InvokeError extends Error {
 	type: InvokeErrorType
 	retryable: boolean
@@ -35,24 +49,8 @@ export class InvokeError extends Error {
 		super(message)
 		this.name = 'InvokeError'
 		this.type = type
-		this.retryable = this.isRetryable(type, rawError)
+		this.retryable = RETRYABLE_TYPES.includes(type)
 		this.rawError = rawError
 		this.rawResponse = rawResponse
 	}
-
-	private isRetryable(type: InvokeErrorType, rawError?: unknown): boolean {
-		const isAbortError = (rawError as any)?.name === 'AbortError'
-		if (isAbortError) return false
-
-		const retryableTypes: InvokeErrorType[] = [
-			InvokeErrorTypes.NETWORK_ERROR,
-			InvokeErrorTypes.RATE_LIMIT,
-			InvokeErrorTypes.SERVER_ERROR,
-			InvokeErrorTypes.NO_TOOL_CALL,
-			InvokeErrorTypes.INVALID_TOOL_ARGS,
-			InvokeErrorTypes.TOOL_EXECUTION_ERROR,
-			InvokeErrorTypes.UNKNOWN,
-		]
-		return retryableTypes.includes(type)
-	}
 }

packages/llms/src/index.ts

@@ -50,65 +50,43 @@ export class LLM extends EventTarget {
 		abortSignal: AbortSignal,
 		options?: InvokeOptions
 	): Promise<InvokeResult> {
-		return await withRetry(
+		return await withRetry(async () => this.client.invoke(messages, tools, abortSignal, options), {
-			async () => {
-				// in case user aborted before invoking
-				if (abortSignal.aborted) throw new Error('AbortError')
-
-				const result = await this.client.invoke(messages, tools, abortSignal, options)
-
-				return result
-			},
-			// retry settings
-			{
 			maxRetries: this.config.maxRetries,
-				onRetry: (attempt: number) => {
+			onRetry: (attempt, lastError) => {
 				this.dispatchEvent(
-						new CustomEvent('retry', { detail: { attempt, maxAttempts: this.config.maxRetries } })
+					new CustomEvent('retry', {
+						detail: { attempt, maxAttempts: this.config.maxRetries, lastError },
+					})
 				)
 			},
-				onError: (error: Error) => {
+		})
-					this.dispatchEvent(new CustomEvent('error', { detail: { error } }))
-				},
-			}
-		)
 	}
 }
 
+/**
+ * Retry a function until it succeeds or reaches the maximum number of retries.
+ */
 async function withRetry<T>(
 	fn: () => Promise<T>,
 	settings: {
 		maxRetries: number
-		onRetry: (attempt: number) => void
+		onRetry: (attempt: number, lastError: Error) => void
-		onError: (error: Error) => void
 	}
 ): Promise<T> {
 	let attempt = 0
-	let lastError: Error | null = null
+	while (true) {
-	while (attempt <= settings.maxRetries) {
-		if (attempt > 0) {
-			settings.onRetry(attempt)
-			await new Promise((resolve) => setTimeout(resolve, 100))
-		}
-
 		try {
 			return await fn()
 		} catch (error: unknown) {
-			// do not retry if aborted by user
+			if ((error as any)?.name === 'AbortError') throw error
-			if ((error as any)?.rawError?.name === 'AbortError') throw error
-
-			console.error(error)
-			settings.onError(error as Error)
-
-			// do not retry if error is not retryable (InvokeError)
 			if (error instanceof InvokeError && !error.retryable) throw error
-
-			lastError = error as Error
 			attempt++
+			if (attempt > settings.maxRetries) throw error
+
+			console.debug('[LLM] retryable failure, will retry:', error)
+			settings.onRetry(attempt, error as Error)
 
 			await new Promise((resolve) => setTimeout(resolve, 100))
 		}
 	}
-
-	throw lastError!
 }