unhandled panics are swallowed, crashes wasm "runtime"

[jzaefferer]

Running random bool panicks and only dumps that error with console.error

panicked at 'could not initialize thread_rng: getrandom: this target is not supported', /Users/jzaefferer/.cargo/registry/src/github.com-1ecc6299db9ec823/rand-0.7.3/src/rngs/thread.rs:65:17

I guess the console.error comes from the console_error_panic_hook crate

Is there some way to have the wasm.run_nu Promise rejected instead, to handle the error in our "application" code?

If not, can we deal with panics on the Rust side and forward them through the existing serialized error channel?

It also looks like something "exits" once this type of error occurs, and running other commands isn't possible anymore (a page reload helps).

[jzaefferer]

@jonathandturner could we pair on this? I think together we have a much bigger chance of solving this. I'm on GMT+2 and could generally make time on weekdays after 8pm.

[jzaefferer]

Copied from Discord:

@jonathandturner I asked around a bit about that a few weeks ago when I noticed that calling into standard library calls in Rust that aren't supported in wasm just panic. Looks like there isn't a great fix for it. Some people do some pretty aggressive hacks to avoid it
I think for us it'd probably be easier to make a list of Nu commands that panic, and reimplement them or add in a dummy command that just says "this command is not yet supported"

@jzaefferer Hm, alright. That is generally the right approach, but it'll still crash for anything new added to nu, which is especially likely for people wanting to try it out when a new release comes out. So a generic error handler that avoids the whole thing dying would be good...

If there's little to nothing we can do on the Rust side, I'll take another stab if there's something we can do on the JS side

Maybe the await wasm.run_nu(input); actually throws a promise rejection that needs to be caught?

[sophiajt]

Ah, that could be. Adding a simple fix to see if that's what it is.

[jzaefferer]

Doesn't seem to help. Maybe we can replace console_error_panic_hook with something that exposes the error to the JS side?

[jzaefferer]

@jonathandturner maybe we can clone https://github.com/rustwasm/console_error_panic_hook/blob/master/src/lib.rs and change it to call a JS function that can do more than just console.error? Display the error on the page, along with a hint to reload if everything stops working.

Otherwise we could try to intercept console.error calls, but that's generally very fragile.

[jzaefferer]

ping @jonathandturner - what do you think about my suggestion (previous comment)?

[jzaefferer]

It might be worth trying to turn the async run_nu fn into a sync one, using something like https://docs.rs/async_executors/0.3.0/async_executors/

Generally wasm-bindgen seems to handle sync functions better than async ones. Though it could also be worth investigating why the try/catch can't capture the rejected promise - that seems like a bug in wasm-bindgen.

[jzaefferer]

#63 hugely improve this. The runtime(?) still crashes, but at least the panic is now visible and a button helps to reload with a safe command.