feat(custom-resources): rename `catchErrorPattern` to `ignoreErrorCodesMatching` #6553

iliapolo · 2020-03-03T09:43:54Z

Description

Notice that i've added the ignoreErrorCodesMatching and PhysicalResourceId.fromResponse validation even on the onDelete. This is because it might result in an empty physical resource id, which will in turn cause a failure when submitting the response to CloudFormation:

2 | 8:26:06 PM | DELETE_FAILED        | Custom::AWS           | Playground/FakeResource/Resource/Default (PlaygroundFakeResourceBC34BA32) Invalid PhysicalResourceId
	new CustomResource (/Users/epolon/dev/src/github.com/aws/aws-cdk/packages/@aws-cdk/aws-cloudformation/lib/custom-resource.ts:163:21)
	\_ new AwsCustomResource (/Users/epolon/dev/src/github.com/aws/aws-cdk/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts:259:27)
	\_ new AwsCdkPlaygroundConstruct (/Users/epolon/dev/src/github.com/aws/aws-cdk-playground/lib/aws-cdk-playground-stack.ts:19:5)
	\_ Object.<anonymous> (/Users/epolon/dev/src/github.com/aws/aws-cdk-playground/bin/aws-cdk-playground.ts:15:1)
	\_ Module._compile (internal/modules/cjs/loader.js:778:30)
	\_ Module.m._compile (/Users/epolon/dev/src/github.com/aws/aws-cdk-playground/node_modules/ts-node/src/index.ts:536:23)
	\_ Module._extensions..js (internal/modules/cjs/loader.js:789:10)
	\_ Object.require.extensions.(anonymous function) [as .ts] (/Users/epolon/dev/src/github.com/aws/aws-cdk-playground/node_modules/ts-node/src/index.ts:539:12)
	\_ Module.load (internal/modules/cjs/loader.js:653:32)
	\_ tryModuleLoad (internal/modules/cjs/loader.js:593:12)
	\_ Function.Module._load (internal/modules/cjs/loader.js:585:3)
	\_ Function.Module.runMain (internal/modules/cjs/loader.js:831:12)
	\_ main (/Users/epolon/dev/src/github.com/aws/aws-cdk-playground/node_modules/ts-node/src/bin.ts:212:14)
	\_ Object.<anonymous> (/Users/epolon/dev/src/github.com/aws/aws-cdk-playground/node_modules/ts-node/src/bin.ts:470:3)
	\_ Module._compile (internal/modules/cjs/loader.js:778:30)
	\_ Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
	\_ Module.load (internal/modules/cjs/loader.js:653:32)
	\_ tryModuleLoad (internal/modules/cjs/loader.js:593:12)
	\_ Function.Module._load (internal/modules/cjs/loader.js:585:3)
	\_ Function.Module.runMain (internal/modules/cjs/loader.js:831:12)

Commit Message

