reqwest-middleware
reqwest-middleware copied to clipboard
feat: wasm32-unknown-unknown support
This replaces task-local-extensions with http's extensions, as http was already in the dependency closure anyway and the other features of task-local-extensions (that required an incompatible-with-wasm part of tokio) were not used anyway.
I tried it out in the project that triggered this PR, and it seems to work perfectly fine!
Thanks for the PR! I had the same idea about http extensions in #77.
The extra wasm support code is definitely interesting. It's going to take me a bit to review it as I'm not an expert in wasm and reqwest
:+1: To explain the wasm-related changes:
- task-local-extensions requires tokio with the rt feature, which does not support the wasm async runtime
- reqwest does not support the timeout function on wasm32, so I'm just cfg-ing it out
- reqwest futures are not Send on wasm32, but wasm32 has a single thread anyway so we can just make the async-trait and BoxFuture !Send on wasm32
- reqwest on wasm does not depend on hyper (which does not support wasm anyway), so I'm cfg-ing out the workaround for a reqwest limitation that requires a direct dependency on hyper from reqwest-middleware
- tokio::time does not support wasm, so I'm using wasm-timer
- reqwest on wasm32 does not have .is_connect() for the Error, so I'm cfg-ing the call out too (this is arguably an issue in reqwest upstream, that should have an is_connect() that always returns false)
Hopefully it can make the code review easier! :)
Hey! Just checking if you had the time to have a look at my replies to your comments? :)
Yeah, code wise I'm happy. I'm still going to have a think about future abstractions. Since this is a breaking change because it replaces the extensions type. Perhaps we could modify task-local-extensions to be a wrapper around http::Extensions and support wasm32 and be fully compatible
Maintenance wise, I'd like a way to test it to ensure we don't break things in future by forgetting attributes. Do you think you could come up with a mechanism to compile and run our tests on wasm in github CI?
Makes sense, please let me know what your conclusions are about http::Extensions and task_local_extensions!
As for testing on wasm in github CI, unfortunately wasm-timer only supports wasm32-unknown-unknown (with web_sys support) and not wasm32-wasi (which is a very different platform). And widespread testing tools only support wasm32-wasi for now, which makes sense as otherwise one needs to actually spawn a browser to run tests. (To be precise, the webassembly-test crate can run tests for wasm32-unknown-unknown… but does not actually run them inside a browser)
So the best bet to run tests would I think be to use fantoccini and geckodriver to run a compiled-to-wasm version of the tests for this crate; maybe using webassembly-test to at least be able to reuse some infra of making it a test runner.
However, considering the small amount of code that is wasm32-specific after this PR, I don't think this is worth it, as the resulting codebase would probably end up larger than all of reqwest-middleware.
So maybe it'd make sense to just have CI compile to wasm32-unknown-unknown, and check that no regressions are made there? I've just added a commit that does that :)
This has been released