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

Refactor error system #164

Closed
plafer opened this issue Oct 11, 2022 · 4 comments · Fixed by #255
Closed

Refactor error system #164

plafer opened this issue Oct 11, 2022 · 4 comments · Fixed by #255
Labels
O: new-feature Objective: aims to add new feature

Comments

@plafer
Copy link
Contributor

plafer commented Oct 11, 2022

Currently, most of our defined errors are unused (they were previously created for hermes needs). This creates confusion in how to use the errors properly.

We should also revisit whether we need flex_error, or if instead we want to move to a more canonical & simple error crate such as anyhow.

@plafer plafer added the O: new-feature Objective: aims to add new feature label Oct 11, 2022
@mzabaluev
Copy link
Contributor

if instead we want to move to a more canonical & simple error crate such as anyhow.

Anyhow is too freeform for a good library API. thiserror would be great, but it's unconditionally tied to std::error::Error so it's a no go for no_std.

flex_error has the appropriate feature set, the problems I see with it are mainly in readability. The custom macro is less transparent than the minimalistic approach taken by thiserror, is somewhat boilerplatey, and requires user discipline to avoid excessive allocations. It also creates magic constructor function names that cannot be grepped from the source.

@DaviRain-Su
Copy link
Contributor

DaviRain-Su commented Oct 12, 2022

I've seen std::error::Error moved to the core::error::Error eyre should soon support no_std. hack-ink/array-bytes#18
eyre-rs/eyre#83

@plafer
Copy link
Contributor Author

plafer commented Oct 12, 2022

After discussing with @romac, a good direction also is to go by this comment's advice. In short, use displaydoc + implement std::error::Error if std is enabled.

The main reason is that we don't need any fancy error functionality in the handlers (e.g. no backtrace functionality needed) that were useful for hermes. Therefore, our "Error strategy" should be equally simple.

@DaviRain-Su
Copy link
Contributor

I think once the core::error::Error stabilizes in a rust stable version, some error processing libraries will be supported soon.

livelybug pushed a commit to octopus-network/ibc-rs that referenced this issue Oct 14, 2022
@DaviRain-Su DaviRain-Su mentioned this issue Nov 22, 2022
7 tasks
shuoer86 pushed a commit to shuoer86/ibc-rs that referenced this issue Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: new-feature Objective: aims to add new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants