-
Notifications
You must be signed in to change notification settings - Fork 815
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: improve error message on custom policies validation failure #9862
Conversation
This pull request introduces 1 alert when merging 850680a into c4b7487 - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM , just one question
printer.error(`${resourceName} ${categoryName} custom-policies.json failed validation:`); | ||
formatter.list((validatePolicy?.errors || []).map(err => `${err.dataPath} ${err.message}`)); | ||
throw new CustomPoliciesFormatError(` | ||
Invalid custom IAM policies for ${resourceName} ${categoryName}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need an e2e test fix, since out messages have changed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I searched for the original string in our test suite and didn't get any hits, so hopefully we're good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
π Hi, this pull request was referenced in the v7.6.24 release! Check out the release notes here https://github.com/aws-amplify/amplify-cli/releases/tag/v7.6.24. |
Description of changes
Improve error message when custom-policies.json validation fails. The message will now include the path with the failure message.
Issue #, if available
fixes #9861
Description of how you validated changes
Tested manually. Message used to look like:
and now looks like:
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.