From 928afd3607653d2fa83dadb4c11e3faa9601dea6 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:44:13 +0100 Subject: [PATCH] Finalize Yieldstar integration error handling --- packages/cli/src/deploy.ts | 6 +-- packages/cli/src/destroy.ts | 6 +-- packages/cli/src/index.ts | 7 ++- packages/cli/src/plan.ts | 50 +++++++------------ packages/cli/src/run-with-error-handling.ts | 15 +++--- .../cli/test/run-with-error-handling.test.ts | 41 +++++++++++++++ packages/reconciler/src/logger-subscriber.ts | 2 +- .../reconciler/test/logger-subscriber.test.ts | 14 +++++- 8 files changed, 90 insertions(+), 51 deletions(-) create mode 100644 packages/cli/test/run-with-error-handling.test.ts diff --git a/packages/cli/src/deploy.ts b/packages/cli/src/deploy.ts index ec1ff9e..3f141c1 100644 --- a/packages/cli/src/deploy.ts +++ b/packages/cli/src/deploy.ts @@ -7,7 +7,6 @@ import { randomUUID } from "node:crypto"; import { compile } from "./compile"; import { defaultLogger, type Logger } from "./logger"; import { redirectStdoutToStderr } from "./stdio"; -import { runWithCliErrorHandling } from "./run-with-error-handling"; export type DeployCommandOptions = { json?: boolean; @@ -29,8 +28,5 @@ export async function deploy( const executionId = opts.executionId ?? randomUUID(); logger.info(`Yieldstar execution ${executionId}`); - await runWithCliErrorHandling( - () => deployApp({ entryPoint, emit, executionId }), - { logger, command: "deploy" }, - ); + await deployApp({ entryPoint, emit, executionId }); } diff --git a/packages/cli/src/destroy.ts b/packages/cli/src/destroy.ts index 89ea2f1..9352f3c 100644 --- a/packages/cli/src/destroy.ts +++ b/packages/cli/src/destroy.ts @@ -7,7 +7,6 @@ import { randomUUID } from "node:crypto"; import { compile } from "./compile"; import { defaultLogger, type Logger } from "./logger"; import { redirectStdoutToStderr } from "./stdio"; -import { runWithCliErrorHandling } from "./run-with-error-handling"; export type DestroyCommandOptions = { json?: boolean; @@ -29,8 +28,5 @@ export async function destroy( const executionId = opts.executionId ?? randomUUID(); logger.info(`Yieldstar execution ${executionId}`); - await runWithCliErrorHandling( - () => destroyApp({ entryPoint, emit, executionId }), - { logger, command: "destroy" }, - ); + await destroyApp({ entryPoint, emit, executionId }); } diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index b623a63..7ffe504 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -4,6 +4,8 @@ import { compile } from "./compile"; import { deploy } from "./deploy"; import { destroy } from "./destroy"; import { plan } from "./plan"; +import { defaultLogger } from "./logger"; +import { runWithCliErrorHandling } from "./run-with-error-handling"; import { visualise } from "./visualise"; import { watch } from "./watch"; import { startDashboardServer } from "@notation/dashboard"; @@ -77,4 +79,7 @@ program await watch(entryPoint); }); -program.parse(process.argv); +process.exitCode = await runWithCliErrorHandling( + () => program.parseAsync(process.argv), + { logger: defaultLogger, command: process.argv[2] ?? program.name() }, +); diff --git a/packages/cli/src/plan.ts b/packages/cli/src/plan.ts index 960f2f3..35841cc 100644 --- a/packages/cli/src/plan.ts +++ b/packages/cli/src/plan.ts @@ -19,39 +19,27 @@ const decisionSymbols: Record = { export async function plan(entryPoint: string, opts: PlanCommandOptions = {}) { const logger = opts.logger ?? defaultLogger; - try { - if (opts.json) { - let result: Plan; - const { restore } = redirectStdoutToStderr(); - try { - await compile(entryPoint, { logger }); - result = await planApp({ - entryPoint, - }); - } finally { - restore(); - } - process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); - return; + if (opts.json) { + let result: Plan; + const { restore } = redirectStdoutToStderr(); + try { + await compile(entryPoint, { logger }); + result = await planApp({ + entryPoint, + }); + } finally { + restore(); } - - await compile(entryPoint, { logger }); - logger.info(`Planning ${entryPoint}\n`); - const result = await planApp({ - entryPoint, - }); - printPlanSummary(result, logger); - } catch (err: any) { - if (err.name === "CredentialsProviderError") { - logger.error( - "\nAWS credentials not found.", - "\n\nEnsure you have a default profile set up in ~/.aws/credentials.", - "\n\nIf using another profile run AWS_PROFILE=otherProfile notation plan.\n", - ); - process.exit(1); - } - throw err; + process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); + return; } + + await compile(entryPoint, { logger }); + logger.info(`Planning ${entryPoint}\n`); + const result = await planApp({ + entryPoint, + }); + printPlanSummary(result, logger); } function printPlanSummary(result: Plan, logger: Logger) { diff --git a/packages/cli/src/run-with-error-handling.ts b/packages/cli/src/run-with-error-handling.ts index 51c9f8d..85f8f78 100644 --- a/packages/cli/src/run-with-error-handling.ts +++ b/packages/cli/src/run-with-error-handling.ts @@ -1,21 +1,22 @@ import type { Logger } from "./logger"; export async function runWithCliErrorHandling( - fn: () => Promise, + fn: () => Promise, opts: { logger: Logger; command: string }, -): Promise { +): Promise<0 | 1> { try { await fn(); - } catch (err: any) { - if (err.name === "CredentialsProviderError") { + return 0; + } catch (error: unknown) { + if (error instanceof Error && error.name === "CredentialsProviderError") { opts.logger.error( "\nAWS credentials not found.", "\n\nEnsure you have a default profile set up in ~/.aws/credentials.", `\n\nIf using another profile run AWS_PROFILE=otherProfile notation ${opts.command}.\n`, ); - process.exit(1); + return 1; } - opts.logger.error(err); - process.exit(1); + opts.logger.error(error); + return 1; } } diff --git a/packages/cli/test/run-with-error-handling.test.ts b/packages/cli/test/run-with-error-handling.test.ts new file mode 100644 index 0000000..2ca64e9 --- /dev/null +++ b/packages/cli/test/run-with-error-handling.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it, vi } from "vitest"; +import { runWithCliErrorHandling } from "../src/run-with-error-handling"; + +describe("CLI error handling", () => { + it("reports credential failures with command-specific guidance", async () => { + const error = new Error("Could not load credentials"); + error.name = "CredentialsProviderError"; + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + + const exitCode = await runWithCliErrorHandling( + async () => { + throw error; + }, + { logger, command: "deploy" }, + ); + + expect(exitCode).toBe(1); + expect(logger.error).toHaveBeenCalledOnce(); + expect(logger.error).toHaveBeenCalledWith( + "\nAWS credentials not found.", + "\n\nEnsure you have a default profile set up in ~/.aws/credentials.", + "\n\nIf using another profile run AWS_PROFILE=otherProfile notation deploy.\n", + ); + }); + + it("reports non-credential failures unchanged", async () => { + const error = new Error("deploy failed"); + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + + const exitCode = await runWithCliErrorHandling( + async () => { + throw error; + }, + { logger, command: "deploy" }, + ); + + expect(exitCode).toBe(1); + expect(logger.error).toHaveBeenCalledOnce(); + expect(logger.error).toHaveBeenCalledWith(error); + }); +}); diff --git a/packages/reconciler/src/logger-subscriber.ts b/packages/reconciler/src/logger-subscriber.ts index 02ca5b0..e756ff8 100644 --- a/packages/reconciler/src/logger-subscriber.ts +++ b/packages/reconciler/src/logger-subscriber.ts @@ -17,7 +17,7 @@ export function createLoggerReconcilerSubscriber( return; } - if (event.event === "reconciler.orphan-deletion.skipped") { + if (event.level === "warn") { logger.warn(event.event, event); return; } diff --git a/packages/reconciler/test/logger-subscriber.test.ts b/packages/reconciler/test/logger-subscriber.test.ts index 49569dd..721e884 100644 --- a/packages/reconciler/test/logger-subscriber.test.ts +++ b/packages/reconciler/test/logger-subscriber.test.ts @@ -27,6 +27,13 @@ describe("logger reconciler subscriber", () => { resourceId: "resource-2", resourceType: "test/service/subscriber", }); + await emit({ + level: "warn", + event: "reconciler.coordination.waiting", + deploymentId: "deployment-1", + executionId: "execution-2", + holderExecutionId: "execution-1", + }); await emit({ level: "error", event: "reconciler.operation.lifecycle", @@ -39,7 +46,12 @@ describe("logger reconciler subscriber", () => { }); expect(info).toHaveBeenCalledOnce(); - expect(warn).toHaveBeenCalledOnce(); + expect(warn).toHaveBeenCalledTimes(2); + expect(warn).toHaveBeenNthCalledWith( + 2, + "reconciler.coordination.waiting", + expect.objectContaining({ level: "warn" }), + ); expect(error).toHaveBeenCalledOnce(); }); });