Improve the builtin Request/Response types #1939
Conversation
Signed-off-by: Ryan Levick <[email protected]>
sdk/rust/src/http.rs
Outdated
| /// Technically headers do not have to be utf8 encoded but this type assumes they are. | ||
| /// If you know you'll be dealing with non-utf8 encoded headers, reach more a more powerful | ||
| /// response type like that found in the `http` crate. | ||
| pub fn headers(&self) -> &[(String, String)] { |
There was a problem hiding this comment.
could we maybe instead introduce a headers_raw (and headers_raw_mut) that returns (String, Vec<u8>)?
I'm imagining that we'd internally store headers as something like (String, HeaderVal), where HeaderVal is something like
enum HeaderVal {
Bytes(Vec<u8>),
String(String),
Both(Vec<u8>, String),
}I bet there's some more optimal representation of this for the case where Bytes contains valid unicode, too.
There was a problem hiding this comment.
I am somewhat worried about this. I think I've managed to push a change that supports this without being a maintenance burden, be we do have to get comfortable at some point saying that these types will never cover 100% of use cases and a 3rd party crate might be necessary.
Signed-off-by: Ryan Levick <[email protected]>
rylev
force-pushed
the
improve-builtin-request-response
branch
from
011576c to
b363229
Compare
rylev
force-pushed
the
improve-builtin-request-response
branch
from
0d182a6 to
90e8c8f
Compare
Signed-off-by: Ryan Levick <[email protected]>
rylev
force-pushed
the
improve-builtin-request-response
branch
from
90e8c8f to
aa06684
Compare
Signed-off-by: Ryan Levick <[email protected]>
Signed-off-by: Ryan Levick <[email protected]>
rylev
force-pushed
the
improve-builtin-request-response
branch
from
ecdce33 to
b337273
Compare
| @@ -99,6 +384,23 @@ impl std::fmt::Display for Method { | |||
| } | |||
|
|
|||
| impl IncomingRequest { | |||
| /// The incoming request Uri | |||
| pub fn uri(&self) -> String { | |||
There was a problem hiding this comment.
Can the user get this as a Url object or would they need to parse the string?
There was a problem hiding this comment.
They would need to parse. The thought by behind it is to not make the http crate part of the public API for these types.
This is meant as an alternative to #1931. It improves the
Request/Responsetypes with an API that is useable, flexible, and powerful and hopefully enough to cover 80% of use cases. For more advanced use cases, we would recommend using a third party crate likehttp.Note: The one slightly awkward API choice is that
RequestBuilder::uriis infallible so that even if the user passes a garbage URI it still "works" - they'll likely only see any error when the try to use the uri. I think this is the right balance of easy of use without sacrificing too much protection against misuse.