diff --git a/src/host/README.md b/src/host/README.md index e5ff579c..0304f2ca 100644 --- a/src/host/README.md +++ b/src/host/README.md @@ -62,7 +62,7 @@ - `allocateSession` throws `INVALID_DIMENSIONS` for non-positive `cols` or `rows`, `STORAGE_READ_ERROR` when `cwd` is not a directory and `INVALID_INPUT` when the shell path is not a file. An empty `command` defaults to the shell path. It creates the session directory and calls `launch` only after these checks pass. [lifecycle.ts:allocateSession] - `buildReplayInput` validates the manifest with `SessionRecordSchema` and requires its `sessionId` to match. It validates the events with `validateEventRecords` unless `trustValidated` is set. `targetSeq` is -1 for no events, else from 0 to the last `seq`. [replay.ts:buildReplayInput] - `RpcServer.listen` removes a stale socket file (no answer within 1,000 ms, `ECONNREFUSED` or `ENOENT`) and throws when another server is listening. It asserts a socket path of at most 104 characters. [rpcServer.ts:RpcServer] -- `RpcServer` and `sendRpc` cap a message at 1,048,576 characters. The server validates the request with `RpcRequestSchema`, the params and result with `RpcMethodSchemas`, and the response with `RpcResponseSchema` (all from `src/protocol/messages.ts`). Unknown methods get an error response. [rpcServer.ts:RpcServer] [rpcClient.ts:sendRpc] +- `RpcServer` and `sendRpc` cap a message at 1,048,576 characters. The server validates the request with `RpcRequestSchema`, the params and result with `RpcMethodSchemas`, and the response with `RpcResponseSchema` (all from `src/protocol/messages.ts`). A handler that throws a `CliError` gets an error response with its code, message and, when it has any, its `details`. It leaves `details` out when they do not serialize to JSON (a `BigInt`, a circular object) or take more than 65,536 characters, so the response stays under the 1,048,576-character cap and keeps the error's code. `sendRpc` rebuilds that `CliError` with the same details. Any other thrown error becomes `RPC_ERROR` without details. Unknown methods get an error response. [rpcServer.ts:RpcServer] [rpcClient.ts:sendRpc] - `sendRpc` waits 5,000 ms by default. It requires the response `id` to match the request. It maps `ECONNREFUSED`, `ENOENT` and `ECONNRESET` to `HOST_UNREACHABLE`, a socket timeout to `HOST_TIMEOUT` and other transport errors to `RPC_ERROR`. [rpcClient.ts:sendRpc] - `HostRendererManager` runs backend operations one at a time. `withBackend` requires the replay input `sessionId` to match, and recreates the backend when the initial size changes. [renderer.ts:HostRendererManager] - A waited `run` calls `EventLog.startAppend`, `registerWaitedRun` and `pty.write` with no `await` in between, then awaits `written`. When `registerWaitedRun` or `pty.write` throws, it waits for `written` to settle and rethrows that error, so a failed write never becomes an unhandled rejection. So no other input reaches the PTY or the log between the command's `seq` and its write, and the marker is registered before the command can print. [hostMain.ts:runHost] diff --git a/src/host/rpcClient.ts b/src/host/rpcClient.ts index 52e47bdd..6d78dc9e 100644 --- a/src/host/rpcClient.ts +++ b/src/host/rpcClient.ts @@ -91,12 +91,16 @@ function toTransportCliError( }); } -function toResponseCliError(code: string, message: string): CliError { +function toResponseCliError( + code: string, + message: string, + details: Record | undefined, +): CliError { if (isProtocolErrorCode(code)) { - return makeCliError(code, { message }); + return makeCliError(code, { message, ...(details && { details }) }); } - return new CliError(code, message); + return new CliError(code, message, { ...(details && { details }) }); } function isMissingSocket(error: unknown): boolean { @@ -313,7 +317,11 @@ async function sendRpcAt( } settlers.reject( - toResponseCliError(response.error.code, response.error.message), + toResponseCliError( + response.error.code, + response.error.message, + response.error.details, + ), ); } catch (error) { settlers.reject( diff --git a/src/host/rpcServer.ts b/src/host/rpcServer.ts index ee2e1553..f4cc0962 100644 --- a/src/host/rpcServer.ts +++ b/src/host/rpcServer.ts @@ -19,6 +19,7 @@ import { ResourceScope } from '../util/resourceScope.js'; const MAX_UNIX_SOCKET_PATH = 104; const MAX_RPC_BUFFER_BYTES = 1_048_576; +const MAX_RPC_ERROR_DETAILS_CHARS = 65_536; const SOCKET_LIVENESS_PROBE_TIMEOUT_MS = 1_000; const UNKNOWN_REQUEST_ID = 'unknown'; @@ -138,13 +139,32 @@ function buildErrorResponse(id: string, message: string): RpcResponse { return responseResult.data; } +function sendableDetails( + details: Record, +): Record | undefined { + if (Object.keys(details).length === 0) { + return undefined; + } + let serialized: string; + try { + serialized = JSON.stringify(details); + } catch { + return undefined; + } + return serialized.length <= MAX_RPC_ERROR_DETAILS_CHARS + ? (JSON.parse(serialized) as Record) + : undefined; +} + function buildCliErrorResponse(id: string, error: CliError): RpcResponse { + const details = sendableDetails(error.details); const response = { id, ok: false, error: { code: error.code, message: error.message, + ...(details === undefined ? {} : { details }), }, } as const; const responseResult = RpcResponseSchema.safeParse(response); diff --git a/src/protocol/README.md b/src/protocol/README.md index 1fb4448a..04d6fd22 100644 --- a/src/protocol/README.md +++ b/src/protocol/README.md @@ -35,6 +35,7 @@ - `WaitForRenderParamsSchema` requires at least one of `text`, `regex`, `screenStableMs`, `cursorRow` or `cursorCol`. `text` and `regex` are mutually exclusive, and scope `cursor-line` needs `text` or `regex`. [schemas.ts:WaitForRenderParamsSchema] - `RecordDiffResultSchema` checks that `identical` matches the equality of the two screen hashes. An identical result must have an empty diff and equal side row counts. A side with `capturedAtSeq` -1 must hash to a blank screen. For a non-identical result, the rows rebuilt from the diff must hash to each side's `screenHash`. [schemas.ts:RecordDiffResultSchema] - `RpcRequestSchema` requires non-empty `id` and `method`. It does not restrict `method` to `RpcMethodSchema`. [messages.ts:RpcRequestSchema] +- `RpcErrorSchema` requires non-empty `code` and `message`, and allows an optional `details` record. A CLI from before `details` was added rejects an error response that has it, so that CLI reports `RPC_ERROR` (`retryable: true`) for such an error from a newer host, and retrying does not help. A newer CLI reads an older host's error responses. [messages.ts:RpcErrorSchema] - `RpcMethodSchemas` uses `satisfies Record`, so TypeScript requires one entry per method. [messages.ts:RpcMethodSchemas] - `RunParamsSchema` defaults `noWait` to `false`. [messages.ts:RunParamsSchema] - `MouseInputSchema` and `MouseParamsSchema` require `button` for `press` and `release`, reject it for `move`, and reject `release` of a wheel button. `InputMouseEventPayloadSchema` applies the first two rules and requires `dataBase64`, which is empty for a suppressed report. [messages.ts:MouseInputSchema] [schemas.ts:InputMouseEventPayloadSchema] diff --git a/src/protocol/messages.ts b/src/protocol/messages.ts index 0f71ba6d..d67134d8 100644 --- a/src/protocol/messages.ts +++ b/src/protocol/messages.ts @@ -82,6 +82,7 @@ export const RpcErrorSchema = z .object({ code: NonEmptyStringSchema, message: NonEmptyStringSchema, + details: z.record(z.string(), z.unknown()).optional(), }) .strict(); export type RpcError = z.infer; diff --git a/test/integration/screenshot-out.test.ts b/test/integration/screenshot-out.test.ts index 78f435bf..725bff89 100644 --- a/test/integration/screenshot-out.test.ts +++ b/test/integration/screenshot-out.test.ts @@ -197,6 +197,7 @@ describe('screenshot --out', { timeout: 120_000 }, () => { const expectedError = { code: 'STORAGE_WRITE_ERROR', message: `Artifacts path ${artifactsPath} is not a folder.`, + details: { path: artifactsPath }, }; const live = screenshot([]); diff --git a/test/unit/host/rpcServer.test.ts b/test/unit/host/rpcServer.test.ts index 37e653ea..57c81666 100644 --- a/test/unit/host/rpcServer.test.ts +++ b/test/unit/host/rpcServer.test.ts @@ -5,7 +5,10 @@ import net from 'node:net'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import type { CliError } from '../../../src/cli/errors.js'; +import { sendRpc } from '../../../src/host/rpcClient.js'; import { RpcServer } from '../../../src/host/rpcServer.js'; +import { ERROR_CODES, makeCliError } from '../../../src/protocol/errors.js'; let tempDir = ''; @@ -58,3 +61,91 @@ describe('RpcServer request abort handling', () => { } }); }); + +describe('RpcServer CliError responses', () => { + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), 'agent-tty-rpc-server-')); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); + tempDir = ''; + }); + + async function rejectionFrom(handlerError: Error): Promise { + const socketFile = join(tempDir, 'rpc.sock'); + const server = new RpcServer(socketFile, { + inspect: () => Promise.reject(handlerError), + }); + await server.listen(); + try { + return await sendRpc(socketFile, 'inspect', {}, 5_000).catch( + (error: unknown) => error, + ); + } finally { + await server.close(); + } + } + + it('carries the code, message and details of a CliError to the client', async () => { + const error = await rejectionFrom( + makeCliError(ERROR_CODES.STORAGE_WRITE_ERROR, { + message: 'Artifacts path /run/artifacts is not a folder.', + details: { path: '/run/artifacts', errno: 'ELOOP' }, + }), + ); + + expect(error).toMatchObject({ + code: 'STORAGE_WRITE_ERROR', + message: 'Artifacts path /run/artifacts is not a folder.', + details: { path: '/run/artifacts', errno: 'ELOOP' }, + }); + }); + + it.each<[string, () => Record]>([ + [ + 'too large for one response', + () => ({ + issues: Array.from({ length: 50_000 }, (_, index) => ({ + path: ['artifacts', index], + message: 'Required', + })), + }), + ], + ['a BigInt', () => ({ size: 1n })], + [ + 'a circular object', + () => { + const details: Record = {}; + details.self = details; + return details; + }, + ], + ])( + 'keeps the code and message but drops details that are %s', + async (_case, makeDetails) => { + const error = await rejectionFrom( + makeCliError(ERROR_CODES.MANIFEST_VALIDATION_ERROR, { + message: 'Artifact manifest is invalid.', + details: makeDetails(), + }), + ); + + expect(error).toMatchObject({ + code: 'MANIFEST_VALIDATION_ERROR', + message: 'Artifact manifest is invalid.', + }); + expect((error as CliError).details).toEqual({}); + }, + ); + + it('gives an error that is not a CliError as RPC_ERROR without details', async () => { + const error = await rejectionFrom(new Error('renderer failed')); + + expect(error).toMatchObject({ + code: 'RPC_ERROR', + message: 'renderer failed', + details: {}, + }); + }); +});