refactor(cli): scope IoHost listeners via suppressMessages decorator - #1887
Merged
Merged
Conversation
Several CdkToolkit methods registered preventDefault listeners on the
CliIoHost to drop specific toolkit-lib messages and then restored
behavior with a blanket removeAllListeners() in a finally block.
Introduce a @suppressMessages(...codes) method decorator that registers
a single method-scoped drop-listener (via matchAny) before the method
runs and disposes exactly that listener when the method settles. Apply
it to metadata, destroy, and list, and replace every remaining
removeAllListeners() with disposal of exactly the listeners each method
registered.
This also fixes two latent leaks: destroy running as part of a deploy
(empty-resource stacks) no longer wipes deploy's I5060 rewrite
listener, and list's one-shot I2901 formatter no longer leaks when
synthesis fails before the listing is emitted.
The list suppression is now asserted behaviorally in the list command
tests via IoHostRecorder.entries({ includeDropped: true }), keeping
dropped messages out of the committed snapshots.
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1887 +/- ##
=======================================
Coverage 91.36% 91.37%
=======================================
Files 80 80
Lines 12207 12254 +47
Branches 1751 1749 -2
=======================================
+ Hits 11153 11197 +44
- Misses 1018 1021 +3
Partials 36 36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 task
mrgrain
disabled the stack merge
August 24, 2026 12:25
rix0rrr
approved these changes
Aug 24, 2026
mrgrain
added a commit
to go-to-k/aws-cdk-cli
that referenced
this pull request
Aug 24, 2026
Stacked on aws#1887. After the `suppressMessages` decorator took over the static message drops, the remaining dynamic listeners in `CdkToolkit` — deploy's approval-framing rewrite, destroy's level override, decline-aborts, and `--force` responder, and the one-shot listing formatters — still required collecting dispose functions and calling them in a `finally` block, which is verbose and easy to get wrong. This change makes the listener registration methods on `CliIoHost` (`on`, `once`, `rewrite`, `rewriteOnce`, `respond`, `respondOnce`) return a `DisposeListener`: still a plain callable remover, but also a `Disposable`, so call sites can bind it with a `using` declaration and drop the `try`/`finally` entirely. Disposal then happens at scope exit, including early returns and throws, and conditional listeners come for free because `using x = cond ? ioHost.respondOnce(...) : undefined` simply skips disposal for `undefined`. The repository already relies on TypeScript explicit resource management (`using` for telemetry timers), so no toolchain change is needed, and the return type change is backwards compatible for every existing caller. Most of the diff in `deploy()` is the mechanical de-indent from removing its `try`/`finally` wrapper. Tested with a new unit test asserting the remover works as a `using` resource (listener active inside the block, gone at scope exit), plus the existing decorator and command suites; 234 tests across the affected suites pass and lint is clean. ### Checklist - [ ] This change contains a major version upgrade for a dependency and I confirm all breaking changes are addressed - Release notes for the new version: --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several
CdkToolkitmethods drop specific toolkit-lib messages by registeringpreventDefaultlisteners on theCliIoHostand then restoring behavior with a blanketremoveAllListeners()in afinallyblock. That pattern is repetitive, and the blanket removal is a blunt instrument: it tears down every user listener on the shared host, not just the ones the method registered.This change introduces a
@suppressMessages(...codes)method decorator that registers a single method-scoped drop-listener (viamatchAny) before the decorated method runs and disposes exactly that listener when the method settles, whether it returns or throws. It is applied tometadata,destroy, andlist, and every remainingremoveAllListeners()call is replaced with disposal of exactly the listeners the method itself registered.Scoping the disposal also fixes two latent bugs. When
destroyran as part of adeploy(a stack with no resources triggers a delete mid-deployment), itsremoveAllListeners()wiped deploy'sI5060approval-rewrite listener for the rest of the deployment. Andlist's one-shotI2901formatter leaked with stale options whenever synthesis failed before the listing was emitted.Tested with new unit tests for the decorator (suppression is active only for the duration of the call, including the throwing path) and a behavioral test on the
cdk listpath that asserts the synth-time (I1000) and dependency-expansion (I1002) lines are emitted but dropped, using a newIoHostRecorder.entries({ includeDropped: true })override so dropped messages stay out of the committed NDJSON snapshots. Thedestroy.test.tsfailures visible in this area reproduce identically on a cleanmaincheckout and are unrelated.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license