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

Propose the 1.0 Roadmap #2806

Merged
merged 1 commit into from
May 20, 2022
Merged

Propose the 1.0 Roadmap #2806

merged 1 commit into from
May 20, 2022

Conversation

seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Apr 6, 2022

The VISION outlines a decision-making framework, use-cases, and general shape
of hyper. This roadmap describes the currently known problems with hyper, and
then shows what changes are needed to make hyper 1.0 look more like what is in
the VISION.

Rendered

(Another step in the hyper 1.0 timeline.)

@seanmonstar seanmonstar force-pushed the roadmap branch 2 times, most recently from c9f21d1 to ff8bad4 Compare April 6, 2022 17:06
@avjewe
Copy link

avjewe commented Apr 7, 2022

There's something I need to do, that I don't seem to be able to do with the current version. For archival purposes, I would like to fetch an url, and then store not only the bytes of body, but the bytes of the request and the bytes of the returned http headers. It seems like a simple thing, but none of the rust http libraries support it (as far as I can tell). My apologies if this is already available, or somehow impossible.

docs/ROADMAP.md Show resolved Hide resolved
docs/ROADMAP.md Show resolved Hide resolved
docs/ROADMAP.md Show resolved Hide resolved
docs/ROADMAP.md Show resolved Hide resolved
docs/ROADMAP.md Outdated Show resolved Hide resolved
@agraven
Copy link

agraven commented Apr 7, 2022

Not sure if this is the best place to mention this, but as discussed in rustls/hyper-rustls#167, the Service bound on the Connect trait only accepting Uri as request type is rather limiting, especially when it comes to adjusting behavior for individual requests.

@agraven
Copy link

agraven commented Apr 7, 2022

There's something I need to do, that I don't seem to be able to do with the current version. For archival purposes, I would like to fetch an url, and then store not only the bytes of body, but the bytes of the request and the bytes of the returned http headers. It seems like a simple thing, but none of the rust http libraries support it (as far as I can tell). My apologies if this is already available, or somehow impossible.

I've also wanted this for debugging purposes in the past, essentially as an equivalent of curl -v

@liamwarfield
Copy link
Contributor

There's something I need to do, that I don't seem to be able to do with the current version. For archival purposes, I would like to fetch an url, and then store not only the bytes of body, but the bytes of the request and the bytes of the returned http headers. It seems like a simple thing, but none of the rust http libraries support it (as far as I can tell). My apologies if this is already available, or somehow impossible.

I've also wanted this for debugging purposes in the past, essentially as an equivalent of curl -v

Two suggestions here:

  1. The roadmap mentioned that "for hyper 1.0, the main API will focus on the "lower-level" connection types". Would a RawConnnection type that just provides (and takes in) raw bytes be wanted here?
  2. I've been working on a flag to preserved header order (fix(ffi): Change hyper_headers_foreach to iterate in recieved order #2798) to fix some the libcurl tests. This gets closer to what you want. Currently it feels a little weird to add this overhead to all the connection types (currently its behind certain feature flags, so it's not present in default use), so having something like option 1 might be better.

docs/ROADMAP.md Outdated Show resolved Hide resolved
docs/ROADMAP.md Outdated Show resolved Hide resolved
docs/ROADMAP.md Show resolved Hide resolved
The `AddrStream` struct will be completely removed, as it provides no value but
causes binary bloat.

Similar to `client`, and as describe in the *Design*, the `conn` modules will
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will conn modules be promoted to the main client/server modules?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. We still want the ability to eventually promote higher-level Client or Server types.

Maybe having less-nested modules is a good thing, I could be convinced of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue that we don't have that many structs in the conn module if we name things correctly it shouldn't be that hard. In addition, we can always put the Client/Server at the root of the crate rather than in client/server modules.

docs/ROADMAP.md Show resolved Hide resolved
docs/ROADMAP.md Show resolved Hide resolved
docs/ROADMAP.md Show resolved Hide resolved

