Comments (12)
I've never used error chain...
Would it work if there was an http::Error
type and conversions from all other errors into http::Error
?
from http.
Ah I think error chain itself isn't so much of a thing to worry about here but rather the interconversion of so many error types. The error chain pattern is one where it's nice to only have one error per lib you're workign with (rather than N inside each of the M libs), but in general it's nicer to talk about just one kind of error vs many.
Would it work if there was an http::Error type and conversions from all other errors into http::Error?
I was hoping so! I initially though we could do this but unfortunately it doesn't work. What we're looking at is functions that are like:
fn foo() -> MyResult<()> {
some_http_function()?;
}
So the ?
operator needs a conversion from all errors in the HTTP crate to MyError
(or w/e the error is in MyResult
). The problem is that it needs a direct conversion, so you'd have to implement it once per HTTP error.
Another reason we might want this is that there's no ergonomic method of writing a function that calls multiple HTTP apis but only return one kind of error, right now you'd be forced to always define your own!
from http.
I'm Ok w/ whatever @seanmonstar / @withoutboats agree to.
from http.
In the conversion errors, I think it is not helpful to distinguish source type in the errors. That is, I think these sets of errors should be merged into a single zero sized type:
status::FromU16Error
andstatus::FromStrError
header::name::FromBytesError
andheader::name::FromStrError
These errors should also all be renamed so that they just indicate that the value of this type was invalid - similar to InvalidValueError
they should be InvalidStatusError
, InvalidNameError
, etc, instead of saying they are a From{X}Error
.
It could be a convenience to also have a unified error type which indicates all the ways constructing an HTTP message could fail, to be used either by the request/response builders (depending on how they shake out), or just to make it easier for downstream users to coallesce these errors together.
from http.
I suppose a lot of this issue can be boiled down to the builder PR, for example, what does this look like?
fn foo() -> Result<(), ???> {
let request = Request::builder()
.header("Foo", "Bar") /* ? */
.method("POST") /* ? */
.uri("https://www.rust-lang.org") /* ? */
.body(); /* ? */
// use request
}
How many ?
is necessary to get that working, and what error do we fill in to get it working? I've been thinking we want one ?
operator and a "by default" error to fill in there, but we could move the slider elsewhere perhaps!
from http.
Right, my thinking was that if each of those methods return Results, we could either:
- Have them return a unified error type.
- Have a unified error type exist, so they don't have to, but
foo
could return it and they could all be?
'd because of theFrom
impl.
The second seems slightly more "pure" but also less obvious how you can use it ergonomically.
from http.
Have a unified error type exist, so they don't have to, but foo could return it and they could all be ?'d because of the From impl.
The downside here is that it won't jump 2 Into
s for you. So, if each method returns a separate error type, then either this doesn't work, or you gotta implement From
a whole bunch of times:
fn do_a_thing() -> Result<(), MyError> {
let req = Request::builder()
.method("GET")?
.uri("https://www.rust-lang.org")?
.build();
}
from http.
Yeah that's what I was thinking as well. The downside of (2) you listed though is that let's say you have:
fn foo() -> Result<(), MyCustomError> {
let request = Request::builder()
// ...
.body(());
// large block of code returning `MyCustomError` everywhere
}
So here you're using the http
crate in the context of a larger application, which means that your function now returns your application's error instead of http crate's "blanket error". In this case though you're required to do:
impl From<http::Error> for MyCustomError {}
impl From<http::InvalidUri> for MyCustomError {}
impl From<http::InvalidStatus> for MyCustomError {}
impl From<http::InvalidHeaderName> for MyCustomError {}
impl From<http::InvalidHeaderValue> for MyCustomError {}
// ...
That's what initially led me down the road of one blanket error everywhere. I figured that the ergonomics of converting HTTP errors into your application would come up quite frequently, whereas inspecting the error cause would come up quite rarely, so I figured we could optimize the other way for now. Having only one error type though is certainly far less flexible!
from http.
That's a good point. I'd be happy using 1 error type in the request/response builder, and if users want to avoid the "overhead" of that type they can use the methods like HeaderValue::from_bytes
etc.
from http.
Er this comment maybe should have been placed here instead
from http.
Do you think though that rationalizing this with the rest of the library would make sense? For method_from to return a http::Error but Method::try_from_slice("POST") would return a http::InvalidMethod error?
Its definitely an unfortunate outcome, but it seems like our options are (in a sort of spectrum):
- Only have a unified error type, no individual types.
- Have the builders return a unified error type, but the other methods return individual error types.
- Have a unified error type, but everything returns individual types and you have to perform the cast.
- Have no unified error type, only individual types.
All of these have downsides. I lean toward the second one but I could probably be convinced of the first if you think we should do that. The downside of moving away from individual types I see is that with ZSTs I'm pretty sure you get the NPO today if you use things like HeaderValue::from_bytes
.
from http.
An excellent way to put it!
It seems like we can "safely" eliminate the last one at least. I'd be a little less worried about the ZST-ness of the errors in the sense of that they're all in theory going to grow more payload information over time. (in the sense that we probably wouldn't want to guarantee ZST-ness)
I've been wondering if we could have a unified error with tons of accessors or something like that. In general it seems like 90% of users don't need the benefit of fine-grained errors, so if we could continue to cater to the 10% with perhaps a little less 'static guarantees' it may be a win-win, but unsure.
from http.
Related Issues (20)
- Any way to convert `HeaderValue` to `Bytes` without copying? HOT 1
- Clear contents of sensitive header values on drop HOT 2
- Why does canonical_reason() return a Option? HOT 3
- Rename 422 from "Unprocessable Entity" to "Unprocessable Content"
- const fn for `StatusCode::as_u16` HOT 1
- can't parse uri without scheme but having path HOT 4
- 1.0 vs 0.2 incompatibility HOT 1
- Allow customizing header casing
- Add a `Display` impl for `Request` and `Response` to reconstruct the raw HTTP message HOT 2
- impl std::error::Error for StatusCode HOT 1
- http::method::Method::FromStr implementation allows invalid methods, conflicting with from_str documentation HOT 2
- Invalid uri parse HOT 1
- `http::uri::Uri` is excessively large and makes unnecessary allocations HOT 2
- Failed to parse a valid url HOT 1
- Uri does not parse valid URIs with empty authority portion HOT 1
- Add 420 Enhance Your Calm status code
- How to append a queryparams string after using uri_mut().into_parts()? HOT 1
- `index out of bounds` when parsing a valid header value HOT 2
- Request headers containing Russian (and possibly other non-English) characters fail with `UTF-8 encoding error` HOT 1
- HeaderMap::try_with_capacity can panic when capacity is large
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from http.