pimeys / rust-web-push

A Web Push library for Rust
Apache License 2.0
113 stars 21 forks source link

Make the crate work either with Tokio or async-std #25

Closed pimeys closed 2 years ago

pimeys commented 3 years ago

By default, uses Tokio 1.0. If wanting to use with async-std, please disable default features and compile with the feature flag rt-async-std.

pimeys commented 3 years ago

@platy @tiesselune please check this branch out. This tries to make the crate working with both major runtimes. It might have some performance limitations, but this is the way I'd like the crate to go in the future.

platy commented 3 years ago

Hi @pimeys , this does look very interesting - I especially like the look of the async-h1 crate, I haven't seen it before and it really has the api I've been looking for.

On first glance it looks like the runtimes are only needed in order to provide a pool, in that case I would prefer them both to be optional, I'll try to make a PR today to show what I mean.

pimeys commented 3 years ago

Hey @platy! We need the runtimes for TcpStream, which on Tokio will just panic if ran on async-std, and on async-std will create its own runtime when ran on Tokio.

Also the connection management here is probably not the most performant. I have no ways anymore to test this with massive traffic, but I'd take a look a bit how hyper does the pooling, and then implement something similar here too. The file in question would be the pool.rs. Some things: how many connections should we reserve per host? Also we don't manage TCP keep-alive here at all. Tokio doesn't give the possibility to do this in their TcpStream, so we must start with the std::net::TcpStream, then make it non-blocking, set the keep-alive and make it async either with mio or smol's Async.

Some work to do, maybe even make a new crate for this.

pimeys commented 3 years ago

See here how to construct an async TcpStream from a raw socket. Some unsafe is needed too...

platy commented 3 years ago

Yeah I see again that it is more complicated than I thought! 😆

I was wondering if it was possible to do it in a really runtime-independent way, rather than selecting between these 2. For my use case, which is push notifications to quite a small number of users, even single threaded and blocking, I could overload the users. So for simplicity I would rather not use async and just block. I would quite like to be able to use a library like this with no runtime and have interfaces which convert types to/from http request/response objects. Those objects could then be passed to whichever http client/pool I want to use. Something like the types from the http/http-types crates, though neither of them have as wide adoption as I would like.

pimeys commented 3 years ago

If you want, you can take a look into this branch, and try to make it so that IO is a feature flag. Without it, you'd have just something that builds a Request and something that then parses the response. You'd then need to implement some IO by yourself using the HTTP client of your choice. The crate http-types is quite nice due to it being supported many of the HTTP clients in the ecosystem...

This crate has quite isolated IO already as you can read from the source. It's basically just the client that does it.

platy commented 3 years ago

@pimeys I've made a PR into this one which puts the client behind a feature flag : https://github.com/pimeys/rust-web-push/pull/26

pimeys commented 3 years ago

Going to try to push this to finale this weekend. Missing: keep-alive and better connection handling.