workers-rs Switch to using `http::Request` and `http::Response`

This PR switches the library to use http for its request and response types rather than providing its own. This reduces the amount of code needed to maintain and gives access to the wider Rust ecosystem. With this PR it becomes possible to use libraries like axum directly with workers-rs.

The largest changes in this PR include:

Remove worker::Request and worker::Response and use http::Request and http::Response instead.
Provide a Body type that wraps any http_body::Body. All request and response bodies become streaming by default.
Remove the routing logic

There are a couple of points that need to be addressed before merging this PR:

[x] should Body implement .json() and .text() as functions for ease of use? When relying on axum this is not necessary but when writing a worker without an additional layer it would reduce code slightly.
[x] should other library functions, such as R2, return or wrap Body as well to unify interfaces.
[ ] all documentation needs to be updated.

An example of calling axum from a worker:

fn router() -> Router<AppState, Body> {
    Router::new()
        .route("/application", post(application))
        .route("/users", get(users))
        .route("/.well-known/jwks.json", get(jwks))
        .nest("/oauth", oauth::router())
}

#[event(fetch)]
async fn fetch(req: Request<Body>, env: Env, _ctx: Context) -> Result<Response<Body>> {
    console_error_panic_hook::set_once();

    let env = Arc::new(env);
    let kv = Arc::new(env.kv("KV").unwrap());

    let res = router()
        .with_state(AppState { env, kv })
        .call(req)
        .await
        .unwrap();

    Ok(res.map(Body::new))
}

Feb 27 '23 15:02 SebastiaanYN

Love this!!

Mar 03 '23 22:03 spencerbart

@SebastiaanYN what can I do to help this move through?

Mar 05 '23 21:03 spencerbart

Providing feedback and pointing out any issues you run into is welcomed. Outside of that there's currently not much to do

Mar 05 '23 21:03 SebastiaanYN

Hey @SebastiaanYN ,

I've been playing around with the this branch and I've been unable to get WebSocket's to work with durable objects.

I've created a small example that reproduces the issue: https://github.com/jdon/worker-websocket

Running that repo locally using wrangler dev --local, works as expected, I'm able to connect to ws://127.0.0.1:8787/websocket using insomnia and it'll echo back what I send to the server.

After publishing the worker using wrangler publish insomnia fails to connect to the WebSocket at: wss://websocket-worker.jdon.workers.dev/websocket.

When looking at the Real-time Logs in the Cloudflare dashboards any requests to the WebSocket have the status of Canceled. Other requests to the durable object like http://websocket-worker.jdon.workers.dev/add work as expected. I don't see any errors in the logs for the cancelled requests, so I'm not sure if the issue is with workers-rs or my code.

Mar 07 '23 01:03 jdon

@jdon some of the issues were related to response setting a body when it shouldn't. That's now fixed. The other issue is that the request you make to the durable object does not set the upgrade: websocket header as documented on https://developers.cloudflare.com/workers/learning/using-websockets/#writing-a-websocket-client. You can set the headers as following:

stub.fetch_with_request(
    http::Request::get(format!("http://example.com/{path}"))
        .header("upgrade", "websocket")
        .body(().into()),
)
.await;

Thanks for pointing out the issue :)

Mar 14 '23 21:03 SebastiaanYN

Thanks @SebastiaanYN , I really appreciate you taking the time to look into the issue.

Updating the branch and including the upgrade header fixed it.

Mar 14 '23 22:03 jdon

The readme code still has to be updated

Apr 11 '23 16:04 SebastiaanYN

@SebastiaanYN Do you need any help getting this PR across the finish line? I'd be happy to help.

May 11 '23 13:05 nicksrandall

Just to give a bit of a status update on where this is for those wondering, this is still on the radar for a 0.1.x release. There's some further work tangentially related to this PR that needs to be done (ideally) before this gets merged in to make this a smooth transition. That being said if anyone tries this and runs into issues in the meantime please let us know

May 31 '23 02:05 zebp

Any update on this PR? eagerly waiting for this to merge for the next version.

Sep 24 '23 02:09 sam-rusty

Also looking for this to be merged. The fact that we are locked into using the provided "router" is kinda ridiculous

Nov 19 '23 16:11 JorgeAtPaladin

Besides the obvious, that you can always create a fork, merge this change and use the git reference in cargo and off you go. As simple as that...

But actually we are not, there is this approach to run axum for instance https://logankeenan.com/posts/rust-axum-and-cloudflare-workers/

