-
Notifications
You must be signed in to change notification settings - Fork 5
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 CDK errors not being logged #211
Conversation
For some reason trying to log an error using `error` does not log anything. Also, we were not throwing the error so we were continuing until it failed later with a different very unhelpful error message. Instead of logging (since it doesn't work) just throw the error
@@ -152,15 +151,13 @@ export class App | |||
const message = e.message as string; | |||
const messageParts = message.split('Context lookups have been disabled. '); | |||
const missingParts = messageParts[1].split('Missing context keys: '); | |||
error( | |||
throw new Error( |
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.
There is a control flow difference between before vs after. Before, the error
command did not terminate the control flow. After, throw
terminates the control flow. Which one we want here?
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.
We want to throw. These errors that are thrown by CDK are fatal errors that prevent synthesis from completing. If we don't terminate here we end up with a very unhelpful error about not being able to read the cloud assembly.
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
This PR has been shipped in release v1.0.0. |
For some reason trying to log an error using
error
does not loganything. Also, we were not throwing the error so we were continuing
until it failed later with a different very unhelpful error message.
Instead of logging (since it doesn't work) just throw the error