feat(custom-resources): rename catchErrorPattern to ignoreErrorCodesMatching (#6553)

In the spirit of being "Explicit and Clear*, renaming catchErrorPattern to ignoreErrorCodesMatching since it better describes the meaning of this property.

In addition, the following validations were added:

ignoreErrorCodesMatching cannot be used with PhysicalResourceId.fromResponse since the response might not exist.
ignoreErrorCodesMatching cannot be used with getData or getDataString since the resource might not have any attributes due to the error catching.

Relates to #5873

BREAKING CHANGE: catchErrorPattern was renamed to ignoreErrorCodesMatching. In addition, a few synth time validations were added when using this property. See Error Handling for details.

End Commit Message

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…ng with ignoreErrorCodesMatching

packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts

packages/@aws-cdk/custom-resources/README.md

packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts

aws-cdk-automation · 2020-03-03T10:26:38Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 90c2f4e
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-03-03T14:02:46Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 656725b
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts

jogold · 2020-03-04T08:40:04Z

packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts

@@ -190,9 +190,21 @@ export interface AwsCustomResourceProps {
 *
 */
 export class AwsCustomResource extends cdk.Construct implements iam.IGrantable {
+
+  private static breakIgnoreErrorsCircuit(sdkCalls: Array<AwsSdkCall | undefined>, caller: string) {


personal taste... why not a non exported function outside the class for this?

I just prefer starting with the minimal visibility necessary. The function is used to validate specific configuration that can (currently) only arise in the AwsCustomResource class. I'd like additional usages of it to be very intentional and not just treat it as a sort of utility function.

packages/@aws-cdk/custom-resources/test/aws-custom-resource/aws-custom-resource.test.ts

packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts

…invocation fails

aws-cdk-automation · 2020-03-04T10:15:06Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 61051b1
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-03-04T10:15:14Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…attern

aws-cdk-automation · 2020-03-04T10:33:47Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 2eaa440
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-03-04T11:54:18Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2020-03-04T11:55:19Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 12212f8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…esMatching` (#6553) In the spirit of being "Explicit and Clear*, renaming `catchErrorPattern` to `ignoreErrorCodesMatching` since it better describes the meaning of this property. In addition, the following validations were added: - `ignoreErrorCodesMatching` cannot be used with `PhysicalResourceId.fromResponse` since the response might not exist. - `ignoreErrorCodesMatching` cannot be used with `getData` or `getDataString` since the resource might not have any attributes due to the error catching. Relates to #5873 BREAKING CHANGE: `catchErrorPattern` was renamed to `ignoreErrorCodesMatching`. In addition, a few synth time validations were added when using this property. See [Error Handling](https://github.com/aws/aws-cdk/tree/master/packages/%40aws-cdk/custom-resources#error-handling-1) for details.

…esMatching` (aws#6553) In the spirit of being "Explicit and Clear*, renaming `catchErrorPattern` to `ignoreErrorCodesMatching` since it better describes the meaning of this property. In addition, the following validations were added: - `ignoreErrorCodesMatching` cannot be used with `PhysicalResourceId.fromResponse` since the response might not exist. - `ignoreErrorCodesMatching` cannot be used with `getData` or `getDataString` since the resource might not have any attributes due to the error catching. Relates to aws#5873 BREAKING CHANGE: `catchErrorPattern` was renamed to `ignoreErrorCodesMatching`. In addition, a few synth time validations were added when using this property. See [Error Handling](https://github.com/aws/aws-cdk/tree/master/packages/%40aws-cdk/custom-resources#error-handling-1) for details.

iliapolo added 5 commits March 2, 2020 17:11

rename catchErrorPattern to ignoreErrorCodesMatching

007774a

added validation that PhysicalResourceId.fromResponse is not used alo…

737617e

…ng with ignoreErrorCodesMatching

added section in readme about property

71d45a5

added validation that is not used with getData and getDataString

e57f5c5

amend expected template for integ test

90c2f4e

iliapolo requested review from eladb and rix0rrr March 3, 2020 09:44

mergify bot added the contribution/core This is a PR that came from AWS. label Mar 3, 2020

iliapolo commented Mar 3, 2020

View reviewed changes

packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts Show resolved Hide resolved

iliapolo mentioned this pull request Mar 3, 2020

Graduate @aws-cdk/custom-resources to stable #5873

Closed

rix0rrr requested changes Mar 3, 2020

View reviewed changes

rix0rrr mentioned this pull request Mar 3, 2020

feat(custom-resources): rename getData* to getResponseField* #6556

Merged

code review comments

656725b

iliapolo requested a review from rix0rrr March 3, 2020 13:21

SomayaB assigned eladb Mar 3, 2020

jogold reviewed Mar 4, 2020

View reviewed changes

packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts Outdated Show resolved Hide resolved

iliapolo added 3 commits March 4, 2020 11:25

use conditional chaining instead of null check

555d2ab

extract resource creation outside of 'expect' to make sure the right …

1e1ac83

…invocation fails

oops - revert wrong condition

61051b1

rix0rrr approved these changes Mar 4, 2020

View reviewed changes

Merge branch 'master' into epolon/custom-resources-rename-catchErrorP…

2eaa440

…attern

fix test compilation due to API change merged from master

12212f8

mergify bot merged commit 94599f8 into master Mar 4, 2020

mergify bot deleted the epolon/custom-resources-rename-catchErrorPattern branch March 4, 2020 11:54

This was referenced Mar 4, 2020

chore(prlinter): added commit message validation #6573

Merged

fix(batch): managed compute environment now properly works with compute resources and instanceRole has correct docstring and type definition #6549

Merged

snyk-bot mentioned this pull request Apr 10, 2020

[Snyk] Upgrade aws-cdk from 1.28.0 to 1.30.0 ammobinDOTca/ammobin-cdk#2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(custom-resources): rename `catchErrorPattern` to `ignoreErrorCodesMatching` #6553

feat(custom-resources): rename `catchErrorPattern` to `ignoreErrorCodesMatching` #6553

iliapolo commented Mar 3, 2020 •

edited

Loading

aws-cdk-automation commented Mar 3, 2020

aws-cdk-automation commented Mar 3, 2020

jogold Mar 4, 2020

iliapolo Mar 4, 2020

aws-cdk-automation commented Mar 4, 2020

mergify bot commented Mar 4, 2020

aws-cdk-automation commented Mar 4, 2020

mergify bot commented Mar 4, 2020

aws-cdk-automation commented Mar 4, 2020

feat(custom-resources): rename catchErrorPattern to ignoreErrorCodesMatching #6553

feat(custom-resources): rename catchErrorPattern to ignoreErrorCodesMatching #6553

Conversation

iliapolo commented Mar 3, 2020 • edited Loading

Description

Commit Message

End Commit Message

aws-cdk-automation commented Mar 3, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Mar 3, 2020

AWS CodeBuild CI Report

jogold Mar 4, 2020

Choose a reason for hiding this comment

iliapolo Mar 4, 2020

Choose a reason for hiding this comment

aws-cdk-automation commented Mar 4, 2020

AWS CodeBuild CI Report

mergify bot commented Mar 4, 2020

aws-cdk-automation commented Mar 4, 2020

AWS CodeBuild CI Report

mergify bot commented Mar 4, 2020

aws-cdk-automation commented Mar 4, 2020

AWS CodeBuild CI Report

feat(custom-resources): rename `catchErrorPattern` to `ignoreErrorCodesMatching` #6553

feat(custom-resources): rename `catchErrorPattern` to `ignoreErrorCodesMatching` #6553

iliapolo commented Mar 3, 2020 •

edited

Loading