diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index b5349565b..d4d2dea9d 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -15,11 +15,10 @@ on: workflow_dispatch: {} merge_group: {} push: - branches: [ "main" ] - pull_request: - branches: [ "main" ] + branches: ["main"] + pull_request: {} schedule: - - cron: '21 23 * * 5' + - cron: "21 23 * * 5" jobs: analyze: @@ -45,8 +44,8 @@ jobs: fail-fast: false matrix: include: - - language: javascript-typescript - build-mode: none + - language: javascript-typescript + build-mode: none # CodeQL supports the following values keywords for 'language': 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'swift' # Use `c-cpp` to analyze code written in C, C++ or both # Use 'java-kotlin' to analyze code written in Java, Kotlin or both @@ -56,45 +55,45 @@ jobs: # If you are analyzing a compiled language, you can modify the 'build-mode' for that language to customize how # your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages steps: - - name: Checkout repository - uses: actions/checkout@v4 + - name: Checkout repository + uses: actions/checkout@v4 - # Add any setup steps before running the `github/codeql-action/init` action. - # This includes steps like installing compilers or runtimes (`actions/setup-node` - # or others). This is typically only required for manual builds. - # - name: Setup runtime (example) - # uses: actions/setup-example@v1 + # Add any setup steps before running the `github/codeql-action/init` action. + # This includes steps like installing compilers or runtimes (`actions/setup-node` + # or others). This is typically only required for manual builds. + # - name: Setup runtime (example) + # uses: actions/setup-example@v1 - # Initializes the CodeQL tools for scanning. - - name: Initialize CodeQL - uses: github/codeql-action/init@v4 - with: - languages: ${{ matrix.language }} - build-mode: ${{ matrix.build-mode }} - # If you wish to specify custom queries, you can do so here or in a config file. - # By default, queries listed here will override any specified in a config file. - # Prefix the list here with "+" to use these queries and those in the config file. + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v4 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. - # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs - # queries: security-extended,security-and-quality + # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs + # queries: security-extended,security-and-quality - # If the analyze step fails for one of the languages you are analyzing with - # "We were unable to automatically build your code", modify the matrix above - # to set the build mode to "manual" for that language. Then modify this step - # to build your code. - # ℹī¸ Command-line programs to run using the OS shell. - # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun - - if: matrix.build-mode == 'manual' - shell: bash - run: | - echo 'If you are using a "manual" build mode for one or more of the' \ - 'languages you are analyzing, replace this with the commands to build' \ - 'your code, for example:' - echo ' make bootstrap' - echo ' make release' - exit 1 + # If the analyze step fails for one of the languages you are analyzing with + # "We were unable to automatically build your code", modify the matrix above + # to set the build mode to "manual" for that language. Then modify this step + # to build your code. + # ℹī¸ Command-line programs to run using the OS shell. + # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun + - if: matrix.build-mode == 'manual' + shell: bash + run: | + echo 'If you are using a "manual" build mode for one or more of the' \ + 'languages you are analyzing, replace this with the commands to build' \ + 'your code, for example:' + echo ' make bootstrap' + echo ' make release' + exit 1 - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v4 - with: - category: "/language:${{matrix.language}}" + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v4 + with: + category: "/language:${{matrix.language}}" diff --git a/packages/aws-cdk/lib/cli/cdk-toolkit.ts b/packages/aws-cdk/lib/cli/cdk-toolkit.ts index 7d695989b..6520c3fda 100644 --- a/packages/aws-cdk/lib/cli/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cli/cdk-toolkit.ts @@ -230,7 +230,7 @@ export class CdkToolkit { IO.CDK_TOOLKIT_I1000, // ✨ Synthesis time (info) ) public async metadata(stackName: string, json: boolean) { - const dispose = this.ioHost.once( + using _formatter = this.ioHost.once( IO.CDK_TOOLKIT_I2901, (msg) => ({ action: 'metadata', @@ -238,17 +238,13 @@ export class CdkToolkit { }), ); - try { - await this.toolkit.list(this.props.cloudExecutable, { - stacks: { - patterns: [stackName], - strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, - expand: ExpandStackSelection.NONE, - }, - }); - } finally { - dispose(); - } + await this.toolkit.list(this.props.cloudExecutable, { + stacks: { + patterns: [stackName], + strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, + expand: ExpandStackSelection.NONE, + }, + }); } public async acknowledge(noticeId: string) { @@ -467,151 +463,147 @@ export class CdkToolkit { // toolkit-lib emits the approval request (I5060) without mentioning // `--require-approval`. The CLI owns that flag, so it adds the framing here. // Both deploy paths resolve through this host, so one listener covers both. - // Disposed in the finally below: `deploy()` can run repeatedly (watch mode), - // and each run must register a rewrite for its own `requireApproval` value. - const disposeRewrite = this.ioHost.rewrite(IO.CDK_TOOLKIT_I5060, (msg) => { + // Method-scoped (`using`): `deploy()` can run repeatedly (watch mode), and + // each run must register a rewrite for its own `requireApproval` value. + using _approvalFraming = this.ioHost.rewrite(IO.CDK_TOOLKIT_I5060, (msg) => { const updateTypeText = msg.data.permissionChangeType !== PermissionChangeType.NONE ? 'security-sensitive updates' : 'updates'; return `Stack includes ${updateTypeText} and "--require-approval" is set to '${requireApproval}'.\nDo you wish to deploy these changes?`; }); - try { - // execute-change-set is a new flow that we can just delegate to toolkit-lib - if (options.deploymentMethod?.method === 'execute-change-set') { - await this.toolkit.deploy(this.props.cloudExecutable, { - deploymentMethod: options.deploymentMethod, - stacks: { - patterns: options.selector.patterns, - strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, - expand: ExpandStackSelection.NONE, - }, - roleArn: options.roleArn, - forceDeployment: options.force, - rollback: options.rollback, - reuseAssets: options.reuseAssets, - concurrency: options.concurrency, - traceLogs: options.traceLogs, - notificationArns: options.notificationArns, - tags: options.tags, - outputsFile: options.outputsFile, - assetParallelism: options.assetParallelism, - assetBuildConcurrency: options.assetBuildConcurrency, - assetBuildTime: options.assetBuildTime, - parameters: undefined, // parameters are only set during change set creation, so this is explicitly unset because change set already exists - }); - return; - } - - const startSynthTime = new Date().getTime(); - const stackCollection = await this.selectStacksForDeploy( - options.selector, - options.exclusively, - options.cacheCloudAssembly, - options.ignoreNoStacks, - ); - const elapsedSynthTime = new Date().getTime() - startSynthTime; - await this.ioHost.asIoHelper().defaults.info(`\n✨ Synthesis time: ${formatTime(elapsedSynthTime)}s\n`); + // execute-change-set is a new flow that we can just delegate to toolkit-lib + if (options.deploymentMethod?.method === 'execute-change-set') { + await this.toolkit.deploy(this.props.cloudExecutable, { + deploymentMethod: options.deploymentMethod, + stacks: { + patterns: options.selector.patterns, + strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, + expand: ExpandStackSelection.NONE, + }, + roleArn: options.roleArn, + forceDeployment: options.force, + rollback: options.rollback, + reuseAssets: options.reuseAssets, + concurrency: options.concurrency, + traceLogs: options.traceLogs, + notificationArns: options.notificationArns, + tags: options.tags, + outputsFile: options.outputsFile, + assetParallelism: options.assetParallelism, + assetBuildConcurrency: options.assetBuildConcurrency, + assetBuildTime: options.assetBuildTime, + parameters: undefined, // parameters are only set during change set creation, so this is explicitly unset because change set already exists + }); + return; + } - if (stackCollection.stackCount === 0) { - await this.ioHost.asIoHelper().defaults.error('This app contains no stacks'); - return; - } + const startSynthTime = new Date().getTime(); + const stackCollection = await this.selectStacksForDeploy( + options.selector, + options.exclusively, + options.cacheCloudAssembly, + options.ignoreNoStacks, + ); + const elapsedSynthTime = new Date().getTime() - startSynthTime; + await this.ioHost.asIoHelper().defaults.info(`\n✨ Synthesis time: ${formatTime(elapsedSynthTime)}s\n`); - const migrator = new ResourceMigrator({ - deployments: this.props.deployments, - ioHelper: asIoHelper(this.ioHost, 'deploy'), - }); - await migrator.tryMigrateResources(stackCollection, { - toolkitStackName: this.toolkitStackName, - ...options, - }); + if (stackCollection.stackCount === 0) { + await this.ioHost.asIoHelper().defaults.error('This app contains no stacks'); + return; + } - if (options.deploymentMethod?.method === 'hotswap') { - await this.ioHost.asIoHelper().defaults.warn( - '⚠ī¸ The --hotswap and --hotswap-fallback flags deliberately introduce CloudFormation drift to speed up deployments', - ); - await this.ioHost.asIoHelper().defaults.warn('⚠ī¸ They should only be used for development - never use them for your production Stacks!\n'); - } + const migrator = new ResourceMigrator({ + deployments: this.props.deployments, + ioHelper: asIoHelper(this.ioHost, 'deploy'), + }); + await migrator.tryMigrateResources(stackCollection, { + toolkitStackName: this.toolkitStackName, + ...options, + }); - const stacks = stackCollection.stackArtifacts; + if (options.deploymentMethod?.method === 'hotswap') { + await this.ioHost.asIoHelper().defaults.warn( + '⚠ī¸ The --hotswap and --hotswap-fallback flags deliberately introduce CloudFormation drift to speed up deployments', + ); + await this.ioHost.asIoHelper().defaults.warn('⚠ī¸ They should only be used for development - never use them for your production Stacks!\n'); + } - const assetBuildTime = options.assetBuildTime ?? AssetBuildTime.ALL_BEFORE_DEPLOY; - const prebuildAssets = assetBuildTime === AssetBuildTime.ALL_BEFORE_DEPLOY; - const concurrency = options.concurrency || 1; - if (concurrency > 1) { - // the "bar" progress output doesn't support concurrency, fall back to "events" - if (this.ioHost.stackProgress === StackActivityProgress.BAR) { - this.ioHost.stackProgress = StackActivityProgress.EVENTS; - } + const stacks = stackCollection.stackArtifacts; - // ...but only warn if the user explicitly requested "bar" progress - if (options.progress === StackActivityProgress.BAR) { - await this.ioHost.asIoHelper().defaults.warn('⚠ī¸ The --concurrency flag does not support --progress "bar". Switching to "events".'); - } + const assetBuildTime = options.assetBuildTime ?? AssetBuildTime.ALL_BEFORE_DEPLOY; + const prebuildAssets = assetBuildTime === AssetBuildTime.ALL_BEFORE_DEPLOY; + const concurrency = options.concurrency || 1; + if (concurrency > 1) { + // the "bar" progress output doesn't support concurrency, fall back to "events" + if (this.ioHost.stackProgress === StackActivityProgress.BAR) { + this.ioHost.stackProgress = StackActivityProgress.EVENTS; } - const stacksAndTheirAssetManifests = stacks.flatMap((stack) => [ - stack, - ...stack.dependencies.filter(x => cxapi.AssetManifestArtifact.isAssetManifestArtifact(x)), - ]); - const workGraph = new WorkGraphBuilder( - asIoHelper(this.ioHost, 'deploy'), - prebuildAssets, - ).build(stacksAndTheirAssetManifests); - - // Unless we are running with '--force', skip already published assets - if (!options.force) { - await this.removePublishedAssets(workGraph, options); + // ...but only warn if the user explicitly requested "bar" progress + if (options.progress === StackActivityProgress.BAR) { + await this.ioHost.asIoHelper().defaults.warn('⚠ī¸ The --concurrency flag does not support --progress "bar". Switching to "events".'); } + } - const graphConcurrency: Concurrency = { - 'stack': concurrency, - 'asset-build': (options.assetParallelism ?? true) ? options.assetBuildConcurrency ?? 1 : 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds - 'asset-publish': (options.assetParallelism ?? true) ? 8 : 1, // This will be I/O-bound, 8 in parallel seems reasonable - 'marker': 1, - }; - - const deploymentActions = new WorkGraphDeploymentActions(this.props.deployments, this.ioHost, this, { - roleArn: options.roleArn, - force: options.force, - stackCount: stackCollection.stackCount, - notificationArns: options.notificationArns, - deploymentMethod: options.deploymentMethod, - toolkitStackName: this.toolkitStackName, - reuseAssets: options.reuseAssets, - tags: options.tags, - parameters: options.parameters, - usePreviousParameters: options.usePreviousParameters, - rollback: options.rollback, - concurrency, - requireApproval, - assetParallelism: options.assetParallelism, - extraUserAgent: options.extraUserAgent, - cloudWatchLogMonitor: options.cloudWatchLogMonitor, - sdkProvider: this.props.sdkProvider, - express: options.express, - }); + const stacksAndTheirAssetManifests = stacks.flatMap((stack) => [ + stack, + ...stack.dependencies.filter(x => cxapi.AssetManifestArtifact.isAssetManifestArtifact(x)), + ]); + const workGraph = new WorkGraphBuilder( + asIoHelper(this.ioHost, 'deploy'), + prebuildAssets, + ).build(stacksAndTheirAssetManifests); + + // Unless we are running with '--force', skip already published assets + if (!options.force) { + await this.removePublishedAssets(workGraph, options); + } - const startDeployTime = Date.now(); + const graphConcurrency: Concurrency = { + 'stack': concurrency, + 'asset-build': (options.assetParallelism ?? true) ? options.assetBuildConcurrency ?? 1 : 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds + 'asset-publish': (options.assetParallelism ?? true) ? 8 : 1, // This will be I/O-bound, 8 in parallel seems reasonable + 'marker': 1, + }; - await workGraph.doParallel(graphConcurrency, deploymentActions); + const deploymentActions = new WorkGraphDeploymentActions(this.props.deployments, this.ioHost, this, { + roleArn: options.roleArn, + force: options.force, + stackCount: stackCollection.stackCount, + notificationArns: options.notificationArns, + deploymentMethod: options.deploymentMethod, + toolkitStackName: this.toolkitStackName, + reuseAssets: options.reuseAssets, + tags: options.tags, + parameters: options.parameters, + usePreviousParameters: options.usePreviousParameters, + rollback: options.rollback, + concurrency, + requireApproval, + assetParallelism: options.assetParallelism, + extraUserAgent: options.extraUserAgent, + cloudWatchLogMonitor: options.cloudWatchLogMonitor, + sdkProvider: this.props.sdkProvider, + express: options.express, + }); - if (options.outputsFile) { - // If an outputs file has been specified, create the file path and write stack outputs to it once. - // Outputs are written after all stacks have been deployed. If a stack deployment fails, - // all of the outputs from successfully deployed stacks before the failure will still be written. - await deploymentActions.writeOutputs(options.outputsFile); - } + const startDeployTime = Date.now(); - // Add a timer on the COMMAND span for the full deployment wait time (not the same as the sum of all DEPLOY - // spans because of parallelism). - this.ioHost.telemetry?.commandSpan?.addTimer('totalDeployTime', Date.now() - startDeployTime); + await workGraph.doParallel(graphConcurrency, deploymentActions); - await this.ioHost.asIoHelper().defaults.info(`\n✨ Total time: ${formatTime(Date.now() - startSynthTime)}s\n`); - } finally { - disposeRewrite(); + if (options.outputsFile) { + // If an outputs file has been specified, create the file path and write stack outputs to it once. + // Outputs are written after all stacks have been deployed. If a stack deployment fails, + // all of the outputs from successfully deployed stacks before the failure will still be written. + await deploymentActions.writeOutputs(options.outputsFile); } + + // Add a timer on the COMMAND span for the full deployment wait time (not the same as the sum of all DEPLOY + // spans because of parallelism). + this.ioHost.telemetry?.commandSpan?.addTimer('totalDeployTime', Date.now() - startDeployTime); + + await this.ioHost.asIoHelper().defaults.info(`\n✨ Total time: ${formatTime(Date.now() - startSynthTime)}s\n`); } /** @@ -1040,43 +1032,35 @@ export class CdkToolkit { this.ioHost.stackProgress = StackActivityProgress.EVENTS; } - const disposers = [ - // The success line was `info` in the historical `cdk destroy`, not `result`. - this.ioHost.on(IO.CDK_TOOLKIT_I7900, () => ({ level: 'info' })), // ✅ : destroyed + // The success line was `info` in the historical `cdk destroy`, not `result`. + using _successLevel = this.ioHost.on(IO.CDK_TOOLKIT_I7900, () => ({ level: 'info' })); // ✅ : destroyed - // toolkit-lib logs a declined confirmation (E7010) and returns gracefully. - // The CLI surfaces a decline as a non-zero, soft exit instead: throwing from - // the listener both suppresses the log and aborts the command (the top-level - // renders `AbortError` as "Deletion cancelled"). - this.ioHost.on(IO.CDK_TOOLKIT_E7010, () => { - throw new AbortError('DestroyAborted', 'Deletion cancelled'); - }), - ]; + // toolkit-lib logs a declined confirmation (E7010) and returns gracefully. + // The CLI surfaces a decline as a non-zero, soft exit instead: throwing from + // the listener both suppresses the log and aborts the command (the top-level + // renders `AbortError` as "Deletion cancelled"). + using _declineAborts = this.ioHost.on(IO.CDK_TOOLKIT_E7010, () => { + throw new AbortError('DestroyAborted', 'Deletion cancelled'); + }); - if (options.force) { - disposers.push(this.ioHost.respondOnce(IO.CDK_TOOLKIT_I7010, true)); - } + using _forceConfirms = options.force ? this.ioHost.respondOnce(IO.CDK_TOOLKIT_I7010, true) : undefined; - try { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - `_destroyWithAction` is private; the CLI sets the action label. - await this.toolkit._destroyWithAction(this.props.cloudExecutable, action, { - stacks: { - patterns: options.selector.patterns, - strategy: options.selector.allTopLevel - ? StackSelectionStrategy.MAIN_ASSEMBLY - : options.selector.patterns.length > 0 - ? StackSelectionStrategy.PATTERN_MATCH - : StackSelectionStrategy.ONLY_SINGLE, - expand: options.exclusively ? ExpandStackSelection.NONE : ExpandStackSelection.DOWNSTREAM, - }, - roleArn: options.roleArn, - concurrency: options.concurrency, - express: options.express, - }); - } finally { - disposers.forEach((dispose) => dispose()); - } + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore - `_destroyWithAction` is private; the CLI sets the action label. + await this.toolkit._destroyWithAction(this.props.cloudExecutable, action, { + stacks: { + patterns: options.selector.patterns, + strategy: options.selector.allTopLevel + ? StackSelectionStrategy.MAIN_ASSEMBLY + : options.selector.patterns.length > 0 + ? StackSelectionStrategy.PATTERN_MATCH + : StackSelectionStrategy.ONLY_SINGLE, + expand: options.exclusively ? ExpandStackSelection.NONE : ExpandStackSelection.DOWNSTREAM, + }, + roleArn: options.roleArn, + concurrency: options.concurrency, + express: options.express, + }); } // cdk ls stdout is the stack listing only. Suppress the info status lines synthesis emits next @@ -1086,17 +1070,15 @@ export class CdkToolkit { selectors: string[], options: { long?: boolean; json?: boolean; showDeps?: boolean } = {}, ): Promise { - const dispose = this.ioHost.rewriteOnce(IO.CDK_TOOLKIT_I2901, (msg) => formatStackList(msg.data.stacks, options)); - - try { - await this.toolkit.list(this.props.cloudExecutable, { - stacks: selectors.length > 0 - ? { patterns: selectors, strategy: StackSelectionStrategy.PATTERN_MATCH, expand: ExpandStackSelection.UPSTREAM } - : undefined, - }); - } finally { - dispose(); - } + // One-shot: disposes itself when the listing (I2901) is emitted; the + // `using` covers the case where `list` throws before that happens. + using _formatter = this.ioHost.rewriteOnce(IO.CDK_TOOLKIT_I2901, (msg) => formatStackList(msg.data.stacks, options)); + + await this.toolkit.list(this.props.cloudExecutable, { + stacks: selectors.length > 0 + ? { patterns: selectors, strategy: StackSelectionStrategy.PATTERN_MATCH, expand: ExpandStackSelection.UPSTREAM } + : undefined, + }); return 0; // exit-code } diff --git a/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts b/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts index 793bbae15..768c76c7d 100644 --- a/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts +++ b/packages/aws-cdk/lib/cli/io-host/cli-io-host.ts @@ -215,6 +215,32 @@ export type MessageSelector = | IoRequestMaker | ((msg: IoMessage) => boolean); +/** + * Removes a previously registered message listener. + * + * Callable directly (`dispose()`), and also a `Disposable`, so it can be bound + * to the enclosing scope with a `using` declaration — the listener is then + * removed when the scope exits, even on an early return or a throw: + * + * ```ts + * using _fmt = ioHost.rewrite(IO.CDK_TOOLKIT_I2901, format); + * ``` + * + * `using x = cond ? ioHost.on(...) : undefined` is also valid: disposal is + * simply skipped for `undefined`, which makes conditional listeners cheap. + */ +export interface DisposeListener { + (): void; + [Symbol.dispose](): void; +} + +/** + * Make a plain remover function usable as a `Disposable` (see `DisposeListener`). + */ +function disposeListener(dispose: () => void): DisposeListener { + return Object.assign(dispose, { [Symbol.dispose]: dispose }); +} + /** * How an IoHost processed a single message or request. * @@ -533,12 +559,12 @@ export class CliIoHost implements IIoHost, ObservableIoHost { public on( selector: IoMessageMaker | IoRequestMaker | ((msg: IoMessage) => msg is IoMessage), listener: (msg: IoMessage) => MessageListenerResultOrPromise, - ): () => void; + ): DisposeListener; public on( predicate: (msg: IoMessage) => boolean, listener: (msg: IoMessage) => MessageListenerResultOrPromise, - ): () => void; - public on(selector: MessageSelector, listener: MessageListenerFn): () => void { + ): DisposeListener; + public on(selector: MessageSelector, listener: MessageListenerFn): DisposeListener { return this.addMessageListener({ once: false, fn: listener, matches: messageMatcher(selector) }); } @@ -565,12 +591,12 @@ export class CliIoHost implements IIoHost, ObservableIoHost { public once( selector: IoMessageMaker | IoRequestMaker | ((msg: IoMessage) => msg is IoMessage), listener: (msg: IoMessage) => MessageListenerResultOrPromise, - ): () => void; + ): DisposeListener; public once( predicate: (msg: IoMessage) => boolean, listener: (msg: IoMessage) => MessageListenerResultOrPromise, - ): () => void; - public once(selector: MessageSelector, listener: MessageListenerFn): () => void { + ): DisposeListener; + public once(selector: MessageSelector, listener: MessageListenerFn): DisposeListener { return this.addMessageListener({ once: true, fn: listener, matches: messageMatcher(selector) }); } @@ -610,14 +636,14 @@ export class CliIoHost implements IIoHost, ObservableIoHost { * // Under --force, auto-confirm the destroy prompt without prompting. * const dispose = ioHost.respond(IO.CDK_TOOLKIT_I7010, true); */ - public respond(code: IoRequestMaker, value: U, suppressQuestion = true): () => void { + public respond(code: IoRequestMaker, value: U, suppressQuestion = true): DisposeListener { return this.addMessageListener({ once: false, fn: () => ({ respond: value, preventDefault: suppressQuestion }), matches: messageMatcher(code) }); } /** * Like `respond`, but the answer is given only once and then removed. */ - public respondOnce(code: IoRequestMaker, value: U, suppressQuestion = true): () => void { + public respondOnce(code: IoRequestMaker, value: U, suppressQuestion = true): DisposeListener { return this.addMessageListener({ once: true, fn: () => ({ respond: value, preventDefault: suppressQuestion }), matches: messageMatcher(code) }); } @@ -641,7 +667,7 @@ export class CliIoHost implements IIoHost, ObservableIoHost { code: IoMessageMaker | IoRequestMaker, formatter: (msg: IoMessage) => string, level?: IoMessageLevel, - ): () => void { + ): DisposeListener { return this.on(code, (msg) => ({ message: formatter(msg), ...(level !== undefined ? { level } : {}) })); } @@ -653,22 +679,23 @@ export class CliIoHost implements IIoHost, ObservableIoHost { code: IoMessageMaker | IoRequestMaker, formatter: (msg: IoMessage) => string, level?: IoMessageLevel, - ): () => void { + ): DisposeListener { return this.once(code, (msg) => ({ message: formatter(msg), ...(level !== undefined ? { level } : {}) })); } /** - * Add a listener to the registry and return a function that removes it. + * Add a listener to the registry and return a remover for it (callable and + * `using`-compatible, see `DisposeListener`). */ - private addMessageListener(listener: MessageListener): () => void { + private addMessageListener(listener: MessageListener): DisposeListener { this.messageListeners.push(listener); - return () => { + return disposeListener(() => { const index = this.messageListeners.indexOf(listener); if (index >= 0) { this.messageListeners.splice(index, 1); } - }; + }); } /** @@ -1127,12 +1154,10 @@ export function suppressMessages(...selectors: MessageSelector[]) { throw new ToolkitError('InvalidDecoratorTarget', 'suppressMessages can only decorate methods'); } descriptor.value = async function (this: { readonly ioHost: CliIoHost }, ...args: A): Promise { - const dispose = this.ioHost.on(matchAny(...selectors), () => ({ preventDefault: true })); - try { - return await original.apply(this, args); - } finally { - dispose(); - } + using _suppress = this.ioHost.on(matchAny(...selectors), () => ({ preventDefault: true })); + // `return await` (not a bare `return`) so the listener is only disposed + // after the method has actually settled. + return await original.apply(this, args); }; }; } diff --git a/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts b/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts index 37704cf1c..d03d1af2a 100644 --- a/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts +++ b/packages/aws-cdk/test/cli/io-host/cli-io-host.test.ts @@ -342,6 +342,19 @@ describe('CliIoHost', () => { expect(answer).toBe(true); }); + test('listener removers are using-compatible (Symbol.dispose)', async () => { + { + using _suppress = ioHost.on(IO.CDK_TOOLKIT_I2901, () => ({ preventDefault: true })); + await ioHost.notify(listMessage('inside')); + } + + // disposed at block exit: the listener no longer applies + await ioHost.notify(listMessage('outside')); + + expect(mockStdout).not.toHaveBeenCalledWith('inside\n'); + expect(mockStdout).toHaveBeenCalledWith('outside\n'); + }); + test('on() with matchAny() fires for any of the given codes', async () => { const observed: string[] = []; track(ioHost.on(matchAny(IO.CDK_TOOLKIT_I2901, IO.CDK_TOOLKIT_I1000), (msg) => {