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

Improve error messages for cdk migrate in cdk-from-cfn #29558

Closed
TheRealAmazonKendra opened this issue Mar 20, 2024 · 2 comments
Closed

Improve error messages for cdk migrate in cdk-from-cfn #29558

TheRealAmazonKendra opened this issue Mar 20, 2024 · 2 comments
Assignees
Labels
in-progress This issue is being actively worked on. toolkit/migrate Related to cdk migrate

Comments

@TheRealAmazonKendra
Copy link
Contributor

Context

In cdk-from-cfn, the current code defines a single error named TransmuteError and all other errors are defined using one of the panic!, unimplemented!, or unreachable! macros. TransmuteError is all encompassing for any transmutation related errors such as resource translation errors, parsing errors, import instruction errors, etc. Any use of the panic!, unimplemented!, or unreachable! macro will cause the source code to panic in place. In wasm builds, this causes a non-descriptive runtime error with a message of "unreachable".

In general, our current mechanism for defining error types and messages causes several problems:

  1. Our code is harder to debug since error types are all encompassing and too widely scoped
  2. Error messages are too generic when they originate from the panic!, unimplemented!, or unreachable! macros
  3. New custom error types must be implemented manually which slows down development speed and is error prone
  4. Users don't have clear error messages that help them to effectively debug resulting in a frustrating UX and increasing issues in our queue

Task Description

Use the thiserror crate to build a new mechanism for defining errors with more specific error types and messages.

Acceptance Criteria

  1. We have a single enum with all errors defined.
  2. All instances of panic!, unreachable!, and unimplemented! are replaced with a well-defined error type and a descriptive message.
  3. Unit tests have been created for each defined error.
  4. The updated code is built to wasm and error message formatting is checked on the CLI side.
  5. If needed, the CLI error formatting code must be updated in a follow-up PR.
  6. Version of cdk-from-cfn is released to aws-cdk and all unit/integ tests still pass or are fixed.

Comments:
cdklabs/cdk-from-cfn#600

@TheRealAmazonKendra TheRealAmazonKendra added toolkit/migrate Related to cdk migrate in-progress This issue is being actively worked on. labels Mar 20, 2024
@colifran
Copy link
Contributor

Closed with - cdklabs/cdk-from-cfn#600

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-progress This issue is being actively worked on. toolkit/migrate Related to cdk migrate
Projects
None yet
Development

No branches or pull requests

2 participants