Don't depend on a particular ahash rng

[SamWilsn]

I'm not very familiar with ahash, or WASM in general, but it seems like ahash's default randomness source panics when run inside a WASM environment. This change should make it so that jsonschema uses the runtime randomness source by default, but will not require it when included with default-features = false.

[Stranger6667]

Oh, interesting! Could you share some more info about that panic inside WASM env?

[codecov[bot]]

Codecov Report

Merging #401 (ecbc135) into master (f44f195) will decrease coverage by 0.65%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #401      +/-   ##
==========================================
- Coverage   79.52%   78.87%   -0.66%     
==========================================
  Files          52       54       +2     
  Lines        4455     4497      +42     
==========================================
+ Hits         3543     3547       +4     
- Misses        912      950      +38     

Impacted Files	Coverage Δ
jsonschema/src/lib.rs	57.14% <0.00%> (ø)
jsonschema/src/main.rs	0.00% <0.00%> (ø)
jsonschema/src/resolver.rs	78.51% <0.00%> (+0.64%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[SamWilsn]

Sure! It's from inside a GitHub action, so I don't have a ton of detail, but this is the backtrace:

 wasm://wasm/035f2c1a:1

RuntimeError: unreachable
    at __rust_start_panic (wasm://wasm/035f2c1a:wasm-function[29902]:0x995f8d)
    at rust_panic (wasm://wasm/035f2c1a:wasm-function[28684]:0x98dfbb)
    at std::panicking::rust_panic_with_hook::h56566d5bea7a37c0 (wasm://wasm/035f2c1a:wasm-function[12547]:0x7f7625)
    at std::panicking::begin_panic_handler::{{closure}}::h03bdc827acd443d9 (wasm://wasm/035f2c1a:wasm-function[14777]:0x85961e)
    at std::sys_common::backtrace::__rust_end_short_backtrace::h9e7fb40f8771dce2 (wasm://wasm/035f2c1a:wasm-function[28353]:0x98a7d3)
    at rust_begin_unwind (wasm://wasm/035f2c1a:wasm-function[21798]:0x91e971)
    at core::panicking::panic_fmt::h9bfc1f59490d9b53 (wasm://wasm/035f2c1a:wasm-function[26863]:0x97782b)
    at core::result::unwrap_failed::h922cc556d7bf74df (wasm://wasm/035f2c1a:wasm-function[17235]:0x8ad6b2)
    at core::result::Result<T,E>::expect::hd3153d6e5de234a4 (wasm://wasm/035f2c1a:wasm-function[15729]:0x87c178)
    at ahash::random_state::get_fixed_seeds::{{closure}}::h6fd8a8af4df30846 (wasm://wasm/035f2c1a:wasm-function[510]:0x2bb7b0)

[whereistejas]

The specific error is:

error: the wasm32-unknown-unknown target is not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support
   --> /Users/abc/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.8/src/lib.rs:263:9
    |
263 | /         compile_error!("the wasm32-unknown-unknown target is not supported by \
264 | |                         default, you may need to enable the \"js\" feature. \
265 | |                         For more information see: \
266 | |                         https://docs.rs/getrandom/#webassembly-support");
    | |________________________________________________________________________^

A temporary fix for this is to add the pin getrandom in Cargo.toml:

[target.'cfg(target_arch = "wasm32")'.dependencies]
getrandom = { version = "0.2.5", features = ["js"] }

[Stranger6667]

@whereistejas Thank you for sharing this!

Indeed, it makes jsonschema compile on wasm32-unknown-unknown. As for the next step, I'd like to support wasm32-wasi too, but it requires re-working our CLI (probably moving it into a separate crate). One of the clap dependencies requires a nightly feature, but I'd like to make it compile on stable.

I am going to push a few changes to this PR, including a CI job that verifies wasm32-unknown-unknown compilation. Running tests on that platform will require some more changes as it heavily relies on resolving

[SamWilsn]

Now if a non-browser WASM crate includes jsonschema, it won't run because of the js feature, right? The advantage of my initial approach was that the downstream crate can choose which implementation to use, by directly depending on ahash and its features.

[Stranger6667]

What target do you mean by non-browser WASM? From getrandom docs:

This feature has no effect on targets other than wasm32-unknown-unknown.

But also they have this:

Library crates should generally not enable this feature, leaving such a decision to users of their library

Hm, wasm32-wasi would require some extra work because of clap anyway, and I didn't try wasm32-unknown-emscripten, so not sure if the original approach would work there. I might be completely wrong and would be happy to investigate deeper during the weekend.

What was your original compilation target where you got that error?

[SamWilsn]

What target do you mean by non-browser WASM?

Something like wasmer or wasmi that might not have access to the JavaScript runtime. Could also apply if you want to run an identical binary in a browser and under wasi.

What was your original compilation target where you got that error?

wasm32-unknown-unknown, specifically under nodejs.