### Should there be `hyper::io` traits?

### Should returned body types be `impl Body`?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check on the progress of the rustc features to make this easier xD

@seanmonstar
Copy link
Member Author

seanmonstar commented Apr 9, 2022

@avjewe @agraven

For archival purposes, I would like to fetch an url, and then store not only the bytes of body, but the bytes of the request and the bytes of the returned http headers.

This is possible even now by providing an IO transport that logs the bytes that read and written. For example, reqwest's ClientBuilder::connection_verbose (src) does this.

(Since this is possible, and to tidy up a little, I may hide the comments related to this topic :D)

@seanmonstar
Copy link
Member Author

@agraven

the Service bound on the Connect trait only accepting Uri as request type is rather limiting

Yea, I've heard it both ways: people want a more limited type, or a more expressive type. Either way, I think that proves it's not stable, and should go into hyper-util.

docs/ROADMAP.md Show resolved Hide resolved
docs/ROADMAP.md Show resolved Hide resolved
@lucacasonato
Copy link

Wow, this is really great. Thanks for writing it up @seanmonstar!

The most exciting changes for me are the separate h1 and h2 public client APIs. I think this will be a really great change. I have had many uses for this over the last year. It'd be especially great if the hyper::client::http2::SendRequest is cloneable like h2::client::SendRequest, allowing for multiple simultaneous requests on the same SendRequest (unlike hyper::client::conn::SendRequest).

The more generic connection pool also seems very useful.

The rest of the changes also seem very reasonable. We'll figure out how they impact our integration in Deno when we have a RC release to integrate :-)

Very exciting! 🎉

docs/ROADMAP.md Show resolved Hide resolved
docs/ROADMAP.md Show resolved Hide resolved
Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks good! Have nothing else to add that haven't been brought up already 😊

docs/ROADMAP.md Show resolved Hide resolved
@nox
Copy link
Contributor

nox commented May 2, 2022

For the connection types, have we ever considered making HTTP/1 APIs look like HTTP/2 APIs? I've often wished there was something similar to SendRequest/SendBody/RecvResponse/RecvBody for HTTP/1.

Also, do we plan to make a h1 crate like there is a h2 crate?

focus on the "lower-level" connection types. The `Client` and `Server` helpers
will be moved to `hyper-util`.

## Public API
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One area I'd love to see some improvement on is that hyper has a number of quasi-public types and traits that are inaccessible from outside hyper. For example, in #2831 (comment):

This makes it complicated to do more complicated things with hyper, since the bounds used in the code and in the documentation are not directly usable. I ran into this in https://fuchsia-review.googlesource.com/c/fuchsia/+/669708, when I was migrating my server from hyper::server::Server to hyper::server::conn. I wanted to implement my own service wrapper to help track down who was holding onto an Arc, but hyper::server::Builder::serve takes a MakeServiceRef, which is not accessible to users. It took me way too long to figure out how to get it to work with a tower::Service.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of those types are public simply to allow propagating the Sendiness of the user's provide Service and Body types, so that someone can use a single-threaded executor and use !Send types. If it were possible to make that happen without having those types quasi-public, I would do that.

The MakeServiceRef will be going away from hyper core.

@nox
Copy link
Contributor

nox commented May 18, 2022

In my previous comment I said:

For the connection types, have we ever considered making HTTP/1 APIs look like HTTP/2 APIs? I've often wished there was something similar to SendRequest/SendBody/RecvResponse/RecvBody for HTTP/1.

Something that I just realised today: if there was such a API, it would mean I could just use it for all HTTP/1 and HTTP/2 incoming requests, and I could more easily send back a specific HTTP/2 RST_STREAM code whenever I need to on receiving the request, through SendResponse::send_reset.

@seanmonstar
Copy link
Member Author

I do think we could much of what is in hyper::proto::h1 into an http1 crate. That's an eventual goal, but not something that I personally would hope to tackle for hyper 1.0, since we only have so much time and energy, and it wouldn't block hyper 1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet