'not in a browser' panic when encrypting with wasm under node.js

[richvdh]

when calling OlmMachine::encryptRoomEvent from a jest test environment:

  console.error
    panicked at 'not in a browser', /home/rav/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/wasm-timer-0.2.5/src/wasm.rs:59:14
    
    Stack:
    
    Error: 
        at /home/rav/work/matrix-rust-sdk/bindings/matrix-sdk-crypto-js/pkg/matrix_sdk_crypto_js.js:6313:17
        at apply (/home/rav/work/matrix-rust-sdk/bindings/matrix-sdk-crypto-js/pkg/matrix_sdk_crypto_js.js:238:18)
        at logError (/home/rav/work/matrix-rust-sdk/bindings/matrix-sdk-crypto-js/pkg/matrix_sdk_crypto_js.js:6312:65)
        at console_error_panic_hook::hook::hd75b82779b76d148 (wasm://wasm/017ee94e:1:2325260)
        at core::ops::function::Fn::call::h4cac9659a5558564 (wasm://wasm/017ee94e:1:3719207)
        at std::panicking::rust_panic_with_hook::h74caca580525a466 (wasm://wasm/017ee94e:1:2590223)
        at std::panicking::begin_panic_handler::{{closure}}::h80a55f911a785530 (wasm://wasm/017ee94e:1:2819541)
        at std::sys_common::backtrace::__rust_end_short_backtrace::h1bd9f2a83d49f95a (wasm://wasm/017ee94e:1:3546133)
        at rust_begin_unwind (wasm://wasm/017ee94e:1:3259323)
        at core::panicking::panic_fmt::h4c9b8223c2dfa034 (wasm://wasm/017ee94e:1:3448564)
        at core::panicking::panic_display::h34f73254ef3bad25 (wasm://wasm/017ee94e:1:3297762)
        at core::panicking::panic_str::ha5eb71c9a86565c1 (wasm://wasm/017ee94e:1:3579407)
        at core::option::expect_failed::ha80939628256a21e (wasm://wasm/017ee94e:1:3696930)
        at wasm_timer::wasm::Instant::now::h643a6967f0f940c6 (wasm://wasm/017ee94e:1:2939869)
        at wasm_timer::timer::ext::TryFutureExt::timeout::h9f8683d1a0ef6be0 (wasm://wasm/017ee94e:1:3126214)
        at matrix_sdk_crypto::identities::manager::KeysQueryListener::wait_if_user_pending::{{closure}}::h9bdd193e72ba766b (wasm://wasm/017ee94e:1:1243765)
        at matrix_sdk_crypto_js::future::future_to_promise::{{closure}}::{{closure}}::hb944fc57494748d3 (wasm://wasm/017ee94e:1:473917)
        at wasm_bindgen_futures::task::singlethread::Task::run::hfcb83b359a2a7000 (wasm://wasm/017ee94e:1:2845964)
        at wasm_bindgen_futures::queue::Queue::new::{{closure}}::h463f5a5dff63ed3c (wasm://wasm/017ee94e:1:2596602)
        at <dyn core::ops::function::FnMut<(A,)>+Output = R as wasm_bindgen::closure::WasmClosure>::describe::invoke::h81c39ae715f1b29c (wasm://wasm/017ee94e:1:3627751)
        at _dyn_core__ops__function__FnMut__A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h81c39ae715f1b29c (/home/rav/work/matrix-rust-sdk/bindings/matrix-sdk-crypto-js/pkg/matrix_sdk_crypto_js.js:303:10)
        at f (/home/rav/work/matrix-rust-sdk/bindings/matrix-sdk-crypto-js/pkg/matrix_sdk_crypto_js.js:221:20)
        at runNextTicks (node:internal/process/task_queues:60:5)
        at processImmediate (node:internal/timers:442:9)

It looks like we're calling wasm_timer::TryFutureExt::timeout, which seems to be problematic when used outside the browser due to the lack of a window object (and, for that matter, when used in web workers). See also tomaka/wasm-timer#21 (comment) which suggests that wasm_timer needs replacing anyway.

Possibly also related: #896.

[poljar]

I think we can remove the logic, it's not used anywhere outside of a unit test in EA. We might want to solve the unit test problem with some other way.

[richvdh]

I think we can remove the logic

which logic? The timeout?

[poljar]

Yep the timeout, specifically this part of the backtrace is the source of the panic:

at matrix_sdk_crypto::identities::manager::KeysQueryListener::wait_if_user_pending::{{closure}}::h9bdd193e72ba766b (wasm://wasm/017ee94e:1:1243765)

So I propose we remove the whole KeysQueryListener.

[Hywan]

I'm fine removing it if nobody else is using it.

[richvdh]

I discussed this a bit online with @poljar. My feeling was that the KeysQueryListener is providing a useful service in ensuring we do a /keys/query before trying to send messages to a given user, and that it would be annoying to pull this up to the application.

Accordingly: my preference is to fix the timeout rather than throw away the whole of KeysQueryListener.

[poljar]

The timeout we're talking about can be found here:

matrix-rust-sdk/crates/matrix-sdk-common/src/timeout.rs

Lines 33 to 37 in 6a5fa81

	#[cfg(target_arch = "wasm32")]
	{
	let try_future = async { Ok::<T, std::io::Error>(future.await) };
	try_future.timeout(duration).await.map_err(|_| ElapsedError(()))
	}

It uses the wasm-timer crate using the TruFutureExt trait.

[Hywan]

It's raised from https://github.com/tomaka/wasm-timer/blob/64c812a69981cbf0bc428d290a7887818f451e2d/src/wasm.rs#L59.

Node.js has no global window object, and no Instant. It's gonna be hard to update wasm-timer to support that.
However, Node.js has setTimeout and co. (see timers' documentation). So maybe we can write our own timeout logic that can work on both Node.js and JS?

[richvdh]

However, Node.js has setTimeout and co. (see timers' documentation). So maybe we can write our own timeout logic that can work on both Node.js and JS?

Yes. Possibly we could use https://crates.io/crates/gloo-timers which AFAICT just calls the standard global setTimeout which is available in both Node.js and in browsers?