provide better panic message for Zoned::now on wasm32-unknown-unknown when js feature is not enabled

[jprochazk]

Hi! First off, thanks for publishing this excellent library.

jiff's current Wasm-in-browser support requires enabling a feature flag, js. This makes a lot of sense! While the intentions are good, the feature flag unfortunately leads to a sub-optimal user experience. It's a silent failure mode, meaning it's not obvious as a new user of this library that it is required to make it work in a browser. In my first attempt to use jiff in a Wasm-in-browser environment, I ran into the following panic:

panicked at std/src/sys/pal/wasm/../unsupported/time.rs:31:9:
time not implemented on this platform

After some digging through docs, I discovered the js feature flag, enabled it, and all was well again. Later on, one of my colleagues ran into the same issue. I have a few more anecdotes like this, but the point is that the feature flag has poor discoverability.

I believe the most common use case for Rust Wasm is running the code in a browser, so my suggestion is to make js a default feature. It would still allow people to disable it, but the "happy path" would be better supported, and jiff would lead more people into the pit of success. The new failure mode would be the generated Wasm binary requiring additional imports that the host can't provide, so it would fail to load in non-browser environments. And of course, people disabling default features when adding new dependencies may still run into the same issue.

The alternatives are to somehow better detect the environment, or improve the error message:

• Unfortunately, there is no way to differentiate between a browser and other Wasm hosts. Wasm is inherently a sandboxed environment. There is a possibility Wasm will get optional imports, which would allow jiff to unconditionally import Date.now and just not use it if it wasn't provided. That feature has not received any attention for quite a while now, though.
• Because the error message comes from a panic inside std, jiff could catch_unwind the panic, inspect the error message for "time not implemented on this platform", modify it to provide additional context, and resume the panic. That seems extremely brittle, because changing the message would break the detection.

Improving documentation and making the js feature more prominent there may help as well, but it's not clear how to get people to read the docs :)

[BurntSushi]

Yeah I do sympathize with this, but I unfortunately think that the "right" solution to this problem is for Rust (upstream) to have a wasm web target. And Jiff's design was loosely inspired by what getrandom does. So I think it's kind of an uphill battle here to convince me that Jiff should do something different than other ecosystem crates. In other words, this feels more like a "Rust ecosystem problem" as opposed to something Jiff should specifically try to improve by making js the default.

One of the primary problems I have with enabling js by default is that it is in general much harder to disable a feature than it is to enable it. That is, if js is enabled by default, then users wanting to use Jiff in contexts without js may have an especially difficult time disabling js if one of their other dependencies also depends on Jiff and doesn't itself explicitly opt out of Jiff's default features. This is a well known problem with feature unification, and there unfortunately isn't a whole lot to be done about it other than being careful about what's enabled by default.

Because the error message comes from a panic inside std, jiff could catch_unwind the panic, inspect the error message for "time not implemented on this platform", modify it to provide additional context, and resume the panic. That seems extremely brittle, because changing the message would break the detection.

I would actually potentially be open to this as long as it only runs on wasm and it... actually works. Will wasm32-none-none even do unwinding? I'm not sure. I agree it's bittle, but it's non-essential and is really just about improving failure modes. So I'm more amenable to this.

[hanna-kruppe]

wasm32v1-none is a pure-no_std target, so it dodges the issue of SystemTime::now() panicking of wasm32-unknown-unknown. Both targets only support panic=abort right now and for the forseeable future. Since the definition of wasm32-unknown-unknown changes as more Webassembly proposals become widely supported by engines, panic=unwind via the exception handling proposal will probably become possible at some point in the future. But I don't expect this to happen Very Soon™ and I don't know if and when it'll actually be adopted as default for the pre-build standard library most people use.

One of the primary problems I have with enabling js by default is that it is in general much harder to disable a feature than it is to enable it. That is, if js is enabled by default, then users wanting to use Jiff in contexts without js may have an especially difficult time disabling js if one of their other dependencies also depends on Jiff and doesn't itself explicitly opt out of Jiff's default features. This is a well known problem with feature unification, and there unfortunately isn't a whole lot to be done about it other than being careful about what's enabled by default.

FWIW, as a user who'd had to engage in some careful dependency gardening to keep wasm-bindgen code out of my non-browser-targeted wasm module, I fully agree with this.

[BurntSushi]

Aye thanks for that added context @hanna-kruppe!

So I think to me, it sounds like the only other lever to pull here is docs. I honestly don't have a good sense of how many people are actually using Jiff in a web browser context, but I've heard of at least a handful. Maybe something that is somewhat more prominent in the README? Or the crate docs?

I thought there were other crates that followed this js feature pattern (although it looks like getrandom has changed things up a bit from its 0.2 release), so I was hoping this would be "common knowledge" among folks using Rust & WASM in a web context. But maybe not.

I also don't understand why getrandom changed things here. Specifically, in 0.2 -> 0.3, they:

• Renamed the feature from js to wasm_js.
• Required a second step to opt into this by setting RUSTFLAGS='--cfg getrandom_backend="wasm_js"' (which can also be set in .cargo/config.toml).

I have guesses about the rename, but I don't understand the getrandom_backend config part. And I'm wondering because I'm curious if Jiff should be doing the same thing.

searches the getrandom issue tracker

Oh! Hi @hanna-kruppe :-) rust-random/getrandom#433 (comment)

It looks like probably not? Since Jiff doesn't need to support anything similar to the "alternative backends" that getrandom has AFAIK.

And also, TIL: rust-lang/cargo#10801

Actually, more like TILA (Today I Learned Again) because of BurntSushi/bstr#130 lol

(Sorry for the slight ramble.)

[jprochazk]

I appreciate the response, I didn't think at all about feature unification and the issues that may cause.

I would actually potentially be open to this as long as it only runs on wasm and it... actually works. Will wasm32-none-none even do unwinding? I'm not sure. I agree it's bittle, but it's non-essential and is really just about improving failure modes. So I'm more amenable to this.

I wonder if unwinding is even needed? std::panic::set_hook works even with panic=abort, and is used in crates like console_error_panic_hook to redirect panics to the browser's console. Maybe it'd be possible to unconditionally set this on wasm targets, and inspect the panic message that way? I'll try to experiment with this.

[BurntSushi]

I would be very skeptical of setting panic hooks in a library like Jiff. That seems like speed-running a way toward pissing a whole bunch of people off. Panic hooks are very specifically a global resource.

I acknowledge that console_error_panic_hook is doing it, but it seems like that's the very point of the crate, and it is very loud and explicit about what it's doing. In contrast, doing something like setting a hook and replacing whatever was there previously in a library like Jiff seems like bad juju. On par with unceremoniously writing to stderr from inside a library (but maybe not as bad as writing to stdout).

And imagine if someone uses Jiff and console_error_panic_hook together. Who wins? Oy.

[BurntSushi]

I want to be clear that I acknowledge the bad failure mode you ran into. And I appreciate you brainstorming ways to fix it. :-)

I really do think that for the time being, the best mitigation here is going to be docs. Which is all sorts of imperfect, especially when there are so many docs. But I don't think we can really do much better here presently without a cure that is worse than the disease.

[hanna-kruppe]

There is one other thing that jiff could try doing: whenever it calls SystemTime::now() on wasm32-unknown-unknown without the js feature, panic with a more informative message instead of letting std panic:

#[cfg(all(target_family = "wasm", ..., not(feature = "js")))]
panic!("getting the current time in wasm32-unknown-unknown is not possible,\
        enable jiff's `js` feature if you are targeting a browser environment");
let now = SystemTime::now();

[BurntSushi]

Oh, well that seems fine then. I guess that makes sense because we know it will presumably always panic even in the future? That would be much better. Great idea.

[hanna-kruppe]

Yeah, I expect that if a “wasm-web” target is added in the future, it’ll at least differ in one of the target_* cfgs that distinguish the existing wasm targets from each other (maybe OS or target_env).