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 flex-error to define errors #988

Merged
merged 71 commits into from
Jul 26, 2021
Merged

Use flex-error to define errors #988

merged 71 commits into from
Jul 26, 2021

Conversation

soareschen
Copy link
Contributor

Description

This PR experiments on using flex-error to define application errors using the define_error! macro. Using this, we can easily switch between error tracers such as eyre, anyhow, or a naive string tracer by triggering Cargo feature flags.

For the experiment, only the error definition ics26_routing::error is modified. To inspect how the error definitions are expanded, install cargo-expand and run the following:

$ cd modules && cargo expand ics26_routing::error

More documentation will be written on the flex-error GitHub.


For contributor use:

  • Updated the Unreleased section of CHANGELOG.md with the issue.
  • If applicable: Unit tests written, added test to CI.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments.
  • Re-reviewed Files changed in the Github PR explorer.

@soareschen
Copy link
Contributor Author

soareschen commented May 27, 2021

The documentation for flex-error is now available at https://docs.rs/flex-error/0.2.0/flex_error/

@soareschen soareschen force-pushed the soares/flex-error branch from c710fa3 to 91a2d53 Compare June 7, 2021 09:56
@romac romac added the E: no-std External: support for no_std compatibility label Jul 21, 2021
@soareschen
Copy link
Contributor Author

A new version of flex-error is now published: https://docs.rs/flex-error/0.4.0/flex_error/

Hopefully the documentation is now clear enough that future contributors can understand how to define and update the error types here.

modules/Cargo.toml Outdated Show resolved Hide resolved
@romac
Copy link
Member

romac commented Jul 26, 2021

Great work @soareschen! I have one question left: I see that by default flex-error pulls in both anyhow and eyre, and I wonder if we even need to pull those in? If we need one of the two library, then we can perhaps only enable eyre?

@romac
Copy link
Member

romac commented Jul 26, 2021

Can we also add an entry to .changelog? See #1232 and https://github.com/informalsystems/unclog for some background.

@soareschen
Copy link
Contributor Author

I have one question left: I see that by default flex-error pulls in both anyhow and eyre, and I wonder if we even need to pull those in? If we need one of the two library, then we can perhaps only enable eyre?

I have updated the the dependency to flex-error with default-features = false, and enable the flex-error/eyre_tracer feature by default. So now the default ibc crate will only have eyre as the indirect dependency. The user of ibc crate should be able to override this to use anyhow if they want.

In any case, I leave both options for eyre and anyhow in flex-error because it was undecided which error tracer would work better. With the option there, we can still switch from eyre to anyhow easily if there are issues with eyre in the future.

modules/Cargo.toml Outdated Show resolved Hide resolved
modules/src/ics02_client/error.rs Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E: no-std External: support for no_std compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants