Add WASM support

[AntoniosBarotsis]

Closes #34!

This was a bit of a rabbit hole to get into, even though the changes are (thankfully) tiny but this adds WASM support to the crate. My use case was using a downstream crate in Cloudflare workers and this crate was causing panics because of its time APIs.

The PR uses web-time which was made specifically for this purpose; MonotonicClock is now compatible with WASM environments at, from what I understand, no extra cost to non-WASM environments.

At the same time the library will simply re-export std::time when not using the wasm32-unknown-unknown target and will not pull in any dependencies.

^ From web-time's README

One small point that might cause confusion is when trying to type out a RateLimiter type which can look something like
limiter: Arc<RateLimiter<NotKeyed, InMemoryState, MonotonicClock, NoOpMiddleware<web_time::Instant>>>. If someone accidentally uses std::time::Instant, the errors are about Instant not implementing the traits that are outlined here which can be confusing to those trying to use this. For this I think for one documenting this use case is a must but also re-exporting web_time::Instant should come in handy.

My reasoning for the above is that while the existing docs are good and self explanatory, only someone that would try implementing his own Clock would likely take a look there; since this is an existing, internal Clock provided by the crate, there's a good chance most people using it won't have read that part of the docs.

You will also notice this PR adds getrandom = { version = "0.2", features = ["js"] }, this is required to get rand to compile in a WASM environment (see here).

Lastly, nothing of the included changes is put behind a feature flag but I personally don't think any of the changes could cause issues. Let me know what you think!

[antifuchs]

Wow-eeeee, this is amazing & exciting. I love that it's basically a 4-line diff hiding a ton of hard work by many people, especially you (:

I'll try to read up on what the implications are, but I'm super excited to get this merged asap - thank you for this!

[antifuchs]

Looks like my upstream github workflows use some deprecated version or something, which will prevent some runs from succeeding. I'm looking into it!

[AntoniosBarotsis]

Looking at the performance from this pr vs an older run from master performance seems to indeed be the same which is great!

This PR:

Benchmarking realtime_clock/mostly_allow/MonotonicClock: Collecting 100 samples in estimated 5.0001 s (126M iterations)
Benchmarking realtime_clock/mostly_allow/MonotonicClock: Analyzing
realtime_clock/mostly_allow/MonotonicClock
                        time:   [39.484 ns 39.570 ns 39.712 ns]
                        thrpt:  [25.181 Melem/s 25.272 Melem/s 25.326 Melem/s]
Found 10 outliers among 100 measurements (10.00%)
  2 (2.00%) high mild
  8 (8.00%) high severe

Master:

Benchmarking realtime_clock/mostly_allow/MonotonicClock: Collecting 100 samples in estimated 5.0000 s (129M iterations)
Benchmarking realtime_clock/mostly_allow/MonotonicClock: Analyzing
realtime_clock/mostly_allow/MonotonicClock
                        time:   [38.674 ns 38.693 ns 38.719 ns]
                        thrpt:  [25.827 Melem/s 25.844 Melem/s 25.857 Melem/s]
Found 12 outliers among 100 measurements (12.00%)
  1 (1.00%) low mild
  5 (5.00%) high mild
  6 (6.00%) high severe

So it is an effectively "free" change for anyone using the crate same as before, outside of WASM.

[AntoniosBarotsis]

I'm on Windows and only 1 test is failing by being too slow but I'm pretty sure that's because I use WSL on a sluggish laptop, everything else passes so once the CI upload artifact v3 thing is resolved everything should be green!

failures:

---- proceeds_keyed_n stdout ----
thread 'proceeds_keyed_n' panicked at governor/tests/future.rs:99:5:
assertion failed: `(left <= right) but here (left > right)`
  left: `238.7µs`,
 right: `200µs`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    proceeds_keyed_n

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.75%. Comparing base (3eafb7d) to head (eb38e1f).
Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #255   +/-   ##
=======================================
  Coverage   97.75%   97.75%           
=======================================
  Files          31       31           
  Lines        2049     2049           
=======================================
  Hits         2003     2003           
  Misses         46       46           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[antifuchs]

Looks like there are just no-std builds left due to a downstream dep not being fully no-std compatible (and a cargo-deny migration); I'll fix those on master in a bit.

[antifuchs]

OK, think master's CI status should be fixed now; please rebase & we can get this merged!

[antifuchs]

Oh, just one thought: How did you try this (what toolchains / tools)? I'd love to add that configuration to CI so we keep it working.

[AntoniosBarotsis]

Testing is a bit weird because from what I understand your code might need to be evaluated for it to crash, i.e. just because something compiles for WASM does not mean it will not panic when targeting WASM.

What I did was follow these instructions for the HTTP example and just run worker-build --release followed by npx wrangler dev and visiting the URL.

To illustrate what I mentioned earlier about things compiling even though they might lead to a panic, the following code compiles just fine

use axum::{routing::get, Router};
use tower_service::Service;
use worker::*;

fn router() -> Router {
    Router::new().route("/", get(root))
}

#[event(fetch)]
async fn fetch(
    req: HttpRequest,
    _env: Env,
    _ctx: Context,
) -> Result<axum::http::Response<axum::body::Body>> {
    console_error_panic_hook::set_once();

    let tmp = std::time::Instant::now();
  
    Ok(router().call(req).await?)
}

pub async fn root() -> &'static str {
    "Hello Axum!"
}

But once you visit the URL it panics because of the use of Instant. There has to be better ways of testing for stuff like this (my eye caught this for example which has to be what you're supposed to do) but since I haven't done anything WASM related before, I'm not too sure.

[AntoniosBarotsis]

I tried adding some tests via wasm-bindgen-test but it seems rather annoying getting them to actually work. At first they crash because of proptest, remove that and then rayon causes issues. I'm not sure if there's a way to only compile what's needed for those tests and nothing else, otherwise you'd probably need a separate crate for testing 🤔

[antifuchs]

Ah, yeah, I suppose we can make a sub-crate here that is only for testing. Thankfully the repo is already organized as a workspace.

That's just a follow-up item, though. No need to get busy on that in this PR! (:

[AntoniosBarotsis]

Nice! Is there anything else left to do here?

[antifuchs]

Think that should be all - I'll merge this and later see about testing & cutting a release. Thanks so much!

[AntoniosBarotsis]

If anything interesting pops up in the testing phase let me know as I'll also need to write some wasm tests in my own project 😅 thanks!