Add tokio feature and make tokio optional #1054

fisherdarling · 2022-05-25T11:04:21Z

Motivation

I'm trying to use axum's routing and request extraction hardware for a Cloudflare Worker. Workers run in a WASM context (v8 isolate) and therefore tokio cannot be used.

Solution

This PR adds a tokio feature that enables tokio as well as hyper/tcp and hyper/server. The tokio feature also enables SSE and extract::connect_info.

This is a breaking change since this breaks usages of default_features = false.

davidpdrsn · 2022-05-25T11:08:53Z

Not quite sure how but maybe we should add a step to CI that somehow verifies that tokio isn't pulled in when this feature isn't enabled. Otherwise I imagine its the kind of thing we could easily break by accident.

The feature should also be mentioned here.

fisherdarling · 2022-05-25T11:12:48Z

Not quite sure how but maybe we should add a step to CI that somehow verifies that tokio isn't pulled in when this feature isn't enabled. Otherwise I imagine its the kind of thing we could easily break by accident.

The feature should also be mentioned here.

That's a really good idea. Maybe a combination of cargo metadata / cargo tree and grepping for tokio might suffice. The only way to truly remove tokio is make it optional for hyper, since hyper always pulls in tokio. I'm looking into that now.

And I'll add the feature to the docs.

Are we good with naming it server?

davidpdrsn · 2022-05-25T11:24:41Z

That's a really good idea. Maybe a combination of cargo metadata / cargo tree and grepping for tokio might suffice. The only way to truly remove tokio is make it optional for hyper, since hyper always pulls in tokio. I'm looking into that now.

Yeah something like that could probably work.

Are we good with naming it server?

How about just calling the feature tokio? Maybe thats more clear 🤔

davidpdrsn · 2022-05-27T10:12:10Z

@fisherdarling whats the status of this? I remember we talked a bunch in Discord but don't remember if we reached any conclusions.

fisherdarling · 2022-05-27T12:52:43Z

@davidpdrsn I'm currently blocked on trying to figure out CI. cargo check -p axum --no-default-features --target wasm32-unknown-unknown is failing for me, even though I can successfully use this fork to compile and run a router in a wasm context. I think dead-code elimination is what's allowing me to compile axum as a dependency with --target wasm32-unknown-unknown.

Maybe an example needs to be created that uses axum just for its router, so that mio won't be compiled? Checking that example with the wasm target might work as a CI check.

I'm going to be traveling the next two weeks, but I might have time tonight or this weekend to finish it up.

examples/simple-router-wasm/Cargo.toml

davidpdrsn · 2022-06-11T19:36:27Z

@fisherdarling You mentioned on discord that you had run into issues with having to run !Send futures and how that might be a show stopper for this. What do you think? Should we close this while you try and get it working? I'm a bit reluctant to merge this if the use case we had in mind doesn't actually work.

fisherdarling · 2022-06-12T11:10:25Z

@davidpdrsn I was actually able to get it to work! I've actually switched almost all of my worker code to use the router, which has been extremely nice to have. I just needed a MakeSendFuture like you suggested in the discord.

Axum regularly has breaking changes, so I think removing the tokio feature in the future if nobody uses it would be fine / acceptable. The changes themselves are pretty minimal and don't really infect the code-base, so I think development overhead of having a tokio feature is not too high?

Honestly, I'm happy to just maintain my own fork for now if you don't want to merge it in or are hesitant to do so.

The "true" fix would be to separate the router into an axum-router crate and let people just pull that in.

jplatte · 2022-06-12T14:40:22Z

So do you need the Router for your wasm use case or not? If not, should that be the unit that's made optional maybe?

fisherdarling · 2022-06-12T17:35:46Z

So do you need the Router for your wasm use case or not? If not, should that be the unit that's made optional maybe?

@jplatte I do need the Router for my wasm use case and this PR does allow me to use the Router in a wasm context.

jplatte · 2022-06-12T21:30:50Z

Then what did you mean by

The "true" fix would be to separate the router into an axum-router crate and let people just pull that in.

fisherdarling · 2022-06-13T09:46:56Z

Then what did you mean by

The "true" fix would be to separate the router into an axum-router crate and let people just pull that in.

Sorry, that was definitely a little confusing and needs some context.

Just to be clear, my overall goal is to use anaxum::Router for a wasm32-unknown-unknown target.

Everything that axum provides that requires an underlying runtime or server is not necessary for my use case and generally leads to compilation issues for wasm targets. That's why this PR adds a feature flag for that code.

All of the code that I do want, is all of the awesome code that makes it easy to work with http::{Request, Response}. That includes the Router, the great IntoResponse impls, and the extractors that deal with just the request itself (minus things like connection info). This PR allows me to turn off the tokio parts while still being able to use the features I want.

What I meant by this:

The "true" fix would be to separate the router into an axum-router crate and let people just pull that in.

Is that I think it would be beneficial to have the axum crate's router/extractors be separated into their own crates:

axum-extractors would be all of the "builtin" extractors (essentially everything under axum/src/extract). Extractors that require a runtime, or rather, extractors that may not be needed in every use case, should be feature flagged. For example, the json extractor (which already is behind a flag) and the ConnectInfo extractor.
axum-router would provide the actual router type and the interactions with tower and http. This would be essentially everything in axum except for the extractors. Anything involving tokio directly would be behind a non-default feature flag. This code could (?) also live under axum-core.
axum would then mainly just be re-exports of axum-extractors and axum-router/core along with any helper utilities. It would have essentially the same API as it does now.

With those crates split out, I could just pull in axum and the extractors I want behind a feature flag, or I could stay "lightweight" and pull in axum-router and the extractors I want from axum-extractors. The latter option would hopefully help with code size. I could even pull in just axum-router and then implement the extractors I want by hand.

I think this is something that would be beneficial, but I also think that the number of people that would directly benefit from this is rather low (just me right now). Most people (everyone so far?) are running axum on a server with tokio and don't care about nor need fine-grained crate imports.

Overall, the current PR lets me use the parts of the crate I love for a wasm target. Sorry if this is kind of a ramble.

davidpdrsn · 2022-06-13T11:59:35Z

The idea of putting the router in its own crate has come up before but I'm a bit hesitant towards that. I'm worried we'll end up with too many axum-* crates that'll be hard for people to understand. For context tower used to be split into separate crates but was merged into one with feature flags because users complained.

So I'm in favor of keeping Router in axum and then adding features. Basically what this PR does.

I'm gonna try and take a look at this this week but since its a breaking change it wont be released for a while. We don't have any concrete plans for 0.6 and there are some things in the pipeline I wanna work on first (such as matchit/routing changes)

fisherdarling · 2022-06-13T12:09:57Z

I completely agree and understand. I remember when tokio had so many tokio_* crates and how that was confusing for many people.

I'm gonna try and take a look at this this week but since its a breaking change it wont be released for a while. We don't have any concrete plans for 0.6 and there are some things in the pipeline I wanna work on first (such as matchit/routing changes)

Sounds good, I'll try to keep the branch updated to main.

davidpdrsn

Really awesome work! Sorry for taking so long to review it. I think it looks good but I'm curios if tokio-rs/tokio#4716 is relevant here.

davidpdrsn · 2022-07-13T14:37:57Z

examples/simple-router-wasm/Cargo.toml

@@ -0,0 +1,11 @@
+[package]
+name = "simple-router-wasm"


Suggested change

name = "simple-router-wasm"

name = "example-simple-router-wasm"

All our other examples are named like this.

Could consider that since they're now in their own workspace but we should change that in a stand alone PR.

davidpdrsn · 2022-07-13T14:38:18Z

examples/simple-router-wasm/src/main.rs

+//! Run with
+//!
+//! ```not_rust
+//! cd examples && cargo run -p simple-router-wasm


Suggested change

//! cd examples && cargo run -p simple-router-wasm

//! cd examples && cargo run -p example-simple-router-wasm

davidpdrsn · 2022-07-13T14:40:37Z

examples/simple-router-wasm/Cargo.toml

+publish = false
+
+[dependencies]
+axum = { path = "../../axum", default-features = false }


Maybe call this out specifically so users see it.

Suggested change

axum = { path = "../../axum", default-features = false }

# `default-features = false` to not depend on tokio which doesn't support compiling to wasm

axum = { path = "../../axum", default-features = false }

logankeenan · 2022-08-26T03:07:21Z

@fisherdarling

I think this is something that would be beneficial, but I also think that the number of people that would directly benefit from this is rather low (just me right now).

I'm very interested in this and seeing this PR made my day! I was able to get a web app running in a CF worker using a fork of Tide. However, my PR for WASM support is stagnant. I'd be interested in helping out in any way to help move this PR along.

I was also able to "run" the Tide web app as an SPA, I imagine this PR would allow for the same. I'm going to pull this PR down and give it a try when I have time this weekend.

logankeenan · 2022-09-01T02:42:52Z

This PR works well running in the browser. I made a simple example repo with a demo.

@davidpdrsn - thanks for your help on Discord!

kpietraszko · 2022-09-11T17:45:27Z

On the other hand, this PR doesn't build for me. As you mentioned above, it still pulls tokio because of tower and hyper, as shown in this reverse dependency tree:

tokio v1.21.0
├── hyper v0.14.20
│   └── axum v0.5.7 (https://github.com/fisherdarling/axum?branch=feat/optional-tokio#ff545b78)
│       └── (my crate)
└── tower v0.4.13
    ├── axum v0.5.7 (https://github.com/fisherdarling/axum?branch=feat/optional-tokio#ff545b78) (*)
    └── tower-http v0.3.4
        └── axum v0.5.7 (https://github.com/fisherdarling/axum?branch=feat/optional-tokio#ff545b78) (*)

Which wouldn't be horrible, because some tokio features do work on wasm32-unknown-unknown.
The build fails for me, because axum uses tower's make feature, which in turn uses tokio's std-io feature. It's possible this feature is not really necessary.

edit: Removing the "make" feature in Cargo.toml (l. 44) makes it build 🎉 It seems to be used in 1 test utility, so don't just remove that in this PR

fisherdarling · 2022-09-14T16:35:15Z

Hey all, sorry for being MIA! I just started a new job and have been pretty busy these past couple of months.

@logankeenan I'm glad the PR worked for you and that it benefits you! I would love some help moving this PR along if you can. I won't have the time to work on it for a while. I think next steps are to evaluate if the wasm-wasi change can be used and is helpful. If you want to start from scratch in another PR feel free to. I'll close this one and link to the new one.

@kpietraszko It has to pull in tokio right now because of AsyncRead and AsyncWrite. Super annoying but until that's split out into its own crate we're stuck.

fisherdarling · 2022-09-14T16:46:50Z

@logankeenan feel free to ping me @fisher in the axum Discord if you'd like :)

kpietraszko · 2022-09-14T19:13:48Z

@fisherdarling WASI support is a great addition, but the use-case is a bit different. For example Cloudflare Workers don't support WASI modules out of the box, only WASM. One would need to create their own JS shim I believe.

That said there are some services that allow hosting WASI, such as Fastly.

logankeenan · 2022-09-15T01:32:36Z

@fisherdarling

I haven't worked with WASI before, but that doesn't mean I couldn't learn more about it. Could that work potentially be done in another PR?

PS, congratulations on the new job!

logankeenan · 2022-09-17T13:48:07Z

@fisherdarling - I created a new PR based on your branch.

cc @kpietraszko

davidpdrsn · 2022-09-18T17:47:27Z

Let's continue the in #1382

@fisherdarling thanks for your work on this.

jplatte approved these changes May 25, 2022

View reviewed changes

jplatte added the breaking change A PR that makes a breaking change. label May 25, 2022

fisherdarling force-pushed the feat/optional-tokio branch from 0466105 to c413265 Compare May 25, 2022 11:15

fisherdarling force-pushed the feat/optional-tokio branch 2 times, most recently from 436ae4a to 1b4d13d Compare May 25, 2022 13:45

fisherdarling changed the title ~~Add server feature and make tokio optional~~ Add tokio feature and make tokio optional May 27, 2022

fisherdarling force-pushed the feat/optional-tokio branch 2 times, most recently from 40ff234 to 3a2dd86 Compare June 8, 2022 16:43

jplatte reviewed Jun 8, 2022

View reviewed changes

examples/simple-router-wasm/Cargo.toml Outdated Show resolved Hide resolved

fisherdarling force-pushed the feat/optional-tokio branch 4 times, most recently from 42774ad to b353b27 Compare June 8, 2022 17:16

fisherdarling force-pushed the feat/optional-tokio branch from b353b27 to df3e787 Compare June 12, 2022 10:45

add server feature and make tokio and hyper/server and tcp optional

ff545b7

fisherdarling force-pushed the feat/optional-tokio branch from df3e787 to ff545b7 Compare June 12, 2022 10:56

davidpdrsn added this to the 0.6 milestone Jun 25, 2022

davidpdrsn removed the breaking change A PR that makes a breaking change. label Jul 13, 2022

davidpdrsn reviewed Jul 13, 2022

View reviewed changes

davidpdrsn mentioned this pull request Aug 23, 2022

Bump version of all crates #1310

Merged

Altair-Bueno mentioned this pull request Aug 24, 2022

Proposal: Break down Router into components #1318

Closed

logankeenan mentioned this pull request Sep 17, 2022

Add tokio feature & make tokio optional for WASM support #1382

Merged

davidpdrsn closed this Sep 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tokio feature and make tokio optional #1054

Add tokio feature and make tokio optional #1054

fisherdarling commented May 25, 2022 •

edited

davidpdrsn commented May 25, 2022

fisherdarling commented May 25, 2022

davidpdrsn commented May 25, 2022

davidpdrsn commented May 27, 2022

fisherdarling commented May 27, 2022 •

edited

davidpdrsn commented Jun 11, 2022

fisherdarling commented Jun 12, 2022

jplatte commented Jun 12, 2022

fisherdarling commented Jun 12, 2022 •

edited

jplatte commented Jun 12, 2022

fisherdarling commented Jun 13, 2022 •

edited

davidpdrsn commented Jun 13, 2022 •

edited

fisherdarling commented Jun 13, 2022

davidpdrsn left a comment

davidpdrsn Jul 13, 2022

davidpdrsn Jul 13, 2022

davidpdrsn Jul 13, 2022

davidpdrsn Jul 13, 2022

logankeenan commented Aug 26, 2022 •

edited

logankeenan commented Sep 1, 2022

kpietraszko commented Sep 11, 2022 •

edited

fisherdarling commented Sep 14, 2022

fisherdarling commented Sep 14, 2022

kpietraszko commented Sep 14, 2022

logankeenan commented Sep 15, 2022

logankeenan commented Sep 17, 2022

davidpdrsn commented Sep 18, 2022

	name = "simple-router-wasm"
	name = "example-simple-router-wasm"

	//! cd examples && cargo run -p simple-router-wasm
	//! cd examples && cargo run -p example-simple-router-wasm

	axum = { path = "../../axum", default-features = false }
	# `default-features = false` to not depend on tokio which doesn't support compiling to wasm
	axum = { path = "../../axum", default-features = false }

Add tokio feature and make tokio optional #1054

Add tokio feature and make tokio optional #1054

Conversation

fisherdarling commented May 25, 2022 • edited

Motivation

Solution

davidpdrsn commented May 25, 2022

fisherdarling commented May 25, 2022

davidpdrsn commented May 25, 2022

davidpdrsn commented May 27, 2022

fisherdarling commented May 27, 2022 • edited

davidpdrsn commented Jun 11, 2022

fisherdarling commented Jun 12, 2022

jplatte commented Jun 12, 2022

fisherdarling commented Jun 12, 2022 • edited

jplatte commented Jun 12, 2022

fisherdarling commented Jun 13, 2022 • edited

davidpdrsn commented Jun 13, 2022 • edited

fisherdarling commented Jun 13, 2022

davidpdrsn left a comment

Choose a reason for hiding this comment

davidpdrsn Jul 13, 2022

Choose a reason for hiding this comment

davidpdrsn Jul 13, 2022

Choose a reason for hiding this comment

davidpdrsn Jul 13, 2022

Choose a reason for hiding this comment

davidpdrsn Jul 13, 2022

Choose a reason for hiding this comment

logankeenan commented Aug 26, 2022 • edited

logankeenan commented Sep 1, 2022

kpietraszko commented Sep 11, 2022 • edited

fisherdarling commented Sep 14, 2022

fisherdarling commented Sep 14, 2022

kpietraszko commented Sep 14, 2022

logankeenan commented Sep 15, 2022

logankeenan commented Sep 17, 2022

davidpdrsn commented Sep 18, 2022

fisherdarling commented May 25, 2022 •

edited

fisherdarling commented May 27, 2022 •

edited

fisherdarling commented Jun 12, 2022 •

edited

fisherdarling commented Jun 13, 2022 •

edited

davidpdrsn commented Jun 13, 2022 •

edited

logankeenan commented Aug 26, 2022 •

edited

kpietraszko commented Sep 11, 2022 •

edited