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

Use thiserror in miden-lib and miden-tx #1005

Merged
merged 6 commits into from
Dec 9, 2024

Conversation

PhilippGackstatter
Copy link
Contributor

Use thiserror in miden-lib and miden-tx and refactor some error messages to provide better errors and be in line with the guidelines discussed in #966 and #972.

Some InternalError(String) variants where replaced with a Custom { error_msg: Box<str>, source: Option<Box<dyn Error + Send + Sync + 'static>> } variant which allows implementors of traits like DataStore and TransactionAuthenticator to produce errors that are in line with our source-over-display approach. This is a small breaking change for the client cc @igamigo.

Some errors couldn't be made source errors because the latest miden-vm where errors implement core::error::Error in no-std is not yet published, so I've added them as a TODO here and added a note in #981.

closes #972

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left just a couple of nits inline.

miden-tx/src/errors/mod.rs Outdated Show resolved Hide resolved
miden-tx/src/errors/mod.rs Outdated Show resolved Hide resolved
@PhilippGackstatter PhilippGackstatter merged commit 10a2f06 into next Dec 9, 2024
9 checks passed
@PhilippGackstatter PhilippGackstatter deleted the pgackst-lib-tx-thiserror branch December 9, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants