Rocket icon indicating copy to clipboard operation
Rocket copied to clipboard

Stabilize HTTP library

Open SergioBenitez opened this issue 9 years ago • 67 comments

At present, Rocket reexports Hyper types under http:hyper. Hyper causes a lot of issues, particularly when trying to optimize performance. A decision needs to be made on whether to stabilize these Hyper types, move away from Hyper (likely), or do something else all-together.

If we do move away from Hyper, here are things we should see to it that we re-implement or ensure the new library provides:

  • Automatic injection of the Date header.
  • Validation of outgoing Header names and values.

SergioBenitez avatar Oct 01 '16 02:10 SergioBenitez

I'd be very interested in hearing about your issues here. Also, have you used the tokio branch at all?

steveklabnik avatar Dec 23 '16 16:12 steveklabnik

@SergioBenitez The Tokio team is working toward a 0.1 release of the Tokio stack, after which HTTP is likely to be more of a focus. We should try to work together in exploring the space. Let's be in touch!

aturon avatar Dec 23 '16 16:12 aturon

@SergioBenitez if the plan is to move to an async back end, why does the current API not use Futures? Or is the plan to change the API to a Futures based interface before 1.0? Or do you plan to keep the sync API interface but have an async backend?

yazaddaruvala avatar Dec 24 '16 03:12 yazaddaruvala

I'm also curious about what @yazaddaruvala said. It would seem to me that Futures would be returned as a a way of future-proofing for async. Then again, maybe it's possible to accommodate handlers that return Futures and those that don't? Perhaps even using codegen/procedural macros.

I ask in light of tokio reaching 1.0, hopefully increasing the pace at which async infrastructure is developed.

blaenk avatar Jan 11 '17 22:01 blaenk

Tokio has reached version 0.1, not 1.0!

@yazaddaruvala To directly answer your question, there's no point to having the Rocket API know about any kind of asynchronous I/O as Rocket doesn't perform async. I/O. Having some notions of async. I/O that don't, in reality, perform the I/O asynchronously would be rather confusing.

I don't have a design I'm willing to commit to at this point. The Rust asynchronous I/O space is in heavy flux. The Tokio 0.1 release today is a milestone in reaching stability, but there's still a long road ahead. Rocket will be fully asynchronous in the future, but the approach must be made with care. Usability is of utmost importance, and performing and handling async. I/O with Rocket cannot be an exception.

SergioBenitez avatar Jan 11 '17 22:01 SergioBenitez

You're right! Clearly misread on my part.

blaenk avatar Jan 11 '17 22:01 blaenk

@SergioBenitez I can appreciate that it might be too early to make implementation decisions. However, its not clear from your comment or the rest of this thread, and I'm curious about your vision.

Does sync IO, fit with your current vision of what Rocket will be? Asked a different way: Is there a possibility that, a future 1.0 release of Rocket could use synchronous IO or can you guarantee that the 1.0 release of Rocket (whenever that comes out) will be based on some implementation of async IO?

yazaddaruvala avatar Jan 12 '17 09:01 yazaddaruvala

As I said in my previous comment: Rocket will be fully asynchronous in the future. This issue will not be resolved until the HTTP server backend is stabilized, and this issue is currently set to be resolved before 1.0. I consider asynchronous I/O to be a requirement for stability.

SergioBenitez avatar Jan 12 '17 09:01 SergioBenitez

BTW: Hyper just merged the tokio branch: https://github.com/hyperium/hyper/commit/2d2d5574a698e74e5102d39b9a9ab750860d92d1

flosse avatar Jan 18 '17 09:01 flosse

Do you guys think that HTTP 2 support is a separate issue? I don't think hyper supports it but the https://github.com/mlalic/solicit crate provides some degree of support.

thedrow avatar Jan 23 '17 09:01 thedrow

@thedrow Hyper plans on doing HTTP2 support before hitting 1.0 https://github.com/hyperium/hyper/issues/304

mgattozzi avatar Jan 23 '17 14:01 mgattozzi

@SergioBenitez Do I get this right, you don't have any plans to add futures as return values for handlers?

If so I think that icould be troublesome, because you may query a database in a handler which results mostly in IO. Returning a future would allow nice chaining.

jon-zu avatar Feb 22 '17 02:02 jon-zu

@JonasZ95 I certainly didn't say that.

SergioBenitez avatar Mar 09 '17 02:03 SergioBenitez

https://github.com/hyperium/hyper/issues/304#issuecomment-306962267

webbrandon avatar Jun 10 '17 16:06 webbrandon

FYI: hyper v0.11 just released :)

flosse avatar Jun 13 '17 22:06 flosse

Is this currently a development priority? If so, is someone working on it? I don't see a branch for this, so my guess is no.

I may be interested in helping port to hyper 0.11 (async), but I'm a bit unclear with the Rocket codebase (just started using it for a project) so I don't feel like I have a good intuition as to what a good API would look like.

beatgammit avatar Jul 25 '17 20:07 beatgammit

@beatgammit Maybe look at tokio-minihttp.

Right now Rocket handlers can return anything implementing Responder. As a straw man user-facing API, we might want to allow handlers that look like:

#[get("/")]
// is this the proper syntax?
fn hello() -> impl Future<String, io::Error> {
    future::ok(String::from("hello"))
}

lnicola avatar Jul 25 '17 20:07 lnicola

Futures Await now work on nightly: https://github.com/alexcrichton/futures-await

