diff --git a/bun.lock b/bun.lock index 69e3f22d9..14ce1002f 100644 --- a/bun.lock +++ b/bun.lock @@ -9,6 +9,7 @@ "@aws-sdk/client-bedrock-agentcore-control": "^3.1079.0", "@aws-sdk/client-iam": "^3.1080.0", "@inkui-cli/data-table": "^0.2.0", + "@smithy/core": "3.29.3", "@tanstack/react-query": "^5.101.2", "commander": "^15.0.0", "ink": "^7.1.0", diff --git a/package.json b/package.json index a7ecfa854..3f2182a84 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "@aws-sdk/client-bedrock-agentcore-control": "^3.1079.0", "@aws-sdk/client-iam": "^3.1080.0", "@inkui-cli/data-table": "^0.2.0", + "@smithy/core": "3.29.3", "@tanstack/react-query": "^5.101.2", "commander": "^15.0.0", "ink": "^7.1.0", diff --git a/src/errors/errors.test.tsx b/src/errors/errors.test.tsx new file mode 100644 index 000000000..168d02e4e --- /dev/null +++ b/src/errors/errors.test.tsx @@ -0,0 +1,66 @@ +import { describe, test, expect } from "bun:test"; +import { + AccessDeniedException, + ValidationException, + InternalServerException, +} from "@aws-sdk/client-bedrock-agentcore-control"; +import { AgentCoreCLIError, InputValidationError } from "./errors"; + +describe("AgentCoreCLIError", () => { + test("fromError preserves existing AgentCoreCLIError instances", () => { + const err = new AgentCoreCLIError("cli error", { source: "internal" }); + expect(AgentCoreCLIError.fromError(err)).toBe(err); + }); + + test("fromError preserves AgentCoreCLIError subclasses", () => { + const err = new InputValidationError("bad input"); + expect(AgentCoreCLIError.fromError(err)).toBe(err); + }); + + test.each([ + [ + "AccessDeniedException (403)", + new AccessDeniedException({ $metadata: { httpStatusCode: 403 }, message: "" }), + "user", + ], + [ + "ValidationException (400)", + new ValidationException({ + $metadata: { httpStatusCode: 400 }, + message: "", + reason: "FieldValidationFailed", + }), + "user", + ], + [ + "InternalServerException (500)", + new InternalServerException({ $metadata: { httpStatusCode: 500 }, message: "" }), + "service", + ], + [ + "InternalServerException (no status)", + new InternalServerException({ $metadata: {}, message: "" }), + "service", + ], + ])("fromError SDK %s → expected source", (_label, err, expectedSource) => { + const result = AgentCoreCLIError.fromError(err as Error); + expect(result.json()).toMatchObject({ + name: "AgentCoreCLIError", + source: expectedSource, + }); + }); + + test.each([ + ["Error", new Error("plain error"), "plain error"], + ["string", "string error", "string error"], + ["null", null, "null"], + ["undefined", undefined, "undefined"], + ])("fromError non-SDK %s → internal source", (_label, input, expectedMessage) => { + const result = AgentCoreCLIError.fromError(input); + expect(result.json()).toMatchObject({ + name: "AgentCoreCLIError", + source: "internal", + message: expectedMessage, + }); + }); +}); diff --git a/src/errors/errors.tsx b/src/errors/errors.tsx new file mode 100644 index 000000000..86051d52f --- /dev/null +++ b/src/errors/errors.tsx @@ -0,0 +1,66 @@ +import { ServiceException } from "@smithy/core/client"; +import { ERROR_SOURCE, type ErrorSource } from "./types"; + +export interface AgentCoreCLIErrorOptions extends ErrorOptions { + /** The source fo the error. See {@link ErrorSource} for more information */ + source?: ErrorSource; + /** Arbitrary metdata to attach to errors for logging */ + meta?: Record; + /** Describes the exitCode for the CLI when this error hits the root handler */ + exitCode?: number; +} + +/** Base error for CLI failures, including their source, metadata, and process exit code. */ +export class AgentCoreCLIError extends Error { + readonly source: ErrorSource; + readonly meta: Record; + readonly exitCode: number; + + constructor(message?: string, options?: AgentCoreCLIErrorOptions) { + super(message, options); + this.name = new.target.name; + this.source = options?.source ?? ERROR_SOURCE.INTERNAL; + this.meta = options?.meta ?? {}; + this.exitCode = options?.exitCode ?? 1; + } + /** Convert the error into an object with its attributes enumerated as keys **/ + json(): Record { + return { + name: this.name, + message: this.message, + stack: this.stack, + exitCode: this.exitCode, + meta: this.meta, + source: this.source, + }; + } + + static fromError(error: unknown): AgentCoreCLIError { + if (error instanceof AgentCoreCLIError) return error; + + if (ServiceException.isInstance(error)) { + const httpStatusCode = error.$metadata.httpStatusCode; + const source = + httpStatusCode !== undefined && httpStatusCode >= 400 && httpStatusCode < 500 + ? ERROR_SOURCE.USER + : ERROR_SOURCE.SERVICE; + + return new AgentCoreCLIError(error.message, { + cause: error, + source, + meta: { ...error.$metadata }, + }); + } + + if (error instanceof Error) return new AgentCoreCLIError(error.message, { cause: error }); + + return new AgentCoreCLIError(String(error), { cause: error }); + } +} + +/** Error raised for invalid user input. */ +export class InputValidationError extends AgentCoreCLIError { + constructor(message?: string, options?: Omit) { + super(message, { ...options, source: ERROR_SOURCE.USER }); + } +} diff --git a/src/errors/index.tsx b/src/errors/index.tsx new file mode 100644 index 000000000..5f96f8766 --- /dev/null +++ b/src/errors/index.tsx @@ -0,0 +1 @@ +export { AgentCoreCLIError, InputValidationError } from "./errors"; diff --git a/src/errors/types.tsx b/src/errors/types.tsx new file mode 100644 index 000000000..f8fef1509 --- /dev/null +++ b/src/errors/types.tsx @@ -0,0 +1,9 @@ +export const ERROR_SOURCE = { + // note: this maps to the `client` error source in telemetry. + INTERNAL: "internal", + USER: "user", + SERVICE: "service", +} as const; + +/** Describes the source of the error, whether it was the user, internal to the CLI, a service, or unknown. */ +export type ErrorSource = (typeof ERROR_SOURCE)[keyof typeof ERROR_SOURCE]; diff --git a/src/index.ts b/src/index.ts index 348fb81ca..b36ac6789 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ import { FsReadWriteJson } from "./io"; import { createFileLogger, LOG_LEVEL } from "./logging"; import { runWithExitCode } from "./runnable"; import { DefaultGlobalConfigAccessor } from "./globalConfig"; +import { AgentCoreCLIError } from "./errors"; process.exit( await runWithExitCode(async (argv: string[]) => { @@ -65,11 +66,9 @@ process.exit( // Handle the request await rootHandler.route(argv); } catch (e) { - const error = e instanceof Error ? e : new Error(String(e)); - rootLogger - .child({ errorName: error.name, errorMessage: error.message, stack: error.stack ?? "" }) - .error(); - throw e; + const error = AgentCoreCLIError.fromError(e); + rootLogger.child({ error: error.json() }).error(); + throw error; } finally { await rootLogger.end(); } diff --git a/src/middleware/withLogging.test.ts b/src/middleware/withLogging.test.ts index 68e56af0d..94359f865 100644 --- a/src/middleware/withLogging.test.ts +++ b/src/middleware/withLogging.test.ts @@ -62,7 +62,7 @@ describe("withLogging", () => { ]); }); - test("logs success and error with correct command path bindings", async () => { + test("logs success with the correct command path binding", async () => { const app = new Router("myapp", "test app"); app.use(withLogging({ logger })); app.handler( @@ -72,33 +72,13 @@ describe("withLogging", () => { handle: async () => {}, }), ); - app.handler( - createHandler({ - name: "boom", - description: "throws", - handle: async () => { - throw new Error("connection timeout"); - }, - }), - ); - await app.route(["node", "myapp", "happy"]); - await app.route(["node", "myapp", "boom"]).catch(() => {}); await app.route(["node", "myapp", "happy"]); await assertLogsMatch(tempDir, [ { - filter: (l: any) => - l.msg === "command executed successfully" && l.commandPath === "/myapp/happy", - expectedCount: 2, - }, - { - filter: (l: any) => - l.level === "error" && - l.msg === "command failed" && - l.errorName === "Error" && - l.errorMessage === "connection timeout" && - l.commandPath === "/myapp/boom", + filter: (log: any) => + log.msg === "command executed successfully" && log.commandPath === "/myapp/happy", expectedCount: 1, }, ]); diff --git a/src/middleware/withLogging.tsx b/src/middleware/withLogging.tsx index 2b89084de..3e95e1983 100644 --- a/src/middleware/withLogging.tsx +++ b/src/middleware/withLogging.tsx @@ -25,7 +25,7 @@ function redactSensitiveFlags( /** * Middleware that creates a child logger bound to the current command path - * and logs execution start, success, and failure. + * and logs execution start and success. * * @param config - Contains the root {@link Logger} to derive children from. */ @@ -39,18 +39,11 @@ export function withLogging(config: WithLoggingConfig): Middleware { handle: async (ctx, flags, args) => { const commandPath = ctx.require(PathKey); const logger = config.logger.child({ commandPath }); - try { - const safeFlags = redactSensitiveFlags(flags, h.flags()); - logger.child({ flags: safeFlags, args }).debug("executing command"); - await h.handle(ctx.withValue(LoggerKey, logger), flags, args); - logger.debug("command executed successfully"); - } catch (err) { - const error = err instanceof Error ? err : new Error(String(err)); - logger - .child({ errorName: error.name, errorMessage: error.message, stack: error.stack ?? "" }) - .error("command failed"); - throw err; - } + const safeFlags = redactSensitiveFlags(flags, h.flags()); + + logger.child({ flags: safeFlags, args }).debug("executing command"); + await h.handle(ctx.withValue(LoggerKey, logger), flags, args); + logger.debug("command executed successfully"); }, }); } diff --git a/src/runnable/index.test.ts b/src/runnable/index.test.ts index e9d58ec1d..f756a6e62 100644 --- a/src/runnable/index.test.ts +++ b/src/runnable/index.test.ts @@ -1,8 +1,9 @@ -import { test, expect } from "bun:test"; +import { expect, test } from "bun:test"; -import { runRunnable, runWithExitCode, ExitCode, type Runnable } from "./index.tsx"; +import { AgentCoreCLIError } from "../errors"; +import { runRunnable, type Runnable } from "./index.tsx"; -test("returns SUCCESS and forwards argv when run completes", async () => { +test("returns zero and forwards argv when run completes", async () => { let receivedArgv: string[] | undefined; const runnable: Runnable = { run: async (argv: string[]) => { @@ -13,11 +14,11 @@ test("returns SUCCESS and forwards argv when run completes", async () => { const argv = ["node", "script", "--flag"]; const code = await runRunnable(() => runnable, argv); - expect(code).toBe(ExitCode.SUCCESS); + expect(code).toBe(0); expect(receivedArgv).toEqual(argv); }); -test("returns FAILURE when run rejects with an Error", async () => { +test("returns the default failure code when run rejects with an Error", async () => { const runnable: Runnable = { run: async () => { throw new Error("boom"); @@ -26,18 +27,25 @@ test("returns FAILURE when run rejects with an Error", async () => { const code = await runRunnable(() => runnable, []); - expect(code).toBe(ExitCode.FAILURE); + expect(code).toBe(1); }); -test("returns FAILURE when the factory throws a non-Error value", async () => { +test("returns the default failure code when the factory throws a non-Error value", async () => { const code = await runRunnable(() => { throw "kaboom"; }, []); - expect(code).toBe(ExitCode.FAILURE); + expect(code).toBe(1); }); -test("runWithExitCode returns SUCCESS for a resolving function", async () => { - const code = await runWithExitCode(async () => {}); - expect(code).toBe(ExitCode.SUCCESS); +test("respects custom errors codes from known errors", async () => { + const runnable: Runnable = { + run: async () => { + throw new AgentCoreCLIError("custom failure", { exitCode: 42 }); + }, + }; + + const code = await runRunnable(() => runnable, []); + + expect(code).toBe(42); }); diff --git a/src/runnable/index.tsx b/src/runnable/index.tsx index 51fef1ec6..8f22aa371 100644 --- a/src/runnable/index.tsx +++ b/src/runnable/index.tsx @@ -1,8 +1,4 @@ -// ExitCode provides names for default Unix exit codes. -export enum ExitCode { - SUCCESS = 0, - FAILURE = 1, -} +import { AgentCoreCLIError } from "../errors"; // Runnable can be implemented by any application's main entrypoint. export interface Runnable { @@ -13,7 +9,7 @@ export interface Runnable { export function runRunnable( createRunnable: () => Runnable, argv: string[] = process.argv, -): Promise { +): Promise { return runWithExitCode(async () => { await createRunnable().run(argv); }); @@ -23,13 +19,14 @@ export function runRunnable( export async function runWithExitCode( fn: (argv: string[]) => Promise, argv: string[] = process.argv, -): Promise { +): Promise { try { await fn(argv); - return ExitCode.SUCCESS; + return 0; } catch (e) { const error = e instanceof Error ? e : new Error(String(e)); - console.error(`${error.name}: ${error.message}`); - return ExitCode.FAILURE; + console.error(`Error: ${error.message}`); + + return error instanceof AgentCoreCLIError ? error.exitCode : 1; } }