Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion src/errors/errors.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@ import {
ValidationException,
InternalServerException,
} from "@aws-sdk/client-bedrock-agentcore-control";
import { AgentCoreCLIError, InputValidationError } from "./errors";
import { CommanderError } from "commander";
import {
AgentCoreCLIError,
InputValidationError,
SilentCLIError,
UserCancellationError,
} from "./errors";

describe("AgentCoreCLIError", () => {
test("fromError preserves existing AgentCoreCLIError instances", () => {
Expand All @@ -17,6 +23,31 @@ describe("AgentCoreCLIError", () => {
expect(AgentCoreCLIError.fromError(err)).toBe(err);
});

test.each([
["parse failures", new CommanderError(1, "commander.invalidArgument", "invalid option"), 2],
["help", new CommanderError(0, "commander.helpDisplayed", "help displayed"), 0],
])("fromError classifies Commander %s", (_label, err, exitCode) => {
const result = AgentCoreCLIError.fromError(err);
expect(result).toBeInstanceOf(SilentCLIError);
expect(result.json()).toMatchObject({
name: "CommanderError",
source: "user",
exitCode,
meta: { code: err.code },
});
});

test("UserCancellationError is a silent user interruption", () => {
const error = new UserCancellationError();
expect(error).toBeInstanceOf(SilentCLIError);
expect(error.json()).toMatchObject({
name: "UserCancellationError",
message: "Operation cancelled by user",
source: "user",
exitCode: 130,
});
});

test.each([
[
"AccessDeniedException (403)",
Expand Down
32 changes: 22 additions & 10 deletions src/errors/errors.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ServiceException } from "@smithy/core/client";
import { CommanderError } from "commander";
import { join } from "node:path";
import { ERROR_SOURCE, type ErrorSource } from "./types";

Expand Down Expand Up @@ -41,6 +42,16 @@ export class AgentCoreCLIError extends Error {
static fromError(error: unknown): AgentCoreCLIError {
if (error instanceof AgentCoreCLIError) return error;

if (error instanceof CommanderError) {
return new SilentCLIError(error.message, {
cause: error,
source: ERROR_SOURCE.USER,
name: error.name,
meta: { code: error.code },
exitCode: error.exitCode === 0 ? 0 : 2,
});
}

if (ServiceException.isInstance(error)) {
const httpStatusCode = error.$metadata.httpStatusCode;
const source =
Expand All @@ -62,6 +73,9 @@ export class AgentCoreCLIError extends Error {
}
}

/** Base for CLI errors intentionally omitted from root stderr output. */
export class SilentCLIError extends AgentCoreCLIError {}

/** Error raised for invalid user input. */
export class InputValidationError extends AgentCoreCLIError {
constructor(message?: string, options?: Omit<AgentCoreCLIErrorOptions, "source">) {
Expand Down Expand Up @@ -135,19 +149,17 @@ export class EmbeddedAssetNotFoundError extends AgentCoreCLIError {
}
}

export class RuntimeInvokeInterruptedError extends AgentCoreCLIError {
readonly reported: boolean;

constructor(cause?: unknown, reported = false) {
super("The operation was aborted", { cause, exitCode: 130 });
this.name = "AbortError";
this.reported = reported;
/** Raised when a user intentionally cancels a headless CLI operation. */
export class UserCancellationError extends SilentCLIError {
constructor() {
super("Operation cancelled by user", {
source: ERROR_SOURCE.USER,
exitCode: 130,
});
}
}

export class RuntimeInvokeResponseError extends AgentCoreCLIError {
readonly reported = true;

export class RuntimeInvokeResponseError extends SilentCLIError {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are runtime invoke responses silent? I thought this was the error we get when the stream parsing fails.

constructor(message: string, cause?: unknown) {
super(message, { cause });
}
Expand Down
3 changes: 2 additions & 1 deletion src/errors/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ export {
NotImplementedError,
ProjectFileExistsError,
ResultTruncationError,
RuntimeInvokeInterruptedError,
RuntimeInvokeResponseError,
SilentCLIError,
SourceResolutionError,
UserCancellationError,
type AgentCoreCLIErrorOptions,
} from "./errors";
export { ERROR_SOURCE } from "./types";
37 changes: 37 additions & 0 deletions src/handlers/eval/dataset/dataset.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {
TestCoreClient,
TestGlobalConfigAccessor,
testIO,
waitFor,
} from "../../../testing";
import { UserCancellationError } from "../../../errors";
import { createRootHandler } from "../../index";
import type { CreateDatasetInput } from "../types";

Expand Down Expand Up @@ -421,6 +423,41 @@ describe("dataset get", () => {
expect(call?.args.slice(0, 3)).toEqual(["dataset-orders-abc123", "2", "/tmp/v2.jsonl"]);
});

test("SIGINT cancels a download with the shared user cancellation error", async () => {
const { core, route } = testDatasetCommand();
core.eval.downloadDataset = async (id, version, filePath, options, signal) => {
core.eval.calls.push({
method: "downloadDataset",
args: [id, version, filePath, options, signal],
});
return new Promise<never>((_, reject) => {
const abort = () => reject(signal?.reason);
if (signal?.aborted) abort();
else signal?.addEventListener("abort", abort, { once: true });
});
};
const pending = route([
"eval",
"dataset",
"get",
"--id",
"dataset-orders-abc123",
"--file-path",
"/tmp/out.jsonl",
]);

try {
await waitFor(() => core.eval.calls.some((call) => call.method === "downloadDataset"));
process.emit("SIGINT", "SIGINT");

const signal = core.eval.calls[0]!.args[4] as AbortSignal;
expect(signal.reason).toBeInstanceOf(UserCancellationError);
await expect(pending).rejects.toBe(signal.reason);
} finally {
await pending.catch(() => undefined);
}
});

test("requires --id", async () => {
const { core, route } = testDatasetCommand();

Expand Down
4 changes: 2 additions & 2 deletions src/handlers/eval/dataset/get/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import z from "zod";
import { createHandler, flag } from "../../../../router";
import { InputValidationError } from "../../../../errors";
import { InputValidationError, UserCancellationError } from "../../../../errors";
import { JsonRendererKey } from "../../../../tui";
import type { Core } from "../../../types";
import { coreOptsFromCtx } from "../../../utils";
Expand Down Expand Up @@ -37,7 +37,7 @@ export const createGetDatasetHandler = (core: Core) =>

// --file-path downloads the contents via the presigned download URL in metadata
const controller = new AbortController();
const interrupt = () => controller.abort();
const interrupt = () => controller.abort(new UserCancellationError());
process.once("SIGINT", interrupt);
try {
const response = await core.eval.downloadDataset(
Expand Down
9 changes: 3 additions & 6 deletions src/handlers/runtime/invoke/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import z from "zod";
import {
InputValidationError,
InvalidEnvironmentError,
RuntimeInvokeInterruptedError,
UserCancellationError,
} from "../../../errors";
import { createHandler, flag, PathKey } from "../../../router";
import type { AppIO } from "../../../io";
Expand Down Expand Up @@ -119,7 +119,7 @@ export const createInvokeRuntimeHandler = (core: Core, io: AppIO) =>
throw new InputValidationError("--json cannot be used with --output-file");
}
const controller = new AbortController();
const interrupt = () => controller.abort();
const interrupt = () => controller.abort(new UserCancellationError());
process.once("SIGINT", interrupt);
try {
const applicationHeaders = parseRuntimeInvokeHeaders(flags.header);
Expand Down Expand Up @@ -158,10 +158,7 @@ export const createInvokeRuntimeHandler = (core: Core, io: AppIO) =>
signal: controller.signal,
});
} catch (error) {
if (controller.signal.aborted && (error as Error)?.name === "AbortError") {
if (error instanceof RuntimeInvokeInterruptedError) throw error;
throw new RuntimeInvokeInterruptedError(error);
}
controller.signal.throwIfAborted();
throw error;
} finally {
controller.abort();
Expand Down
73 changes: 65 additions & 8 deletions src/handlers/runtime/invoke/invoke.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
waitFor,
} from "../../../testing";
import { ExitCode, runWithExitCode } from "../../../runnable";
import { InvalidEnvironmentError } from "../../../errors";
import { InvalidEnvironmentError, UserCancellationError } from "../../../errors";
import { createRootHandler } from "../../index";
import * as tui from "../../../tui";
import { RuntimeInvokeLaunchContextKey } from "./launchContext";
Expand Down Expand Up @@ -247,6 +247,64 @@ describe("runtime invoke", () => {
expect((invoke.args[0] as RuntimeInvokeRequest).payload).toEqual(new Uint8Array());
});

test("SIGINT cancels payload stdin resolution with the typed reason", async () => {
const core = new TestCoreClient();
const output = captureIO();
const initialListeners = process.listenerCount("SIGINT");
const pending = runCommand(core, output.io, [
"runtime",
"invoke",
"--id",
RUNTIME_ID,
"--payload",
"-",
]);

try {
await waitFor(() => process.listenerCount("SIGINT") > initialListeners);
process.emit("SIGINT", "SIGINT");

await expect(pending).rejects.toBeInstanceOf(UserCancellationError);
expect(core.runtime.calls).toEqual([]);
} finally {
await pending.catch(() => undefined);
}
});

test("SIGINT replaces a raw Runtime lookup abort with the typed reason", async () => {
const core = new TestCoreClient();
const output = captureIO();
core.runtime.getRuntime = async (id, options, signal) => {
core.runtime.calls.push({ method: "getRuntime", args: [id, options, signal] });
return new Promise<never>((_, reject) => {
const abort = () =>
reject(Object.assign(new Error("lookup aborted"), { name: "AbortError" }));
if (signal?.aborted) abort();
else signal?.addEventListener("abort", abort, { once: true });
});
};
const pending = runCommand(core, output.io, [
"runtime",
"invoke",
"--id",
RUNTIME_ID,
"--payload",
"{}",
]);

try {
await waitFor(() => core.runtime.calls.some((call) => call.method === "getRuntime"));
process.emit("SIGINT", "SIGINT");

const signal = core.runtime.calls[0]!.args[2] as AbortSignal;
expect(signal.reason).toBeInstanceOf(UserCancellationError);
await expect(pending).rejects.toBe(signal.reason);
expect(core.runtime.calls.map((call) => call.method)).toEqual(["getRuntime"]);
} finally {
await pending.catch(() => undefined);
}
});

test("SIGINT aborts an active headless invocation after preserving emitted bytes", async () => {
const core = new TestCoreClient();
const output = captureIO();
Expand Down Expand Up @@ -285,14 +343,14 @@ describe("runtime invoke", () => {
process.emit("SIGINT", "SIGINT");

expect(signal!.aborted).toBe(true);
await expect(pending).rejects.toMatchObject({ name: "AbortError" });
await expect(pending).rejects.toBeInstanceOf(UserCancellationError);
expect(output.bytes().toString()).toBe("partial");
} finally {
await pending.catch(() => undefined);
}
});

test("wraps a raw Core abort after SIGINT", async () => {
test("replaces a raw Core abort with the typed SIGINT reason", async () => {
const core = new TestCoreClient();
const output = captureIO();
const rawAbort = Object.assign(new Error("transport aborted"), { name: "AbortError" });
Expand All @@ -318,11 +376,10 @@ describe("runtime invoke", () => {
await waitFor(() => core.runtime.calls.some((call) => call.method === "invokeRuntime"));
process.emit("SIGINT", "SIGINT");

await expect(pending).rejects.toMatchObject({
name: "AbortError",
cause: rawAbort,
reported: false,
});
const signal = core.runtime.calls.find((call) => call.method === "invokeRuntime")!
.args[2] as AbortSignal;
expect(signal.reason).toBeInstanceOf(UserCancellationError);
await expect(pending).rejects.toBe(signal.reason);
} finally {
await pending.catch(() => undefined);
}
Expand Down
Loading
Loading