lilianmoraru avatar Sep 04 '17 06:09 lilianmoraru

Repost the question #559 here.

Does anyone have experience with actix-web. It seems that it support HTTP, HTTP2, async concurrency and websocket which are needed to implement a morden web server. Currently the Rocket use hyper as then web backend which limits HTTP2 and websocekt features. Shall we use actix-web to replace hyper to get all those features? If we decide not to change our backend, shall we split our rocket sugar syntax code into a separate project, so that we can reuse it's code to add the nice syntax for other web framework?

harryfei avatar Feb 12 '18 09:02 harryfei

Rocket looks so nice, but its synchronous nature is still a showstopper for services requiring to access external APIs while handling requests.

Any progress on bringing async into rocket like an optional feature for writing handlers? Maybe something like a custom Responder with a futures-cpupool as a back-end threadpool? This would allow using async and sync API at the same time, as far as I understand.

How complex would it be to implement an initial support for futures-backed Rocket? I could dive into it, but I definitely would like to hear a team vision on the topic.

mersinvald avatar Feb 14 '18 16:02 mersinvald

@mersinvald it would require updating hyper to 0.11 at the least in order to have async. However with the tokio reform moving to 0.2 soon and hyper 0.12 to follow not long afterwards it might be some time. Really though @SergioBenitez already said why he hasn't moved to async above early on in the thread:

I don't have a design I'm willing to commit to at this point. The Rust asynchronous I/O space is in heavy flux. The Tokio 0.1 release today is a milestone in reaching stability, but there's still a long road ahead. Rocket will be fully asynchronous in the future, but the approach must be made with care. Usability is of utmost importance, and performing and handling async. I/O with Rocket cannot be an exception.

mgattozzi avatar Feb 14 '18 18:02 mgattozzi

@mersinvald I experimented with something like that previously in 2 stages in this branch:

  • using synchronous handlers backed by futures-cpupool & hyper down to async connections ergonomically the same as currently but not really beneficial since you cannot use async handlers
  • introducing asynchronous handlers required enormous BC breaking changes and ergonomically much worse to use than synchronous handlers (This was before futures-await).

steffengy avatar Feb 26 '18 07:02 steffengy

I see that #491 and #209 form a chain of references pointing here. I've just run into the first issue (where address = "localhost" and Rocket binds to just IPv6 and not IPv4, causing the client to be unable to connect) and want to make sure it doesn't get forgotten about. :)

antmoth avatar Feb 28 '18 20:02 antmoth

using synchronous handlers backed by futures-cpupool & hyper down to async connections ergonomically the same as currently but not really beneficial since you cannot use async handlers

My two cents from the peanut gallery is that this is a reasonable way to start:

  • you know you want to use async hyper eventually, and this is a nice small step toward doing so that can be tested independently of rocket interface changes. If it's not actually harmful, I think it's worth doing for that reason alone.
  • it seems valuable for keepalive connections to not be consuming threads.
  • my (untested) impression is that there have been other performance improvements in hyper 0.10.x -> hyper 0.11.x.
  • there will be further improvements to hyper, such as HTTP 2 support in hyper 0.12.x, that are worth picking up.

scottlamb avatar Feb 28 '18 20:02 scottlamb

I propose using https://github.com/actix/actix-web as the HTTP layer. It has the following features

  • Extremely fast
  • Asynchronous
  • HTTP/2 support
  • TLS support
  • Middleware for CSRF, CORS, and others already included
  • High-level, which means that Rocket will in many cases be able to pass data directly to actix-web.
  • Very actively developed.

DemiMarie avatar Mar 28 '18 20:03 DemiMarie

Is there any update on this yet?

Also, I must say, I find the approach of actix rather odd... a) it ignores the community standard HTTP library that is hyper b) it chooses its own actor-based concurrency model rather than the community standard that is tokio (an event-loop-based model).

alexreg avatar Jun 05 '18 03:06 alexreg

[...] it chooses its own actor-based concurrency model rather than the community standard that is tokio (an event-loop-based model).

I'm sorry you're just wrong as you can see here the dependency is declared directly:

tokio = "0.1"

flosse avatar Jun 05 '18 21:06 flosse

Right. No need to be rude about it though.

Anyway, there is still my point a. And regarding b, I find it odd that actix would build itself on top of tokio and yet change the entire paradigm. Maybe someone could kindly clarify.

alexreg avatar Jun 05 '18 22:06 alexreg

Right. No need to be rude about it though.

I'm sorry, I didn't want to be rude.

Anyway, there is still my point a.

Yes but wouldn't say "it ignores the community standard HTTP library that is hyper" but it's a great showcase for Rust and a different approach. That doesn't mean it's good or bad as a layer for Rocket.

And regarding b, I find it odd that actix would build itself on top of tokio and yet change the entire paradigm. Maybe someone could kindly clarify.

Again, I think all projects can profit from learning from different paradigms. We should discuss the pro & cons of a paradigm within a concrete context instead of just talking about "odd" libraries.

flosse avatar Jun 05 '18 23:06 flosse

@flosse No worries. Anyway, I agree experimenting with different paradigms is a good thing, but Rocket is one of the most established web frameworks out there, so maybe not the best case for using such an experimental library. Anyway, I'm sure the devs of actix have a reason for not using hyper, but I'd certainly be curious to know what it is!

alexreg avatar Jun 05 '18 23:06 alexreg