fix(cli): fail cleanly when the --outputs-file write fails

[so0k]

Related issue

No GitHub issue — found while reviewing #375 (the outputs-rendering crash reported via Sentry as CDKTN-5). It is
a separate crash on a different code path, which is why it is split out rather than folded in: the
CDKTN-5 reporter never passed --outputs-file, so none of this is on that crash's path.

Stacked on #375. This PR targets fix/deploy-outputs-undefined-crash, so the diff shown here is only this
change. Review #375 first.

Description

A failed --outputs-file write after an otherwise successful deploy crashed the CLI with a raw stack trace —
having first told the user the file had been written.

Root cause

DeployConfig.onOutputsRetrieved is typed (outputs) => void. A void return type silently accepts an
async function, so TypeScript never flagged that handlers.ts wires it to the async saveOutputs while
runDeploy called it without await:

onOutputsRetrieved(outputs);   // returns a promise nobody holds

A write failure therefore became a floating rejected promise that no catch could observe. runDeploy resolved
normally, execution carried on to print The outputs have been written to <path>, and the process then died on
the unhandled rejection.

This was verified by probe rather than by reading: with an async-throwing callback the catch never fires and
runDeploy resolves; with a sync-throwing one it fires normally.

This is not a success → failure change

The obvious reviewer question is whether making this fatal changes behaviour. It does not — a failing
--outputs-file write has always been fatal, just via an ugly crash. Measured against published CLIs with a
deploy whose outputs-file write fails:

CLI	Exit code	Stack trace	False "written to" line
cdktf-cli@0.21.0 (last cdktf release)	7	yes	yes
cdktn-cli@0.23.4	7	yes	yes
cdktn-cli@0.24.0	1	yes	yes

No version exits 0. The 7 on the two older ones is Node's "Internal Exception Handler Run-Time Failure" —
the Sentry uncaughtException handler faulting during Ink teardown; both ship ink/yoga-layout-prebuilt,
0.24.0 ships neither. Relative to 0.24.0 the exit code is unchanged; only the message improves. All three
printed the false success line before crashing. Reproduced on macOS and on Linux (Ubuntu, Node 22).

Since the strict/lenient decision was effectively already made, no new flag or opt-in is introduced. That would
also cut against the repo's convention, where leniency is opt-in (skip-synth, skip-provider-lock,
ignore-missing-stack-dependencies, force) rather than strictness.

The fix

The callback is awaited, and guarded separately from the rendering, because the two failures deserve
different treatment:

• A failed outputs-file write stays fatal. The user explicitly asked for that file; exiting 0 would let a
  pipeline consume a stale or absent one. It is wrapped as an External error, which cdktn.ts's .fail()
  prints as a single clean line instead of a stack trace and a crash report.
• A failure only rendering the outputs table remains non-fatal, since the deploy and the write have both
  already succeeded by then.

Splitting the guards also means a failing write no longer skips the outputs display, and the written to line is
only reachable once the write has actually succeeded.

cdktn output --outputs-file had the identical fire-and-forget defect and gets the same treatment. watch never
uses this callback.

Note for reviewers

Errors.External fires an unawaited telemetry POST to checkpoint-api.hashicorp.com — every Errors.* factory
does. So this change adds two call sites that report a purely local file-write failure to HashiCorp. The
alternative (not using the helper) means a stack trace plus a Sentry report, which is worse, so External is the
right trade here. Flagging it because it is a live data point for #62, which replaces that transport.

Checklist

• I have updated the PR title to match CDKTN's style guide
• I have run the linter on my code locally
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation if applicable — n/a, the exit-code contract is unchanged
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works if applicable
• New and existing unit tests pass locally with my changes

[sakul-learning]

Verdict: APPROVE — clean fix, well-scoped, good tests, all checks green.

What this fixes

A failed --outputs-file write after a successful deploy crashes the CLI with a raw stack trace — but only after printing the false "The outputs have been written to..." line. The root cause: DeployConfig.onOutputsRetrieved in packages/cdktn-cli/src/bin/cmds/ui/deploy.ts is typed (outputs) => void, so TypeScript silently accepts an async function, and the returned promise is never awaited. A write failure becomes a floating unhandled rejection that runDeploy never catches.

What changed

• handlers.ts — type annotation: void → void | Promise<void> for onOutputsRetrieved (both deploy and output paths)
• deploy.ts — awaits onOutputsRetrieved, wraps failures as Errors.External, moves the "written to" line inside the success path only
• output.ts — identical treatment for runOutput, plus a callback simplification in the runCdktfProject call (the old code fetched outputs, called onOutputsRetrieved, then returned — the new code returns the fetch promise directly and awaits onOutputsRetrieved afterward; same ordering, better error typing)
• deploy.test.ts — two behavioral tests: (1) verifies runDeploy rejects with __type === "External" on async write failure rather than resolving silently, (2) verifies the false success message is not printed

Correctness

✅ The callback refactoring in output.ts is behaviorally equivalent.
✅ Error wrapping is identical between deploy.ts and output.ts.
✅ Errors.External maps to the cdktn.ts .fail() handler that prints a single clean line instead of a stack trace.
✅ Non-Error rejection values handled (e instanceof Error guard).
✅ No behavioral regression: a failing --outputs-file write was always fatal (exit code 1 in v0.24.0, verified against published CLIs).

