fix(wasm): avoid std::time::Instant::now() and expand testing#625
fix(wasm): avoid std::time::Instant::now() and expand testing#625pmarks wants to merge 4 commits intoapache:mainfrom
Conversation
Currently retry and throttle can crash on WASM due to hitting std::time::Instant::now(), and MultiPart upload relies on tokio::task::JoinSet, which doesn't work on WASM. So do the following: - Use n0-futures, which re-exports tokio types on native, and supplies browser-compatible versions on wasm32. Used to get a replacement for tokio::task::JoinSet, used for MultiPart uploads - Use sleep() from gloo-timers on wasm instead of tokio::time::sleep which relies on std::Instant::now() - To test WASM, we use #[wasm_bindgen_test] instead of #[tokio::test] to mark many existing tests. We can conditionally import the appropriate macro, renaming the macro as `async_test` then decorate the tests with #[async_test]. These tests will now run under the existing WASM CI.
… async tests in wasm.
| pub(crate) async fn sleep(duration: Duration) { | ||
| use send_wrapper::SendWrapper; | ||
| if !duration.is_zero() { | ||
| SendWrapper::new(gloo_timers::future::sleep(duration)).await |
There was a problem hiding this comment.
The inner future is not actually Send because it contains JS objects. SendWrapper makes it Send, with dynamic checking that it is not used on another thread. This works for standard wasm32-unknown-unknown, and will crash 'safely' if used across threads somehow with Web Workers, so we can figure out how to fix that later.
|
This is a rather high-level question for |
|
Perhaps we should have a |
|
Yeah, I would feel more comfortable w/ an explicit |
3 participants
Proposed fix to #624 - opening for discussion, I'm happy to rework this.
Currently retry and throttle can crash on WASM due to hitting std::time::Instant::now(), and MultiPart upload relies on tokio::task::JoinSet, which doesn't work on WASM.
TODO: figure out a unit test that actually exercises
RetryableRequestin WASM. Existing tests can't run because they need MockServer, which needs TcpListener.Which issue does this PR close?
Closes #624
Rationale for this change
Solidify WASM support by fixing a couple small issues and greatly expanding WASM test coverage.
What changes are included in this PR?
async_testthen decorate the tests with #[async_test]. These tests will now run under the existing WASM CI.Are there any user-facing changes?
The main Error enum will change for the WASM build - it will now expose
n0_future::task::JoinErrorinstead oftokio::task::JoinError- this is a breaking change.