diff --git a/packages/@cdktn/cli-core/src/lib/models/terraform-cli.ts b/packages/@cdktn/cli-core/src/lib/models/terraform-cli.ts index 834a312bd..fac188f73 100644 --- a/packages/@cdktn/cli-core/src/lib/models/terraform-cli.ts +++ b/packages/@cdktn/cli-core/src/lib/models/terraform-cli.ts @@ -426,7 +426,14 @@ export class TerraformCli implements Terraform { state.event.exitCode !== 0 && !state.context.cancelled // don't fail if we cancelled the run ) { - throw `Invoking Terraform CLI failed with exit code ${state.event.exitCode}`; + // Plain Error (not Errors.External): a non-zero terraform exit is + // exactly the kind of failure debug collection exists for, and + // Errors.External's constructor fires an un-awaited telemetry POST + // (see commons/src/errors.ts's reportPrefixedError) that a terraform + // failure in this fork has no business sending to HashiCorp. + throw new Error( + `Invoking Terraform CLI failed with exit code ${state.event.exitCode}`, + ); } return { cancelled: Boolean(state.context.cancelled) }; diff --git a/packages/@cdktn/cli-core/src/test/lib/cdktf-project.test.ts b/packages/@cdktn/cli-core/src/test/lib/cdktf-project.test.ts index 2e6399b10..830d4b169 100644 --- a/packages/@cdktn/cli-core/src/test/lib/cdktf-project.test.ts +++ b/packages/@cdktn/cli-core/src/test/lib/cdktf-project.test.ts @@ -421,7 +421,7 @@ describeIfDistExists(__dirname)("CdktfProject", () => { throw new Error("This error should not be thrown"); } catch (e) { expect(e).toMatchInlineSnapshot( - `"Invoking Terraform CLI failed with exit code 1"` + `[Error: Invoking Terraform CLI failed with exit code 1]` ); } diff --git a/packages/cdktn-cli/src/bin/__tests__/error-handling.integration.test.ts b/packages/cdktn-cli/src/bin/__tests__/error-handling.integration.test.ts new file mode 100644 index 000000000..323f9bdc2 --- /dev/null +++ b/packages/cdktn-cli/src/bin/__tests__/error-handling.integration.test.ts @@ -0,0 +1,188 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +// +// Child-process smoke test for runCli(): bundles a tiny fixture with esbuild +// (already a devDependency) and runs it as a real, separate Node process, so +// this exercises the actual unhandled-rejection behaviour of the runtime +// rather than a mocked one. Deliberately NOT dist-gated (no prebuilt CLI, no +// terraform, no network) so it runs in normal CI. Bundling takes well under +// a second. +import * as fs from "fs"; +import * as http from "http"; +import type { AddressInfo } from "net"; +import * as os from "os"; +import * as path from "path"; +import * as esbuild from "esbuild"; +import execa from "execa"; + +function fixtureSource(errorHandlingPath: string): string { + return ` + import yargs from "yargs"; + import * as Sentry from "@sentry/node"; + import { runCli } from ${JSON.stringify(errorHandlingPath)}; + + if (process.argv.includes("--with-listener")) { + // stands in for Sentry's OnUnhandledRejection integration: if runCli() + // ever orphans a rejection, this would be the thing that catches it. + process.on("unhandledRejection", (reason) => { + console.error("stub-sentry-caught:", reason); + }); + } + + // Points the SDK at a local sink instead of sentry.io, standing in for + // the initializErrorReporting() call every real command handler makes + // (see cli-core's error-reporting.ts) before runCli() can ever report. + if (process.env.TEST_SENTRY_DSN) { + Sentry.init({ dsn: process.env.TEST_SENTRY_DSN, autoSessionTracking: false }); + } + + const cli = yargs(process.argv.slice(2).filter((a) => a !== "--with-listener")) + .exitProcess(false) + .command( + "rawboom", + "throws an async raw string", + () => {}, + async () => { + throw "raw-string-message"; + }, + ) + .command( + "capturedboom", + "throws an async Error that should reach Sentry", + () => {}, + async () => { + throw new Error("empirical-sentry-message"); + }, + ); + + void runCli(cli); +`; +} + +// A minimal stand-in for Sentry's ingest endpoint: records every request it +// receives so the test can assert an actual envelope was delivered over the +// wire, not just that some in-process function was called. +function startSentrySink(): Promise<{ + port: number; + requests: () => { url: string; body: string }[]; + close: () => Promise; +}> { + const requests: { url: string; body: string }[] = []; + return new Promise((resolve) => { + const server = http.createServer((req, res) => { + const chunks: Buffer[] = []; + req.on("data", (chunk) => chunks.push(chunk)); + req.on("end", () => { + requests.push({ + url: req.url || "", + body: Buffer.concat(chunks).toString("utf8"), + }); + res.writeHead(200, { "Content-Type": "application/json" }); + res.end("{}"); + }); + }); + server.listen(0, "127.0.0.1", () => { + const { port } = server.address() as AddressInfo; + resolve({ + port, + requests: () => requests, + close: () => new Promise((res) => server.close(() => res())), + }); + }); + }); +} + +describe("runCli child-process smoke test", () => { + let bundlePath: string; + + beforeAll(async () => { + const tmpDir = fs.mkdtempSync( + path.join(os.tmpdir(), "cdktn-error-handling-fixture-"), + ); + const fixturePath = path.join(tmpDir, "fixture.ts"); + bundlePath = path.join(tmpDir, "fixture.bundle.js"); + + const errorHandlingPath = path + .resolve(__dirname, "../error-handling.ts") + .replace(/\.ts$/, ""); + fs.writeFileSync(fixturePath, fixtureSource(errorHandlingPath)); + + // The fixture lives under os.tmpdir(), which has no node_modules + // ancestry of its own, so bare-specifier resolution for its direct + // imports (yargs, @sentry/node) needs a hand-rolled alias. Everything + // error-handling.ts itself imports (yargs, @sentry/node, @cdktn/commons) + // resolves normally, because that file's real path is inside the + // workspace; aliasing both here just pins the fixture's copy to the + // same resolved module, so there's one Sentry client instance, not two. + await esbuild.build({ + entryPoints: [fixturePath], + bundle: true, + platform: "node", + format: "cjs", + outfile: bundlePath, + alias: { + yargs: require.resolve("yargs"), + "@sentry/node": require.resolve("@sentry/node"), + }, + }); + }); + + it("prints a raw-string handler rejection exactly once and never crashes the process", async () => { + const result = await execa(process.execPath, [bundlePath, "rawboom"], { + reject: false, + }); + const output = `${result.stdout}\n${result.stderr}`; + + expect(result.exitCode).toBe(1); + expect(output.split("raw-string-message").length - 1).toBe(1); + expect(output).not.toContain("ERR_UNHANDLED_REJECTION"); + expect(output).not.toContain("UnhandledPromiseRejection"); + expect(output).not.toContain("PromiseRejectionHandledWarning"); + expect(output).not.toContain("Node.js v"); + }); + + it("still orphans nothing when a Sentry-style unhandledRejection listener is installed", async () => { + const result = await execa( + process.execPath, + [bundlePath, "rawboom", "--with-listener"], + { reject: false }, + ); + const output = `${result.stdout}\n${result.stderr}`; + + expect(result.exitCode).toBe(1); + expect(output.split("raw-string-message").length - 1).toBe(1); + expect(output).not.toContain("stub-sentry-caught"); + expect(output).not.toContain("ERR_UNHANDLED_REJECTION"); + expect(output).not.toContain("UnhandledPromiseRejection"); + expect(output).not.toContain("PromiseRejectionHandledWarning"); + expect(output).not.toContain("Node.js v"); + }); + + it("delivers the crash to Sentry before the process exits", async () => { + const sink = await startSentrySink(); + try { + const dsn = `http://public@127.0.0.1:${sink.port}/1`; + const result = await execa( + process.execPath, + [bundlePath, "capturedboom"], + { env: { ...process.env, TEST_SENTRY_DSN: dsn }, reject: false }, + ); + + // The process only exits after reportFailure() awaits closeSentry(), + // so if the flush actually delivered the event, the sink has already + // seen it by the time execa resolves - there is nothing left to poll. + expect(result.exitCode).toBe(1); + const envelopeRequests = sink + .requests() + .filter((r) => r.url.includes("/envelope/")); + expect(envelopeRequests.length).toBeGreaterThan(0); + expect( + envelopeRequests.some((r) => + r.body.includes("empirical-sentry-message"), + ), + ).toBe(true); + } finally { + await sink.close(); + } + }, 15000); +}); diff --git a/packages/cdktn-cli/src/bin/__tests__/error-handling.test.ts b/packages/cdktn-cli/src/bin/__tests__/error-handling.test.ts new file mode 100644 index 000000000..fcb39250b --- /dev/null +++ b/packages/cdktn-cli/src/bin/__tests__/error-handling.test.ts @@ -0,0 +1,405 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import yargs, { Argv } from "yargs"; +import { Errors } from "@cdktn/commons"; +import { + describeError, + reportFailure, + runCli, + SENTRY_FLUSH_TIMEOUT_MS, + FailureReporterDeps, +} from "../error-handling"; + +// Node schedules the unhandled-rejection bookkeeping for an orphaned promise +// at the end of the tick in which it is orphaned. Two setImmediate turns land +// strictly after that tick, which is what makes "was anything ever orphaned" +// a deterministic observation instead of a timing-dependent one. +async function flushMicroAndMacrotasks() { + await new Promise((r) => setImmediate(r)); + await new Promise((r) => setImmediate(r)); +} + +function allLoggedText(deps: FailureReporterDeps): string[] { + const log = deps.log as jest.Mock; + const logError = deps.logError as jest.Mock; + return [...log.mock.calls, ...logError.mock.calls].map((args) => args[0]); +} + +function makeDeps(): FailureReporterDeps { + return { + log: jest.fn(), + logError: jest.fn(), + collectDebugInformation: jest + .fn() + .mockImplementation( + () => + new Promise((resolve) => setImmediate(() => resolve({ node: "24" }))), + ), + captureException: jest.fn(), + closeSentry: jest.fn().mockResolvedValue(true), + }; +} + +// A fresh, isolated yargs instance per test: `yargs(args)` (the "yargs" +// package's singleton-factory form used by cdktn.ts itself) builds a brand +// new internal instance and returns it directly, so tests don't leak +// commands or `.fail()` handlers into one another. +function makeCli(args: string[], handlers: { ok?: jest.Mock } = {}) { + return yargs(args) + .exitProcess(false) + .command( + "boom", + "throws an async Error", + () => {}, + async () => { + throw new Error("boom-message"); + }, + ) + .command( + "rawboom", + "throws an async raw string", + () => {}, + async () => { + throw "raw-string-message"; + }, + ) + .command( + "syncboom", + "throws synchronously", + () => {}, + () => { + throw new Error("sync-boom-message"); + }, + ) + .command( + "usageboom", + "throws a Usage error", + () => {}, + // async, like real command handlers (e.g. deploy's), so this exercises + // the same handler-rejection path as `boom`/`rawboom` rather than the + // separate yargs-validation path exercised by `choice` below. + async () => { + throw Errors.Usage("bad-usage-message"); + }, + ) + .command( + "externalboom", + "throws an External error", + () => {}, + async () => { + throw Errors.External("bad-external-message"); + }, + ) + .command( + "ok", + "succeeds", + () => {}, + () => { + handlers.ok?.(); + }, + ) + .command( + "choice", + "has an invalid-choice option", + (cmdYargs: Argv) => + cmdYargs.option("language", { + type: "string", + choices: ["typescript", "python"], + }), + () => { + handlers.ok?.(); + }, + ); +} + +async function runAndCapture( + args: string[], + deps: FailureReporterDeps, + handlers: { ok?: jest.Mock } = {}, +) { + const cli = makeCli(args, handlers); + const exitSpy = jest + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never); + const unhandledRejections: unknown[] = []; + const onUnhandled = (reason: unknown) => unhandledRejections.push(reason); + process.on("unhandledRejection", onUnhandled); + + try { + await runCli(cli, deps); + } finally { + await flushMicroAndMacrotasks(); + process.off("unhandledRejection", onUnhandled); + } + + // Capture everything the mock recorded BEFORE restoring it: mockRestore() + // resets mock.calls (it's mockReset() + putting the real implementation + // back), so reading exitSpy.mock.* after this point always reports empty. + const exitCallCount = exitSpy.mock.calls.length; + const lastCall = exitSpy.mock.calls[exitCallCount - 1]; + const exitCode = lastCall ? (lastCall[0] as number | undefined) : undefined; + exitSpy.mockRestore(); + return { exitCode, exitCallCount, unhandledRejections }; +} + +describe("describeError", () => { + it("handles a real Error", () => { + const e = new Error("oops"); + const { message, stack } = describeError(e); + expect(message).toBe("oops"); + expect(stack).toBe(e.stack); + }); + + it("handles a raw string throw", () => { + expect(describeError("just a string")).toEqual({ + message: "just a string", + }); + }); + + it("handles an error-shaped plain object", () => { + expect(describeError({ message: "shaped", stack: "at x" })).toEqual({ + message: "shaped", + stack: "at x", + }); + }); + + it("handles an unrelated plain object", () => { + const { message, stack } = describeError({ foo: "bar" }); + expect(message).toContain('{"foo":"bar"}'); + expect(stack).toBeUndefined(); + }); + + it("handles null", () => { + const { message } = describeError(null); + expect(message).toContain("null"); + }); + + it("handles a circular object without throwing", () => { + const circular: Record = {}; + circular.self = circular; + const { message } = describeError(circular); + expect(typeof message).toBe("string"); + expect(message).toContain("Unexpected non-Error value thrown"); + }); +}); + +describe("reportFailure", () => { + it("prints Usage errors as a single line with no debug info, and never reports them to Sentry", async () => { + const deps = makeDeps(); + const error = Errors.Usage("bad-usage-message"); + const code = await reportFailure({ message: null, error }, deps); + + expect(code).toBe(1); + expect(deps.collectDebugInformation).not.toHaveBeenCalled(); + expect(allLoggedText(deps)).toEqual(["Usage Error: bad-usage-message"]); + // beforeSend (cli-core's error-reporting.ts) drops "Usage Error" anyway; + // don't even try to capture one. + expect(deps.captureException).not.toHaveBeenCalled(); + expect(deps.closeSentry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); + + it("prints External errors as a single line with no debug info, but does report them to Sentry", async () => { + const deps = makeDeps(); + const error = Errors.External("bad-external-message"); + const code = await reportFailure({ message: null, error }, deps); + + expect(code).toBe(1); + expect(deps.collectDebugInformation).not.toHaveBeenCalled(); + expect(allLoggedText(deps)).toEqual([ + "External Error: bad-external-message", + ]); + expect(deps.captureException).toHaveBeenCalledWith(error); + expect(deps.closeSentry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); + + it("reports a message + Error with message, stack, then debug info, and captures it for Sentry before flushing", async () => { + const deps = makeDeps(); + const error = new Error("boom-message"); + const captureOrder: string[] = []; + (deps.captureException as jest.Mock).mockImplementation(() => + captureOrder.push("capture"), + ); + (deps.closeSentry as jest.Mock).mockImplementation(async () => { + captureOrder.push("flush"); + return true; + }); + const code = await reportFailure({ message: null, error }, deps); + + expect(code).toBe(1); + const texts = allLoggedText(deps); + expect(texts.filter((t) => t === "boom-message").length).toBe(1); + expect(texts).toContain(error.stack); + expect(texts).toContain("Collecting Debug Information..."); + expect(texts).toContain("Debug Information:"); + expect(deps.captureException).toHaveBeenCalledWith(error); + // the capture must happen before the flush, or Sentry.close() has + // nothing queued to send. + expect(captureOrder).toEqual(["capture", "flush"]); + expect(deps.closeSentry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); + + it("never prints the literal 'undefined' for a raw-string throw, and still captures it for Sentry", async () => { + const deps = makeDeps(); + const code = await reportFailure( + { message: null, error: "raw-string-message" }, + deps, + ); + + expect(code).toBe(1); + const texts = allLoggedText(deps); + expect(texts).toContain("raw-string-message"); + expect(texts).not.toContain("undefined"); + expect(deps.captureException).toHaveBeenCalledWith("raw-string-message"); + }); + + it("reports a debug-collection failure without masking the original error", async () => { + const deps = makeDeps(); + (deps.collectDebugInformation as jest.Mock).mockRejectedValue( + new Error("debug collection exploded"), + ); + const code = await reportFailure( + { message: null, error: new Error("boom-message") }, + deps, + ); + + expect(code).toBe(1); + const texts = allLoggedText(deps); + expect(texts).toContain("boom-message"); + expect( + texts.some((t) => t.includes("Could not collect debug information")), + ).toBe(true); + expect(deps.closeSentry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); + + it("still flushes Sentry even if captureException itself throws", async () => { + const deps = makeDeps(); + (deps.captureException as jest.Mock).mockImplementation(() => { + throw new Error("sentry client exploded"); + }); + const code = await reportFailure( + { message: null, error: new Error("boom-message") }, + deps, + ); + + expect(code).toBe(1); + expect(deps.closeSentry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); +}); + +describe("runCli", () => { + it("reports an async Error thrown by a command handler exactly once, with no orphaned rejection", async () => { + const deps = makeDeps(); + const { exitCode, exitCallCount, unhandledRejections } = + await runAndCapture(["boom"], deps); + + expect(unhandledRejections).toEqual([]); + expect(exitCode).toBe(1); + expect(exitCallCount).toBe(1); + const texts = allLoggedText(deps); + expect(texts.filter((t) => t === "boom-message").length).toBe(1); + expect(texts).toContain("Collecting Debug Information..."); + expect(texts).toContain("Debug Information:"); + expect(deps.captureException).toHaveBeenCalledTimes(1); + expect(deps.closeSentry).toHaveBeenCalledWith(SENTRY_FLUSH_TIMEOUT_MS); + }); + + it("reports an async raw-string throw without ever printing 'undefined'", async () => { + const deps = makeDeps(); + const { exitCode, unhandledRejections } = await runAndCapture( + ["rawboom"], + deps, + ); + + expect(unhandledRejections).toEqual([]); + expect(exitCode).toBe(1); + const texts = allLoggedText(deps); + expect(texts).toContain("raw-string-message"); + expect(texts).not.toContain("undefined"); + expect(deps.captureException).toHaveBeenCalledWith("raw-string-message"); + }); + + it("reports a synchronous handler throw", async () => { + const deps = makeDeps(); + const { exitCode } = await runAndCapture(["syncboom"], deps); + + expect(exitCode).toBe(1); + expect(allLoggedText(deps)).toContain("sync-boom-message"); + expect(deps.captureException).toHaveBeenCalledTimes(1); + }); + + it("prints Usage errors as one line, skips debug collection, and orphans no rejection", async () => { + const deps = makeDeps(); + const { exitCode, unhandledRejections } = await runAndCapture( + ["usageboom"], + deps, + ); + + expect(unhandledRejections).toEqual([]); + expect(exitCode).toBe(1); + expect(allLoggedText(deps)).toEqual(["Usage Error: bad-usage-message"]); + expect(deps.collectDebugInformation).not.toHaveBeenCalled(); + expect(deps.captureException).not.toHaveBeenCalled(); + }); + + it("prints External errors as one line, skips debug collection, and orphans no rejection, but does report to Sentry", async () => { + const deps = makeDeps(); + const { exitCode, exitCallCount, unhandledRejections } = + await runAndCapture(["externalboom"], deps); + + expect(unhandledRejections).toEqual([]); + expect(exitCode).toBe(1); + expect(exitCallCount).toBe(1); + expect(allLoggedText(deps)).toEqual([ + "External Error: bad-external-message", + ]); + expect(deps.collectDebugInformation).not.toHaveBeenCalled(); + expect(deps.captureException).toHaveBeenCalledTimes(1); + }); + + it("rejects an invalid choice before the command handler runs", async () => { + const deps = makeDeps(); + const handler = jest.fn(); + const { exitCode } = await runAndCapture( + ["choice", "--language=nope"], + deps, + { ok: handler }, + ); + + expect(exitCode).toBe(1); + expect(handler).not.toHaveBeenCalled(); + expect(allLoggedText(deps).some((t) => t.includes("Invalid values"))).toBe( + true, + ); + // yargs' own validation failures pass no `error` object, only a message, + // so this never reaches the generic Error branch that captures for + // Sentry (there'd be nothing usefully identifying to capture anyway). + expect(deps.captureException).not.toHaveBeenCalled(); + }); + + it("does not report a failure or exit on success", async () => { + const deps = makeDeps(); + const handler = jest.fn(); + const { exitCode, exitCallCount, unhandledRejections } = + await runAndCapture(["ok"], deps, { ok: handler }); + + expect(handler).toHaveBeenCalled(); + expect(unhandledRejections).toEqual([]); + expect(exitCallCount).toBe(0); + expect(exitCode).toBeUndefined(); + expect(deps.log).not.toHaveBeenCalled(); + expect(deps.logError).not.toHaveBeenCalled(); + }); + + it("does not report a failure or exit on --help", async () => { + const deps = makeDeps(); + const { exitCallCount, unhandledRejections } = await runAndCapture( + ["--help"], + deps, + ); + + expect(unhandledRejections).toEqual([]); + expect(exitCallCount).toBe(0); + expect(deps.log).not.toHaveBeenCalled(); + expect(deps.logError).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cdktn-cli/src/bin/cdktn.ts b/packages/cdktn-cli/src/bin/cdktn.ts index cbf731d23..553f56bd7 100644 --- a/packages/cdktn-cli/src/bin/cdktn.ts +++ b/packages/cdktn-cli/src/bin/cdktn.ts @@ -6,13 +6,11 @@ import * as yargs from "yargs"; import * as path from "path"; import * as os from "os"; import * as fs from "fs-extra"; -import * as Sentry from "@sentry/node"; import { readCDKTFManifest, - IsErrorType, - collectDebugInformation, CDKTF_DISABLE_PLUGIN_CACHE_ENV, } from "@cdktn/commons"; +import { runCli } from "./error-handling"; import initCmd from "./cmds/init"; import getCmd from "./cmds/get"; import convertCmd from "./cmds/convert"; @@ -83,8 +81,7 @@ const customCompletion = function ( // for the possible overload (which supports falling back to default completions) // https://github.com/yargs/yargs/blob/d33e9972291406490cd8fdad0b3589be234e0f12/lib/completion.ts#L202 -// eslint-disable-next-line @typescript-eslint/no-unused-expressions -yargs +const cli = yargs .command(initCmd) .command(getCmd) .command(convertCmd) @@ -155,32 +152,6 @@ yargs yargs.showHelp(); process.exit(1); }, - }) - .fail(async (message, error) => { - // will not stop the process, but stops further execution of the handler function - // this is called first because yargs is not waiting for this async function - yargs.exit(1, error); - - // set if e.g. the validation of command arguments failed - if (message) { - console.log(message); - } - - // set if e.g. an handler threw an error while being invoked - if (IsErrorType(error, "Usage") || IsErrorType(error, "External")) { - console.error(error.message); - } else if (error) { - console.error(error.message); - console.error(error.stack); - console.error("Collecting Debug Information..."); - const debugOutput = await collectDebugInformation(); - - console.error("Debug Information:"); - Object.entries(debugOutput).forEach(([key, value]) => { - console.log(`${key}: ${value === null ? "null" : value}`); - }); - } + }); - await Sentry.close(4000); - process.exit(1); - }).argv; +void runCli(cli); diff --git a/packages/cdktn-cli/src/bin/cmds/ui/__tests__/deploy.test.ts b/packages/cdktn-cli/src/bin/cmds/ui/__tests__/deploy.test.ts index ade7ad2cd..c19c73d63 100644 --- a/packages/cdktn-cli/src/bin/cmds/ui/__tests__/deploy.test.ts +++ b/packages/cdktn-cli/src/bin/cmds/ui/__tests__/deploy.test.ts @@ -301,8 +301,8 @@ describe("runDeploy --outputs-file write failures are fatal", () => { expect(caught).toBeDefined(); // A bad --outputs-file path is a usage mistake, not something outside our control: it must be - // typed "Usage", not "External" (see cdktn.ts's `.fail()` handler: External/Usage errors print - // just `error.message`, everything else prints message + stack + "Collecting Debug + // typed "Usage", not "External" (see error-handling.ts's `reportFailure`: External/Usage errors + // print just `error.message`, everything else prints message + stack + "Collecting Debug // Information..."; Usage errors are also excluded from Sentry crash reporting). expect(caught.__type).toBe("Usage"); expect(caught.message).toContain("ENOENT: no such file or directory"); diff --git a/packages/cdktn-cli/src/bin/cmds/ui/deploy.ts b/packages/cdktn-cli/src/bin/cmds/ui/deploy.ts index 942862f99..6b41ca8a3 100644 --- a/packages/cdktn-cli/src/bin/cmds/ui/deploy.ts +++ b/packages/cdktn-cli/src/bin/cmds/ui/deploy.ts @@ -195,7 +195,7 @@ export async function runDeploy({ } // A failed --outputs-file write is a broken promise to the user and must be fatal: await it - // and rethrow as a clean error, which cdktn.ts's top-level `.fail()` handler prints as a + // and rethrow as a clean error, which error-handling.ts's `reportFailure` prints as a // single clean line rather than a stack trace, since the deploy itself already succeeded (and // its outputs were already rendered above, so the write failure below does not hide them). // ENOENT/ENOTDIR means the user pointed --outputs-file at a path that doesn't exist - a usage diff --git a/packages/cdktn-cli/src/bin/cmds/ui/output.ts b/packages/cdktn-cli/src/bin/cmds/ui/output.ts index a60399771..ab988522e 100644 --- a/packages/cdktn-cli/src/bin/cmds/ui/output.ts +++ b/packages/cdktn-cli/src/bin/cmds/ui/output.ts @@ -110,7 +110,7 @@ export async function runOutput({ } // See runDeploy() in ./deploy.ts for the rationale: a failed --outputs-file write must be - // fatal, wrapped as a clean error so cdktn.ts's top-level `.fail()` handler prints a single + // fatal, wrapped as a clean error so error-handling.ts's `reportFailure` prints a single // clean line instead of a stack trace, since the outputs were already rendered above. A bad // path (ENOENT/ENOTDIR) is a usage mistake and reported as Usage, not External. try { diff --git a/packages/cdktn-cli/src/bin/error-handling.ts b/packages/cdktn-cli/src/bin/error-handling.ts new file mode 100644 index 000000000..0977fd807 --- /dev/null +++ b/packages/cdktn-cli/src/bin/error-handling.ts @@ -0,0 +1,140 @@ +// Copyright (c) HashiCorp, Inc +// SPDX-License-Identifier: MPL-2.0 +import * as yargs from "yargs"; +import * as Sentry from "@sentry/node"; +import { IsErrorType, collectDebugInformation } from "@cdktn/commons"; + +export type CliFailure = { message?: string | null; error?: unknown }; + +export interface FailureReporterDeps { + log(msg: string): void; // default: console.log + logError(msg: string): void; // default: console.error + collectDebugInformation(): Promise>; + captureException(error: unknown): void; // default: Sentry.captureException + closeSentry(timeoutMs: number): Promise; // default: Sentry.close +} + +export const SENTRY_FLUSH_TIMEOUT_MS = 4000; // unchanged from the previous cdktn.ts handler + +// Non-Error tolerant. Fixes the `undefined`/`undefined` print that came from +// terraform-cli.ts's raw-string throw (see terraform-cli.ts:429). +export function describeError(e: unknown): { message: string; stack?: string } { + if (e instanceof Error) return { message: e.message, stack: e.stack }; + if (typeof e === "string") return { message: e }; + const o = e as { message?: unknown; stack?: unknown } | null; + if (o && typeof o.message === "string") { + return { + message: o.message, + stack: typeof o.stack === "string" ? o.stack : undefined, + }; + } + let rendered: string; + try { + rendered = JSON.stringify(e); + } catch { + rendered = String(e); + } + return { + message: `Unexpected non-Error value thrown: ${rendered ?? String(e)}`, + }; +} + +const defaultDeps: FailureReporterDeps = { + log: (msg) => console.log(msg), + logError: (msg) => console.error(msg), + collectDebugInformation, + captureException: (error) => { + Sentry.captureException(error); + }, + closeSentry: (timeoutMs) => Sentry.close(timeoutMs), +}; + +export async function reportFailure( + f: CliFailure, + deps: FailureReporterDeps, +): Promise { + const { message, error } = f; + try { + if (message) deps.log(message); // yargs validation text + + if (IsErrorType(error, "Usage")) { + // cli-core's error-reporting.ts `beforeSend` drops "Usage Error" + // exceptions on purpose (it's a user mistake, not a crash) - don't + // even bother capturing one just to have it filtered out downstream. + deps.logError((error as Error).message); // one line, NO debug info + } else if (IsErrorType(error, "External")) { + // Unlike Usage, External isn't filtered by `beforeSend`, so it's + // reported: printing one clean line to the terminal doesn't mean the + // failure is uninteresting to us, just that it's not the user's fault. + deps.logError((error as Error).message); // one line, NO debug info + deps.captureException(error); + } else if (error !== undefined && error !== null) { + const { message: m, stack } = describeError(error); + deps.logError(m); + if (stack) deps.logError(stack); + deps.captureException(error); + deps.logError("Collecting Debug Information..."); + try { + const debugOutput = await deps.collectDebugInformation(); + deps.logError("Debug Information:"); + Object.entries(debugOutput).forEach(([key, value]) => + deps.log(`${key}: ${value === null ? "null" : value}`), + ); + } catch (e) { + deps.logError( + `Could not collect debug information: ${describeError(e).message}`, + ); + } + } + } catch (e) { + deps.logError(`Error while reporting failure: ${describeError(e).message}`); + } + + // >>> FORWARD COUPLING (#62): the one awaited telemetry emission goes here, + // >>> immediately before the Sentry flush. Do not add it in this PR. + try { + await deps.closeSentry(SENTRY_FLUSH_TIMEOUT_MS); + } catch { + /* never mask the original error */ + } + return 1; +} + +export function runCli( + y: yargs.Argv, + deps: FailureReporterDeps = defaultDeps, +): Promise { + let failure: CliFailure | undefined; + + y.fail((message, error) => { + // MUST stay synchronous: yargs discards this callback's return value + // (yargs/build/lib/usage.js -> `fail(msg, err, self)`), so any await here + // races Node's unhandled-rejection reporter. See error-handling.test.ts. + // + // With .exitProcess(false) this does NOT exit; it sets yargs' internal + // `hasOutput` flag, which is what prevents the command handler from + // running after a validation failure. Keep it, and keep it first. + y.exit(1, error as Error); + if (!failure) failure = { message, error }; + }); + + return (async () => { + try { + await y.parseAsync(); + } catch (error) { + // Async handler rejections reach here too (yargs rethrows out of parse() + // after .fail ran) — `failure` is already set, so no double report. + // Synchronous handler throws never reach .fail() at all in yargs 17; + // this is the only place that catches them. + if (!failure) failure = { message: null, error }; + } + if (!failure) return; // success / --help / --version + process.exit(await reportFailure(failure, deps)); + })().catch((e) => { + // belt-and-braces: runCli itself must never reject + console.error( + `Fatal error in cdktn error handling: ${describeError(e).message}`, + ); + process.exit(1); + }); +}