From 83a1eec98643eaebf15ef71f54b5988e1bdef3b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 4 Jul 2026 10:26:24 +0200 Subject: [PATCH] fix: surface typed error codes for metro, MCP, and batch validation failures (#1070) Bare `throw new Error(...)` surfaces to users as code UNKNOWN with a generic hint. Convert the remaining user-reachable clusters from the July 2026 error audit to AppError with the right code, preserving messages: - metro/client-metro.ts: timeouts, bridge failures, and Metro start / not-ready errors are now COMMAND_FAILED (the not-ready error also carries logPath in details); createMetroBridgeRequestError builds on AppError so non-retryable bridge errors that escape via rethrow get the right code too - mcp/command-tools.ts + mcp/router.ts: MCP input validation is now INVALID_ARGS (JSON-RPC responses unchanged; the router only reads .message) - commands/batch/metadata.ts: step validation is now INVALID_ARGS; with the plain-Error opt-out gone, the now-unused BatchStepErrorFactory injection seam is removed from batch-contract.ts - cloud-webdriver/webdriver-source.ts: xml.ts parser failures are wrapped at the consumer boundary into a single COMMAND_FAILED naming what was being parsed --- src/batch-contract.ts | 31 ++++++------------------- src/cloud-webdriver/webdriver-source.ts | 13 ++++++++++- src/commands/batch/metadata.ts | 21 +++++++++-------- src/mcp/command-tools.ts | 14 +++++++---- src/mcp/router.ts | 5 ++-- src/metro/client-metro.ts | 26 ++++++++++++++------- 6 files changed, 60 insertions(+), 50 deletions(-) diff --git a/src/batch-contract.ts b/src/batch-contract.ts index 988e095f6..e5f0fd057 100644 --- a/src/batch-contract.ts +++ b/src/batch-contract.ts @@ -4,35 +4,19 @@ import { isRecord } from './utils/parsing.ts'; export const DEFAULT_BATCH_MAX_STEPS = 100; -// Builds the error thrown by a batch-step validator, so each consumer keeps its -// own thrown type (plain `Error` for metadata, `AppError` for the daemon/CLI) -// while sharing the validation logic and messages. -export type BatchStepErrorFactory = (message: string) => Error; - -const batchInvalidArgsError: BatchStepErrorFactory = (message) => - new AppError('INVALID_ARGS', message); - export function isValidBatchMaxSteps(maxSteps: number): boolean { return Number.isInteger(maxSteps) && maxSteps >= 1 && maxSteps <= 1000; } -export function assertBatchStepCount( - stepCount: number, - maxSteps: number, - makeError: BatchStepErrorFactory = batchInvalidArgsError, -): void { +export function assertBatchStepCount(stepCount: number, maxSteps: number): void { if (stepCount > maxSteps) { - throw makeError(`batch has ${stepCount} steps; max allowed is ${maxSteps}.`); + throw new AppError('INVALID_ARGS', `batch has ${stepCount} steps; max allowed is ${maxSteps}.`); } } -export function readBatchStepRecord( - step: unknown, - stepNumber: number, - makeError: BatchStepErrorFactory = batchInvalidArgsError, -): Record { +export function readBatchStepRecord(step: unknown, stepNumber: number): Record { if (!isRecord(step)) { - throw makeError(`Invalid batch step ${stepNumber}.`); + throw new AppError('INVALID_ARGS', `Invalid batch step ${stepNumber}.`); } return step; } @@ -40,11 +24,10 @@ export function readBatchStepRecord( export function readBatchStepInputObject( record: Record, stepNumber: number, - makeError: BatchStepErrorFactory = batchInvalidArgsError, ): Record { const input = record.input; if (!isRecord(input)) { - throw makeError(`Batch step ${stepNumber} input must be an object.`); + throw new AppError('INVALID_ARGS', `Batch step ${stepNumber} input must be an object.`); } return input; } @@ -52,13 +35,13 @@ export function readBatchStepInputObject( export function parseBatchStepRuntime( value: unknown, stepNumber: number, - makeError: BatchStepErrorFactory = batchInvalidArgsError, ): SessionRuntimeHints | undefined { if (value === undefined) return undefined; try { return daemonRuntimeSchema.parse(value); } catch (error) { - throw makeError( + throw new AppError( + 'INVALID_ARGS', `Batch step ${stepNumber} runtime is invalid: ${error instanceof Error ? error.message : String(error)}`, ); } diff --git a/src/cloud-webdriver/webdriver-source.ts b/src/cloud-webdriver/webdriver-source.ts index b171c79ea..72738a5b5 100644 --- a/src/cloud-webdriver/webdriver-source.ts +++ b/src/cloud-webdriver/webdriver-source.ts @@ -1,9 +1,20 @@ import type { RawSnapshotNode } from '../kernel/snapshot.ts'; +import { AppError } from '../kernel/errors.ts'; import { parseBounds } from '../utils/bounds.ts'; import { parseXmlDocumentSync, type XmlNode } from '../utils/xml.ts'; export function parseWebDriverSource(source: string): RawSnapshotNode[] { - const roots = parseXmlDocumentSync(source); + let roots: XmlNode[]; + try { + roots = parseXmlDocumentSync(source); + } catch (error) { + throw new AppError( + 'COMMAND_FAILED', + `Failed to parse WebDriver page source XML: ${error instanceof Error ? error.message : String(error)}`, + undefined, + error, + ); + } const nodes: RawSnapshotNode[] = []; for (const root of roots) { appendSourceNodes(nodes, root); diff --git a/src/commands/batch/metadata.ts b/src/commands/batch/metadata.ts index 370128b7c..a140c78ea 100644 --- a/src/commands/batch/metadata.ts +++ b/src/commands/batch/metadata.ts @@ -5,8 +5,8 @@ import { parseBatchStepRuntime, readBatchStepInputObject, readBatchStepRecord, - type BatchStepErrorFactory, } from '../../batch-contract.ts'; +import { AppError } from '../../kernel/errors.ts'; import { type SessionRuntimeHints } from '../../kernel/contracts.ts'; import { STRUCTURED_BATCH_COMMAND_NAMES, @@ -30,8 +30,6 @@ import { type InferCommandInput, } from '../command-input.ts'; -const batchPlainError: BatchStepErrorFactory = (message) => new Error(message); - export type BatchCommandStep = { command: string; input: Record; @@ -109,9 +107,9 @@ function readBatchInput(input: unknown, fields: ReturnType): const parsed = readFieldInput(input, fields); const maxSteps = parsed.maxSteps ?? DEFAULT_BATCH_MAX_STEPS; if (!isValidBatchMaxSteps(maxSteps)) { - throw new Error(`Invalid batch maxSteps: ${String(parsed.maxSteps)}`); + throw new AppError('INVALID_ARGS', `Invalid batch maxSteps: ${String(parsed.maxSteps)}`); } - assertBatchStepCount(parsed.steps.length, maxSteps, batchPlainError); + assertBatchStepCount(parsed.steps.length, maxSteps); return { ...parsed, }; @@ -119,7 +117,7 @@ function readBatchInput(input: unknown, fields: ReturnType): function readBatchSteps(steps: unknown, nestedCommands: readonly string[]): BatchCommandStep[] { if (!Array.isArray(steps)) { - throw new Error('Expected steps to be an array.'); + throw new AppError('INVALID_ARGS', 'Expected steps to be an array.'); } return steps.map((step, index) => readBatchStep(step, index + 1, nestedCommands)); } @@ -129,11 +127,11 @@ function readBatchStep( stepNumber: number, nestedCommands: readonly string[], ): BatchCommandStep { - const record = readBatchStepRecord(step, stepNumber, batchPlainError); + const record = readBatchStepRecord(step, stepNumber); assertAllowedKeys(record, ['command', 'input', 'runtime'], `Batch step ${stepNumber}`); return { command: readBatchStepCommand(record, stepNumber, nestedCommands), - input: readBatchStepInputObject(record, stepNumber, batchPlainError), + input: readBatchStepInputObject(record, stepNumber), ...readBatchStepRuntimeProperty(record, stepNumber), }; } @@ -148,7 +146,10 @@ function readBatchStepCommand( } const command = record.command; if (typeof command !== 'string' || !nestedCommands.includes(command)) { - throw new Error(`Expected command to be one of: ${nestedCommands.join(', ')}.`); + throw new AppError( + 'INVALID_ARGS', + `Expected command to be one of: ${nestedCommands.join(', ')}.`, + ); } return command; } @@ -157,6 +158,6 @@ function readBatchStepRuntimeProperty( record: Record, stepNumber: number, ): Pick { - const runtime = parseBatchStepRuntime(record.runtime, stepNumber, batchPlainError); + const runtime = parseBatchStepRuntime(record.runtime, stepNumber); return runtime === undefined ? {} : { runtime }; } diff --git a/src/mcp/command-tools.ts b/src/mcp/command-tools.ts index e36e9a3ff..585d728a2 100644 --- a/src/mcp/command-tools.ts +++ b/src/mcp/command-tools.ts @@ -8,6 +8,7 @@ import { type CommandName, } from '../commands/command-metadata.ts'; import { COMMAND_OUTPUT_SCHEMAS } from './command-output-schemas.ts'; +import { AppError } from '../kernel/errors.ts'; export type ToolResult = { isError: boolean; @@ -61,7 +62,7 @@ export function createCommandToolExecutor(deps: CommandToolExecutorDeps = {}): C return { execute: async (name, input) => { if (!isCommandName(name)) { - throw new Error(`Unknown command tool: ${name}`); + throw new AppError('INVALID_ARGS', `Unknown command tool: ${name}`); } const config = readMcpToolConfig(input); const commandInput = stripMcpConfigFields(input); @@ -124,11 +125,11 @@ function readClientConfig(record: Record): AgentDeviceClientCon const responseLevel = record.responseLevel; const client: AgentDeviceClientConfig = {}; if (stateDir !== undefined && (typeof stateDir !== 'string' || stateDir.length === 0)) { - throw new Error('Expected stateDir to be a non-empty string.'); + throw new AppError('INVALID_ARGS', 'Expected stateDir to be a non-empty string.'); } if (typeof stateDir === 'string') client.stateDir = stateDir; if (includeCost !== undefined && typeof includeCost !== 'boolean') { - throw new Error('Expected includeCost to be a boolean.'); + throw new AppError('INVALID_ARGS', 'Expected includeCost to be a boolean.'); } // Only set when explicitly true so the default request shape is untouched // (cost rides on response.data → structuredContent only when opted in). @@ -143,7 +144,10 @@ function readClientConfig(record: Record): AgentDeviceClientCon function readResponseLevel(value: unknown): ResponseLevel | undefined { if (value === undefined) return undefined; if (typeof value !== 'string' || !(RESPONSE_LEVELS as readonly string[]).includes(value)) { - throw new Error("Expected responseLevel to be one of 'digest', 'default', or 'full'."); + throw new AppError( + 'INVALID_ARGS', + "Expected responseLevel to be one of 'digest', 'default', or 'full'.", + ); } return value as ResponseLevel; } @@ -151,7 +155,7 @@ function readResponseLevel(value: unknown): ResponseLevel | undefined { function readMcpOutputFormat(outputFormat: unknown): McpOutputFormat { if (outputFormat === undefined) return 'optimized'; if (outputFormat !== 'optimized' && outputFormat !== 'json') { - throw new Error('Expected mcpOutputFormat to be "optimized" or "json".'); + throw new AppError('INVALID_ARGS', 'Expected mcpOutputFormat to be "optimized" or "json".'); } return outputFormat; } diff --git a/src/mcp/router.ts b/src/mcp/router.ts index 0c0d20a02..a4113d4c5 100644 --- a/src/mcp/router.ts +++ b/src/mcp/router.ts @@ -1,6 +1,7 @@ import { listCommandTools, commandToolExecutor, type ToolResult } from './command-tools.ts'; import { readVersion } from '../utils/version.ts'; import type { JsonRpcId, JsonRpcRequestEnvelope } from '../kernel/contracts.ts'; +import { AppError } from '../kernel/errors.ts'; const MCP_SERVER_NAME = 'agent-device'; const SUPPORTED_PROTOCOL_VERSION = '2025-11-25'; @@ -87,7 +88,7 @@ function errorResponse(id: JsonRpcId, code: number, message: string): JsonRpcRes function asRecord(value: unknown): Record { if (!value || typeof value !== 'object' || Array.isArray(value)) { - throw new Error('Expected object parameters.'); + throw new AppError('INVALID_ARGS', 'Expected object parameters.'); } return value as Record; } @@ -95,7 +96,7 @@ function asRecord(value: unknown): Record { function stringField(record: Record, key: string): string { const value = record[key]; if (typeof value !== 'string' || value.length === 0) { - throw new Error(`Expected ${key} to be a non-empty string.`); + throw new AppError('INVALID_ARGS', `Expected ${key} to be a non-empty string.`); } return value; } diff --git a/src/metro/client-metro.ts b/src/metro/client-metro.ts index 49493282c..4dc13df8d 100644 --- a/src/metro/client-metro.ts +++ b/src/metro/client-metro.ts @@ -278,7 +278,7 @@ async function fetchText( }; } catch (error) { if (error instanceof Error && error.name === 'TimeoutError') { - throw new Error(`Timed out fetching ${url} after ${timeoutMs}ms`); + throw new AppError('COMMAND_FAILED', `Timed out fetching ${url} after ${timeoutMs}ms`); } throw error; } @@ -393,7 +393,7 @@ function startMetroProcess( } if (!Number.isInteger(pid) || pid <= 0) { - throw new Error('Failed to start Metro. Expected a detached child PID.'); + throw new AppError('COMMAND_FAILED', 'Failed to start Metro. Expected a detached child PID.'); } return { @@ -433,7 +433,7 @@ function createMetroBridgeRequestError( message: string, retryable: boolean, ): MetroBridgeRequestError { - const error = new Error(message) as MetroBridgeRequestError; + const error = new AppError('COMMAND_FAILED', message) as MetroBridgeRequestError; error.retryable = retryable; return error; } @@ -598,7 +598,8 @@ function describeBridgeFailure( function requireBridgeRuntimeDescriptor(baseUrl: string, bridge: MetroBridgeResult | null): void { if (!bridge?.iosRuntime.bundleUrl) { - throw new Error( + throw new AppError( + 'COMMAND_FAILED', describeBridgeFailure( baseUrl, 'bridge descriptor is missing ios_runtime.metro_bundle_url', @@ -769,7 +770,8 @@ async function configureMetroBridgeUntilReady(options: { } catch (error) { lastBridgeError = error instanceof Error ? error.message : String(error); if (!isRetryableBridgeError(error)) { - throw new Error( + throw new AppError( + 'COMMAND_FAILED', describeBridgeFailure( options.baseUrl, lastBridgeError, @@ -777,6 +779,8 @@ async function configureMetroBridgeUntilReady(options: { options.initialBridgeError, options.companionLogPath, ), + undefined, + error, ); } } @@ -787,7 +791,8 @@ async function configureMetroBridgeUntilReady(options: { } } - throw new Error( + throw new AppError( + 'COMMAND_FAILED', describeBridgeFailure( options.baseUrl, lastBridgeError, @@ -820,8 +825,10 @@ async function ensureMetroProcessReady( } await stopSpawnedMetroProcess(startedProcess.pid).catch(() => {}); - throw new Error( + throw new AppError( + 'COMMAND_FAILED', `Metro did not become ready at ${statusUrl} within ${settings.startupTimeoutMs}ms. Check ${settings.logPath}.`, + { logPath: settings.logPath }, ); } @@ -886,13 +893,16 @@ async function configureProxyBridgeViaCompanion( }); companionLogPath = companion.logPath; } catch (error) { - throw new Error( + throw new AppError( + 'COMMAND_FAILED', describeBridgeFailure( settings.proxyBaseUrl, error instanceof Error ? error.message : String(error), bridge, initialBridgeError, ), + undefined, + error, ); }