Skip to content
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

CFn: optional verbose errors #8336

Merged
merged 1 commit into from May 31, 2023
Merged

CFn: optional verbose errors #8336

merged 1 commit into from May 31, 2023

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented May 18, 2023

There is currently lots of exception handling in and around the template deployer within CloudFormation. This makes it more complex to understand what goes wrong with stack deployments.

This change adds a config variable to present exceptions either:

  • fully, raising the exception up, or
  • logging at exception level the message, exception and traceback, instead of logging at a lower level (often debug) with a possibly less-than-helpful error message.

This feature can be enabled with the CFN_VERBOSE_ERRORS configuration variable/environment variable.

@simonrw simonrw added the semver: patch Non-breaking changes which can be included in patch releases label May 18, 2023
@simonrw simonrw self-assigned this May 18, 2023
@coveralls
Copy link

Coverage Status

Coverage: 82.239% (-0.01%) from 82.254% when pulling 4e7a08e on cfn/verbose-errors into 8f43432 on master.

@github-actions
Copy link

LocalStack Community integration with Pro

2 045 tests   1 761 ✔️  1h 16m 30s ⏱️
       2 suites     284 💤
       2 files           0

Results for commit 4e7a08e.

@simonrw simonrw marked this pull request as ready for review May 19, 2023 12:26
@simonrw simonrw requested a review from thrau as a code owner May 19, 2023 12:26
Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @simonrw! It's great to see instrumentation improving in LocalStack and giving us more dials to turn when debugging.

I'm trying to understand whether this flag is targeted at users to allow them to look into why their stack isn't deploying, or whether this is targeted as a debugging tool for LocalStack (much like DEBUG=1. In which case my follow up question would be whether this is generalizable under a DEBUG_CFN flag that enables a debug mode for cloudformation (we have this pattern in other areas as well, DEBUG_HANDLER_CHAIN, DEBUG_ANALYTICS, DEBUG_LICENSING, etc).

If there is a particular reason to toggle only exception reporting, it seems we may consider doing things differently all together and raise exceptions rather than catching and logging them?

@simonrw
Copy link
Contributor Author

simonrw commented May 22, 2023

I'm trying to understand whether this flag is targeted at users to allow them to look into why their stack isn't deploying, or whether this is targeted as a debugging tool for LocalStack (much like DEBUG=1.

I would see this as mostly helpful for us - if a user receives a verbose error or exception, they are not immediately going to know how to resolve their problem, or even if it's a problem on their end.

In which case my follow up question would be whether this is generalizable under a DEBUG_CFN flag that enables a debug mode for cloudformation (we have this pattern in other areas as well, DEBUG_HANDLER_CHAIN, DEBUG_ANALYTICS, DEBUG_LICENSING, etc).

I'm happy to include this plus potentially more introspection tools under a common flag.

If there is a particular reason to toggle only exception reporting, it seems we may consider doing things differently all together and raise exceptions rather than catching and logging them?

Only that it is the minimal set of changes. We will likely revisit this concept when we get further into our refactoring. In addition, the current implementation is very trial and error, and it doesn't make sense to raise every exception since some are sort of expected (not ideal IMO).

@thrau thrau merged commit 55c871f into master May 31, 2023
27 checks passed
@thrau
Copy link
Member

thrau commented May 31, 2023

merging this for now, since i actually needed it the other day :P

@thrau thrau deleted the cfn/verbose-errors branch May 31, 2023 13:05
@dfangl dfangl added this to the 2.2 milestone Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants