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

[PoC] [RFC] Allow routes to return anyhow::Error #588

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thomaseizinger
Copy link
Contributor

@thomaseizinger thomaseizinger commented May 26, 2020

Using anyhow allows for really ergonomic error handling in Rust code. Unfortunately, these ergonomics end at the integration layer with warp. Currently, we work around this by having separate handler functions that return an anyhow::Result which we then pack into a rejection (we also have an intermediary step for HttpApiProblem but that is not important at this stage):

https://github.com/comit-network/comit-rs/blob/301eb63afea6327df8eb7281bc8251090cc120a3/cnd/src/http_api/routes.rs#L30-L37

I am putting this PR up as a proof-of-concept to show that we could improve ergonomics here fairly easily :)

I wasn't quite sure how to deal with the CombineRejection trait. The compiler complained so I implemented it in a way that seems to work but I feel that this was not the original purpose of it.

@jxs
Copy link
Collaborator

jxs commented May 26, 2020

Hi, this has already been discussed, see #527, #374

@thomaseizinger
Copy link
Contributor Author

thomaseizinger commented May 27, 2020

Hi, this has already been discussed, see #527, #374

I am aware of these discussions, I was even part of some of them :)
The reason I am opening this PR is because in my opinion this was never properly addressed.
Ironically, #307 was just brought to my attention again through a different comment so I am gonna try and continue the discussion there!

@jxs jxs mentioned this pull request Aug 5, 2020
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.

None yet

2 participants