Skip to content

Use concrete errors #171

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

Merged
merged 3 commits into from
Jan 25, 2021
Merged

Use concrete errors #171

merged 3 commits into from
Jan 25, 2021

Conversation

jbr
Copy link
Member

@jbr jbr commented Dec 20, 2020

using a dynamic error like anyhow is ideally suited to application code, not library code. async-h1 can enumerate every type of error that can occur, and it should do so.

we may want to revise the details of error enum before 3.0, but this represents a first effort at removing http_types::Error from async-h1.

@jbr jbr added this to the v3 milestone Dec 20, 2020
@jbr jbr force-pushed the use-concrete-errors branch 2 times, most recently from 79ce820 to ba94fa7 Compare December 20, 2020 00:58
@yoshuawuyts
Copy link
Member

Can you clarify how this is intended to deal with HTTP status codes? I like the way the code looks in async-h1, but I worry about the integration with Tide, since it appears this design requires manually matching the error kinds to their corresponding status codes. Whereas now the status codes are encoded into the error itself.

@jbr
Copy link
Member Author

jbr commented Dec 26, 2020

At the async-h1 level, status codes were always ignored previously. If an error occurs at the async-h1 level, there is no opportunity for tide to render a response. A few of these should have error handling inside of async-h1, but all async-h1 errors are irrecoverable from a higher framework perspective. The important point is that previous to this change, the only handling of async-h1 errors in tide was to log, as they are outside of the request-response cycle. After this change, a further PR will add selective error handling to async-h1 for any errors that make sense to return a response to, using match.

@jbr
Copy link
Member Author

jbr commented Jan 25, 2021

Merging this into the v3 branch

@jbr jbr changed the base branch from main to v3 January 25, 2021 20:27
mu2019 and others added 3 commits January 25, 2021 12:30
firefox send websocket upgrade header "Connection:keep-alive, Upgrade" can't match with connection_header_as_str.eq_ignore_ascii_case("upgrade")
With the next http-types release, this will allow omitting the cookie
feature and associated dependencies.
@jbr jbr force-pushed the use-concrete-errors branch from ba94fa7 to 550fb1c Compare January 25, 2021 20:52
@jbr jbr merged commit dabe27d into v3 Jan 25, 2021
@delete-merged-branch delete-merged-branch bot deleted the use-concrete-errors branch January 25, 2021 21:01
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.

4 participants