And in really you don't even have to use workers-rs, or rust, as long as you can compile to wasm and call it from a JS function you can use what you want https://developers.cloudflare.com/workers/runtime-apis/webassembly/javascript/

Nov 19 '23 23:11 spigaz

@spigaz The mappings look great, thanks! Would love for these to be moved into this repository for maintenance and security reasons.

@kflansburg Can these two functions be moved into the workers-rs repo?

https://github.com/logankeenan/axum-cloudflare-adapter/blob/a475acb54a9f1667c588d7e7a2c928e94afa92ad/adapter/src/lib.rs#L66-L116

Nov 20 '23 11:11 JorgeAtPaladin

@kflansburg Can these two functions be moved into the workers-rs repo?

https://github.com/logankeenan/axum-cloudflare-adapter/blob/a475acb54a9f1667c588d7e7a2c928e94afa92ad/adapter/src/lib.rs#L66-L116

Those look useful, is it correct to say that they are not needed if we landed this PR instead? I can sync with @zebp on what the remaining blockers are.

Nov 20 '23 16:11 kflansburg

Those look useful, is it correct to say that they are not needed if we landed this PR instead? I can sync with @zebp on what the remaining blockers are.

Yeah, they wouldn't be needed once this PR gets merged in. Sebastiaan created a doc internally about this PR and some other work he'd like for workers-rs, I'll send you the link.

Nov 20 '23 17:11 zebp

@zebp @kflansburg , indeed! I think supporting the native request and response is indeed superior. If this is a priority internally I agree the translation functions are not needed to be implemented in this repository.

Thank you all for prioritizing this internally.

Nov 20 '23 21:11 JorgeAtPaladin

I would love to see this merged into the main branch. Is there anything that community members can contribute to get this PR over the finish line? The OP only mentions updating the documentation as an open item.

Mar 06 '24 10:03 avsaase

I would love to see this merged into the main branch. Is there anything that community members can contribute to get this PR over the finish line? The OP only mentions updating the documentation as an open item.

See #440, there was a bug in streaming bodies that took us a while to track down. What remains is a gap in converting tests over (many are just commented out). If someone could help out with this it would be greatly appreciated.

Mar 06 '24 14:03 kflansburg

@kflansburg I went through the PR diff and I don't see any tests that are commented out or that otherwise need to be changed.

Mar 06 '24 20:03 avsaase

@avsaase Unfortunately I cannot remember the exact place I saw the changes. It's also confusing because #440 is based on a branch that Zeb put together to debug. Anyway, if you look at failing tests for #440 you can see that there are a bunch related to R2 and queues (among others) which I believe never had their routes re-implemented for http. I may be wrong though.

Mar 06 '24 20:03 kflansburg

Yeah, for me as an outsider it's a bit hard to make sense of all the branches and follow what has been done and what is still to do. How I understand it now is https://github.com/cloudflare/workers-rs/pull/440 targets the zeb/http-types branch which looks like a rebase on main of this PR's base branch with one additional commit. Both branches have conflicts with main which for someone with limited knowledge of the library's internals don't seem trivial to resolve.

I'm afraid I can't be of much help with the failing tests in https://github.com/cloudflare/workers-rs/pull/440 because I'm not familiar with the test harness (and JS and JS tooling for that matter) and I'm not able to reproduce the failing tests locally.

I hope someone else is able to get this PR moving again. Being able to use Axum and it ecosystem (without adapters) would be a huge usability improvement to writing cloudflare workers in Rust.

Mar 06 '24 22:03 avsaase

I'm giving it a look, some are already working, r2, service-bindings and queues, about half I guess.

I had to port some things from the latest version for service-bindings to work, I believe they should be cleared away on rebase.

I'm working on getting streaming to work.

I just posted to avoid duplicate work, I hope to continue tomorrow.

Mar 08 '24 23:03 spigaz

Good news: I only have the D1 tests missing. I expect to continue tomorrow. https://github.com/spigaz/workers-rs/tree/pr-440

Once reviewed, the rebase...

Mar 10 '24 00:03 spigaz

This means #473 > review > rebase...

Mar 10 '24 20:03 spigaz

That is amazing! Great job!!!! <3

Mar 10 '24 21:03 sam-rusty

Regarding the use of unsafe in this PR, do I understand correctly that this is safe because the worker runtime will always execute a worker on a single thread?

Mar 11 '24 16:03 avsaase

Regarding the use of unsafe in this PR, do I understand correctly that this is safe because the worker runtime will always execute a worker on a single thread?

Yes

Mar 11 '24 17:03 kflansburg

Closed in favor of #474

Mar 11 '24 19:03 kflansburg