mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
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
This commit is contained in:
committed by
GitHub
parent
1009086145
commit
83a1eec986
+7
-24
@@ -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<string, unknown> {
|
||||
export function readBatchStepRecord(step: unknown, stepNumber: number): Record<string, unknown> {
|
||||
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<string, unknown>,
|
||||
stepNumber: number,
|
||||
makeError: BatchStepErrorFactory = batchInvalidArgsError,
|
||||
): Record<string, unknown> {
|
||||
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)}`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
@@ -109,9 +107,9 @@ function readBatchInput(input: unknown, fields: ReturnType<typeof batchFields>):
|
||||
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<typeof batchFields>):
|
||||
|
||||
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<string, unknown>,
|
||||
stepNumber: number,
|
||||
): Pick<BatchCommandStep, 'runtime'> {
|
||||
const runtime = parseBatchStepRuntime(record.runtime, stepNumber, batchPlainError);
|
||||
const runtime = parseBatchStepRuntime(record.runtime, stepNumber);
|
||||
return runtime === undefined ? {} : { runtime };
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>): 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<string, unknown>): 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;
|
||||
}
|
||||
|
||||
+3
-2
@@ -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<string, unknown> {
|
||||
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<string, unknown>;
|
||||
}
|
||||
@@ -95,7 +96,7 @@ function asRecord(value: unknown): Record<string, unknown> {
|
||||
function stringField(record: Record<string, unknown>, 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;
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user