aws-cdk
aws-cdk copied to clipboard
fix(custom-resource-handlers): better fallback for require failures
Issue # (if applicable)
Closes #30067.
Reason for this change
Fallback to existing AWS SDK import misses a rare/flaky edge case where the npm install passes, but the subsequent require fails
Description of changes
Fall back to the pre-existing AWS SDK if requiring the latest version fails
Description of how you validated changes
- Fixed no-op test "installs the latest SDK"
- Added test "falls back to installed sdk if require fails"
Checklist
- [x] My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Thanks for your contribution. We can't take this just yet because it will conflict with node runtime upgrades (https://github.com/aws/aws-cdk/pull/30108) but once that PR is merged we will take a look at this one.
Unfortunately, when that one is merged it will cause merge conflicts for all of your integ tests so I would advise holding off on any additional work here until that time so you're not having to deal with a lot of churn. In the meantime, I'm going to add the do-not-close label to this PR so you don't have to recreate it again if merging that one takes too long (it should not, but I'm just going to be cautious).
I've pushed up the integration test snapshot updates I've gotten through so far. I'm out of the office for the weekend, so if someone else wants to help with those that'd be great! Otherwise, I'll finish them up when I get a chance.
@TheRealAmazonKendra I've updated most of the integration tests, but there are a few that appear to be failing due to trying to assume roles without permissions. Anything you can do for the last few tests?
yarn integ --update-on-failed aws-appsync/test/integ.js-resolver.js
yarn integ --update-on-failed aws-certificatemanager/test/integ.dns-validated-certificate.js
yarn integ --update-on-failed aws-chatbot/test/integ.chatbot-logretention.js
yarn integ --update-on-failed aws-cloudfront/test/integ.cloudfront-cross-region-cert.js
yarn integ --update-on-failed aws-cloudfront/test/integ.distribution-function-runtime.js
yarn integ --update-on-failed aws-cloudfront/test/integ.distribution-lambda-cross-region.js
Don't make anymore snapshot updates for now. We are continuing to have some problems around them so I'm going to add a do-not-close label to this PR and then probably appear to be ignoring it for another couple weeks. I really appreciate your patience with these delays.
BTW, I did see your message on cdk.dev and that is the right way to get my attention.
Oh, the label is already added. Good. I absolutely plan to come back to this first once I get the other wrinkles ironed out.
sounds good; I'll check back in after two weeks!
@TheRealAmazonKendra How are we looking now? Ready to pick this back up?
Hello @glitchassassin, the node runtime upgrade is complete and please address the conflicts. I'll take a review on this PR by end of month.
The pull request linter fails with the following errors:
❌ Fixes must contain a change to an integration test file and the resulting snapshot.
PRs must pass status checks before we can provide a meaningful review.
If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.
AWS CodeBuild CI Report
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: 5c0960f09f55fc4c3124fb4d8fa2f686f424ab37
- Result: FAILED
- Build Logs (available for 30 days)
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
@GavinZZ, I re-ran these failing integration tests locally like so:
yarn integ --update-on-failed custom-resources/test/aws-custom-resource/integ.aws-custom-resource.js
For most of these, this appeared to successfully update the snapshot and pass, but when I re-ran yarn integ, all of the tests once again failed.
@glitchassassin Sorry for the late response. Because the changes were made in @aws-cdk/custom-resource-handlers module, you need to
- navigate to
packages/@aws-cdk/custom-resource-handlers/ - run
yarn build - run
npx lerna run build --scope=aws-cdk-lib - then re-run the failed tests.
@GavinZZ Thanks. I did re-run those builds, but it's having no effect: when I update the snapshot, the test still fails the next time I run yarn integ.
Closing in favor of #31571
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.