Skip to content

Commit 7b47f41

Browse files
authored
fix(cli): hotswap deploy fails on multiple CfnEvaluationException (#22339)
closes #22323 To avoid unhandled rejections, we run promises just before we call `Promise.all`. The concern of this fix is that hotswap process may take longer time because now async tasks run lazily. However I don't think it will be a big problem since those tasks are not I/O bound, so most of them are already running sequentially, not in parallel. ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 9934619 commit 7b47f41

File tree

2 files changed

+91
-3
lines changed

2 files changed

+91
-3
lines changed

packages/aws-cdk/lib/api/hotswap-deployments.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ async function findAllHotswappableChanges(
7676
const resourceDifferences = getStackResourceDifferences(stackChanges);
7777

7878
let foundNonHotswappableChange = false;
79-
const promises: Array<Array<Promise<ChangeHotswapResult>>> = [];
79+
const promises: Array<() => Array<Promise<ChangeHotswapResult>>> = [];
8080
const hotswappableResources = new Array<HotswapOperation>();
8181

8282
// gather the results of the detector functions
@@ -97,7 +97,8 @@ async function findAllHotswappableChanges(
9797
} else if (resourceHotswapEvaluation === ChangeHotswapImpact.IRRELEVANT) {
9898
// empty 'if' just for flow-aware typing to kick in...
9999
} else {
100-
promises.push([
100+
// run isHotswappable* functions lazily to prevent unhandled rejections
101+
promises.push(() => [
101102
isHotswappableLambdaFunctionChange(logicalId, resourceHotswapEvaluation, evaluateCfnTemplate),
102103
isHotswappableStateMachineChange(logicalId, resourceHotswapEvaluation, evaluateCfnTemplate),
103104
isHotswappableEcsServiceChange(logicalId, resourceHotswapEvaluation, evaluateCfnTemplate),
@@ -111,7 +112,7 @@ async function findAllHotswappableChanges(
111112
// resolve all detector results
112113
const changesDetectionResults: Array<Array<ChangeHotswapResult>> = [];
113114
for (const detectorResultPromises of promises) {
114-
const hotswapDetectionResults = await Promise.all(detectorResultPromises);
115+
const hotswapDetectionResults = await Promise.all(detectorResultPromises());
115116
changesDetectionResults.push(hotswapDetectionResults);
116117
}
117118

packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts

+87
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Lambda, StepFunctions } from 'aws-sdk';
2+
import { CfnEvaluationException } from '../../../lib/api/evaluate-cloudformation-template';
23
import * as setup from './hotswap-test-setup';
34

45
let hotswapMockSdkProvider: setup.HotswapMockSdkProvider;
@@ -414,3 +415,89 @@ test('A change to both a hotswappable resource and a stack output results in a f
414415
expect(mockUpdateMachineDefinition).not.toHaveBeenCalled();
415416
expect(mockUpdateLambdaCode).not.toHaveBeenCalled();
416417
});
418+
419+
test('Multiple CfnEvaluationException will not cause unhandled rejections', async () => {
420+
// GIVEN
421+
setup.setCurrentCfnStackTemplate({
422+
Resources: {
423+
Func1: {
424+
Type: 'AWS::Lambda::Function',
425+
Properties: {
426+
Code: {
427+
S3Bucket: 'current-bucket',
428+
S3Key: 'current-key',
429+
},
430+
Environment: {
431+
key: 'old',
432+
},
433+
FunctionName: 'my-function',
434+
},
435+
Metadata: {
436+
'aws:asset:path': 'old-path',
437+
},
438+
},
439+
Func2: {
440+
Type: 'AWS::Lambda::Function',
441+
Properties: {
442+
Code: {
443+
S3Bucket: 'current-bucket',
444+
S3Key: 'current-key',
445+
},
446+
Environment: {
447+
key: 'old',
448+
},
449+
FunctionName: 'my-function',
450+
},
451+
Metadata: {
452+
'aws:asset:path': 'old-path',
453+
},
454+
},
455+
},
456+
});
457+
const cdkStackArtifact = setup.cdkStackArtifactOf({
458+
template: {
459+
Resources: {
460+
Func1: {
461+
Type: 'AWS::Lambda::Function',
462+
Properties: {
463+
Code: {
464+
S3Bucket: 'current-bucket',
465+
S3Key: 'current-key',
466+
},
467+
Environment: {
468+
key: { Ref: 'ErrorResource' },
469+
},
470+
FunctionName: 'my-function',
471+
},
472+
Metadata: {
473+
'aws:asset:path': 'new-path',
474+
},
475+
},
476+
Func2: {
477+
Type: 'AWS::Lambda::Function',
478+
Properties: {
479+
Code: {
480+
S3Bucket: 'current-bucket',
481+
S3Key: 'current-key',
482+
},
483+
Environment: {
484+
key: { Ref: 'ErrorResource' },
485+
},
486+
FunctionName: 'my-function',
487+
},
488+
Metadata: {
489+
'aws:asset:path': 'new-path',
490+
},
491+
},
492+
},
493+
},
494+
});
495+
496+
// WHEN
497+
const deployStackResult = hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact);
498+
499+
// THEN
500+
await expect(deployStackResult).rejects.toThrowError(CfnEvaluationException);
501+
expect(mockUpdateMachineDefinition).not.toHaveBeenCalled();
502+
expect(mockUpdateLambdaCode).not.toHaveBeenCalled();
503+
});

0 commit comments

Comments
 (0)