Checks

Command	Result
pnpm exec jest packages/cdktn/test/validations.test.ts --runInBand	✅ 41 passed
pnpm exec nx build cdktn	✅ success
pnpm exec nx test cdktn --runInBand	✅ 43 suites, 542 tests, all passed

One minor note

output.ts (runOutput) has no direct test coverage for its error-wrapping path. The code is structurally identical to the well-tested deploy.ts path, so the risk is low — but a follow-up test for symmetry would close the gap.

Artifact-value

No dead code, no premature abstraction, no low-signal tests. Every artifact earns its maintenance cost. Ship.

---

[jsteinich]

Root cause confirmed, and deploy/output are indeed the only two commands using the callback.

The description and the code disagree on one point.

Splitting the guards also means a failing write no longer skips the outputs display

The throw Errors.External(...) sits before the render block, so a failing write does skip the display:

cdk-terrain/packages/cdktn-cli/src/bin/cmds/ui/deploy.ts

Lines 167 to 183 in d301821

	// A failed --outputs-file write is a broken promise to the user and must be fatal: await it and
	// rethrow as an External error, which cdktn.ts's top-level `.fail()` handler prints as a single
	// clean line rather than a stack trace, since the deploy itself already succeeded.
	try {
	await onOutputsRetrieved(outputs);
	} catch (e) {
	throw Errors.External(
	`Failed to write outputs${outputsPath ? ` to ${outputsPath}` : ""}: ${
	e instanceof Error ? e.message : e
	}`,
	e instanceof Error ? e : undefined,
	);
	}
	if (outputs && Object.keys(outputs).length > 0) {
	try {
	console.log(renderOutputs(outputs));

(same ordering in runOutput at L88-L101). Against 0.24.0's actual async behaviour, the table is now skipped where it previously printed before the crash.

Which did you mean?

• Leftover from an earlier draft → drop the sentence.
• You want the table to still print → the throw moves below the render block (collect, render, throw), plus a test.

Failing fast is defensible; I'd just like the two to agree.

Two smaller things:

1. ENOENT on a user-named directory reads more like Errors.Usage than External. Same clean-print branch either way, so semantics only — but EACCES/ENOSPC do fit External. Your call.
2. The path renders twice: Failed to write outputs to X: ENOENT: no such file or directory, open 'X'.

[so0k]

You're right that they disagree, and the answer is the second option — I want the table to print, so the
throw moves below the render block.

Not a leftover sentence: I'd written the description from the intent and then not checked it against the final
ordering, which is exactly the kind of thing a description should not get away with. Thanks for catching it.

The reasoning for render-then-throw: at that point the deploy has genuinely succeeded and the outputs exist in
memory. The only thing that failed is persisting them. Skipping the table means the user loses the values
entirely — they can't even copy them out of the terminal — for a failure that is often a mistyped --outputs-file
path they can just retry. Printing the table and then failing gives them both the data and the error. Your point
that this is a regression against 0.24.0's actual behaviour (where the table printed before the crash) makes it
worse than a wash, so failing fast here is losing information users had before.

New ordering: render → save → confirm. The written to line stays after the save, so it is still unreachable on
failure. With a test asserting the table is printed and the run rejects.

Errors.Usage for ENOENT — agreed, and it turns out to matter beyond semantics.

Usage errors are excluded from Sentry crash reporting, so classifying a user's mistyped path as External
would file a crash report for something that isn't a bug. Going with: ENOENT/ENOTDIR → Usage, everything
else (EACCES, ENOSPC, …) → External, keyed off err.code rather than message text.

Double path — fixing. The fs error message already names the path, so the prefix drops it:
Failed to write outputs: ENOENT: no such file or directory, open 'X'.

All three incoming; I'll push and re-request. The PR description gets corrected along with the code.

[so0k]

Pushed — rebased onto the updated #375, then 2b03546f6 + c35fcba41. All four points addressed.

• Render before failing. New ordering is render → save → confirm, in both runDeploy and runOutput. The
  written to line stays after the save so it's still unreachable on failure. Test asserts the table is
  printed and the run still rejects. The description sentence is now true rather than deleted.
• ENOENT/ENOTDIR → Errors.Usage, everything else stays External, keyed off err.code rather than
  message text. Tests cover both (ENOENT → Usage, EACCES → External).
• Path printed once — the prefix no longer repeats what the fs error already names.
• runOutput now has direct coverage (ui/__tests__/output.test.ts), modelled on its real shape —
  returnValue from the project callback, not outputsByConstructId.

Two notes against myself:

The first version of those runOutput tests passed for a partly wrong reason: the mock resolved a returnValue
without ever invoking the project callback, so fetchOutputs never ran and the suite would still have passed if
runOutput stopped fetching outputs altogether. Fixed in c35fcba41.

And a behaviour interaction worth knowing when reading the two PRs together: with --outputs-file and all
outputs dropped, #375 alone prints only No outputs found., whereas after this PR it prints that and the
written to line, since that line is now keyed off write success rather than render state.

[jsteinich]

This entire block is pretty much duplicated in deploy.ts. As the complexity has grown, should really make a reusable block.