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

make sure Error types from protocols crates implement std::error::Error #921

Closed
Tracked by #778
plebhash opened this issue May 21, 2024 · 9 comments
Closed
Tracked by #778

Comments

@plebhash
Copy link
Collaborator

plebhash commented May 21, 2024

an idea to improve error handling on the roles implementation is to start using the anyhow crate #736 (comment)

as a prerequisite, all custom Error types from the low-level crates under protocols must implement the std::error::Error trait

@plebhash
Copy link
Collaborator Author

once #932 is merged, the proposed changes on this issue can be achieved via:

#[cfg(not(feature = "no_std"))]
impl std::error::error for Error {}

@rockcoolsaint
Copy link
Contributor

This then means that you're handling this issue right?

@plebhash
Copy link
Collaborator Author

plebhash commented Jun 6, 2024

This then means that you're handling this issue right?

hey! this is actually on my backlog, if you wanna give it a go, feel free!

my suggestion would be:

  • branch off from feat-optional-no-std on my fork, to make sure that we can merge your PR after #932 is merged
  • do what I'm describing on the description of this issue, as well as the comment below it
  • send a draft PR

once you submit your PR, I'll make sure to add to the description of #932 that we should merge yours afterwards

@rockcoolsaint
Copy link
Contributor

Alright

@rockcoolsaint
Copy link
Contributor

@plebhash should the 'anyhow' crate be applied to every instance of unwrap() or strictly functions that return Result<> type?
Moreover, there are way too many instances where unwrap() is used in roles crate alone. Will refactoring everyone of those instances be appropriate?

@plebhash
Copy link
Collaborator Author

this issue is only related to making sure Error types from protocols crates implement std::error::Error

while the potential introduction of anyhow is the motivation, it is not the focus here... that is future discussion.

@rockcoolsaint
Copy link
Contributor

this issue is only related to making sure Error types from protocols crates implement std::error::Error

while the potential introduction of anyhow is the motivation, it is not the focus here... that is future discussion.

I'm kind of having a hard time identifying the custom Errors, as there seems to be Error scattered everywhere in the protocol crate. Please can you show me a reference custom Error. Seems there are going to be many instances to deal with

@plebhash
Copy link
Collaborator Author

@rockcoolsaint thanks for your effort on this one

we decided to go on a different direction (#932 (review)), apologies if you spent a lot of effort on this one already

@rockcoolsaint
Copy link
Contributor

Alright then, I'll pickup another issue

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

No branches or pull requests

2 participants