Add node.js support for timers #185

PhilippGackstatter · 2022-01-17T12:07:18Z

Add support for node.js in gloo-timers. The motivation is to enable rust-libp2p's usage in node.js.

This adds an enum variant to the WindowOrWorker struct. An alternative would be to remove that enum entirely and bind to the global setTimeout, setInterval, clearTimeout and clearInterval functions directly. From my understanding, they should be available in window, web workers and node.js (as well as deno). For now, I've stuck to the enum variant. Let me know what you prefer.

The node.js detection logic is akin to how the browser-or-node npm package does it. What might also be possible is to assume node.js if no window or worker is available.

fixes #176

I have tested it locally in a libp2p example project. cargo test still succeeds locally, too. I'm happy to add dedicated node.js tests as well, if deemed useful, but I'm not sure how I would do that.

PhilippGackstatter · 2022-01-19T12:05:10Z

@hamza1311 would you be able to take a look at this and give feedback? Thanks!

ranile · 2022-01-19T12:08:48Z

@PhilippGackstatter can you just pull the functions from global scope? That way we can avoid checking the platform.

Also a more human usable name for imported functions would be nice since (unlike web sys) this isn't autogenerated code

PhilippGackstatter · 2022-01-19T12:28:37Z

@hamza1311 Done. Using global scope for all platforms now, so it should work for browsers (window, web workers) as well as nodejs (and deno if wasm-bindgen has support for it, not sure).

I've previously used the long names so I wouldn't have to rewrite the macro, but that's also fixed now.

PhilippGackstatter · 2022-01-19T12:34:55Z

@hamza1311 Actually, I'm not sure whether the thread_local is still required. Did WindowOrWorker::new previously bind to the global functions from different threads when it was created? I'm not familiar enough with wasm-bindgen internals to judge.

ranile · 2022-01-19T12:41:51Z

I don't think it is required. Test pass which means it works. I'm split between testing for node in CI because not all of our crates work on node. I guess that's for a different PR.

I'll merge this. It'll be part of the next release

teohhanhui · 2022-01-30T10:27:36Z

Unfortunately it seems it's not actually working?

/home/runner/work/callbag-rs/callbag-rs/target/wasm32-unknown-unknown/wbg-tmp/wasm-bindgen-test.js:616
    throw new Error(getStringFromWasm0(arg0, arg1));
          ^

Error: `unwrap_throw` failed
    at module.exports.__wbindgen_throw (/home/runner/work/callbag-rs/callbag-rs/target/wasm32-unknown-unknown/wbg-tmp/wasm-bindgen-test.js:616:11)
    at wasm_bindgen::throw_str::h8e401d0f67754eb8 (wasm://wasm/0137c46a:wasm-function[12623]:0x3233ac)
    at <core::result::Result<T,E> as wasm_bindgen::UnwrapThrowExt<T>>::expect_throw::h8e33ecaea208e906 (wasm://wasm/0137c46a:wasm-function[3586]:0x2342fd)
    at wasm_bindgen::UnwrapThrowExt::unwrap_throw::h9c7c6768824c8a66 (wasm://wasm/0137c46a:wasm-function[4386]:0x25a395)
    at gloo_timers::callback::Timeout::new::h0f7e6d2fd9da0763 (wasm://wasm/0137c46a:wasm-function[1245]:0x187f06)
    at gloo_timers::future::TimeoutFuture::new::hdd49d2f1cc429bd8 (wasm://wasm/0137c46a:wasm-function[1864]:0x1c39f9)
    at futures_timer::wasm::Delay::new::h8dd6e7d30cb8ef68 (wasm://wasm/0137c46a:wasm-function[3227]:0x221349)
    at <async_executors::exec::async_std::AsyncStd as async_executors::iface::timer::Timer>::sleep::h9425297df3aa4d78 (wasm://wasm/0137c46a:wasm-function[4722]:0x268280)
    at <async_nursery::nursery::Nursery<S,Out> as async_executors::iface::timer::Timer>::sleep::h5fe438d1b052e7f9 (wasm://wasm/0137c46a:wasm-function[7875]:0x2c74c1)
    at async_executors::exec::tracing::<impl async_executors::iface::timer::Timer for tracing_futures::Instrumented<T>>::sleep::hcde93cfe7d7972ed (wasm://wasm/0137c46a:wasm-function[3232]:0x2217c7)

https://github.com/teohhanhui/callbag-rs/runs/4996188586?check_suite_focus=true#step:8:124

ranile · 2022-01-30T10:49:06Z

@teohhanhui Can you pin down what might be causing the error? I can't tell what's happening from stack trace.

This might be related to #187

teohhanhui · 2022-01-30T10:55:09Z

I'm not sure how to debug this. Should I open another issue so we can follow up on this? Or join in on #187?

ctm · 2022-02-05T13:41:13Z

@teohhanhui I apologize for not commenting earlier, but I only saw your problem now.

It's not much, but if you use gloo-timers = "=0.2.2" in your Cargo.toml and the problem persists, then you know it's not related to #187. I think that would be useful information for everyone.

teohhanhui · 2022-02-05T14:08:48Z

@ctm:

if you use gloo-timers = "=0.2.2" in your Cargo.toml and the problem persists

I cannot test with 0.2.2 as Node.js support was only added in 0.2.3

ctm · 2022-02-05T14:38:04Z

Aha. Makes sense. I wasn't going to venture a guess when I mistakenly thought you could test 0.2.2, but since you can't, my guess is it's not the same as #187 (but it could be very much related), because your backtrace has the death coming from gloo_timers::callback::Timeout::new, which works fine when using webpack/wasm-pack.

I'm a bit out of my depth here. I know little about web stuff in general and almost nothing about node. However, from my naive perspective, the question I have is whether js_sys::global() is necessary for Node, because it certainly appears to be—at least currently (perhaps due to a bug in wasm-pack)—for webpack.

ctm · 2022-02-05T20:12:09Z

@teohhanhui: I've quickly hacked in a call to js_sys::global() that works for my application. I just tested it with:

gloo-timers = { version = "0.2.3", git = "https://github.com/ctm/gloo", branch = "js_sys-experiment" }

Perhaps it will work for node, but I don't know how to test it. If you give it a try and it works for node, I'll be happy to turn it into a proper pull-request. If it doesn't work for node, at least we have one more data point.

teohhanhui · 2022-05-11T08:31:46Z

@ctm Sorry for getting back so late, but it doesn't seem to work.

Running

wasm-pack test --node -- --features tracing --test interval

with this tree: https://github.com/teohhanhui/callbag-rs/tree/f7f385a389dea5f54a98c85c2490465939120412

     Running tests/interval.rs (target/wasm32-unknown-unknown/debug/deps/interval-3e8c0cd6704950d3.wasm)
Set timeout to 20 seconds...
running 2 tests                                   

test interval::interval_1000_can_be_disposed_before_anything_is_sent ... ok
test interval::interval_50_sends_5_times_then_we_dispose_it ... ok

test result: ok. 2 passed; 0 failed; 0 ignored

/home/teohhanhui/projects/teohhanhui/callbag-rs/target/wasm32-unknown-unknown/wbg-tmp/wasm-bindgen-test.js:616
    throw new Error(getStringFromWasm0(arg0, arg1));
          ^

Error: `unwrap_throw` failed
    at module.exports.__wbindgen_throw (/home/teohhanhui/projects/teohhanhui/callbag-rs/target/wasm32-unknown-unknown/wbg-tmp/wasm-bindgen-test.js:616:11)
    at wasm_bindgen::throw_str::h02441ac3d7470902 (wasm://wasm/0135eb56:wasm-function[12482]:0x31bada)
    at <core::result::Result<T,E> as wasm_bindgen::UnwrapThrowExt<T>>::expect_throw::h75d6d572b277ba5b (wasm://wasm/0135eb56:wasm-function[3625]:0x232976)
    at wasm_bindgen::UnwrapThrowExt::unwrap_throw::h6ac52f369d9fce66 (wasm://wasm/0135eb56:wasm-function[4367]:0x255bd7)
    at gloo_timers::callback::Timeout::new::{{closure}}::hb68f545b5ecb0faf (wasm://wasm/0135eb56:wasm-function[5077]:0x271ee1)
    at std::thread::local::LocalKey<T>::try_with::hd40a3591f23848b3 (wasm://wasm/0135eb56:wasm-function[2802]:0x20412a)
    at std::thread::local::LocalKey<T>::with::haf5f172e5d586f9e (wasm://wasm/0135eb56:wasm-function[5443]:0x27ebfe)
    at gloo_timers::callback::Timeout::new::h03e92484601dd06b (wasm://wasm/0135eb56:wasm-function[1328]:0x18d892)
    at gloo_timers::future::TimeoutFuture::new::h3064a83cb7c758ea (wasm://wasm/0135eb56:wasm-function[1880]:0x1c02dc)
    at futures_timer::wasm::Delay::new::he7968cdb32ff7472 (wasm://wasm/0135eb56:wasm-function[3247]:0x21e973)
error: test failed, to rerun pass '--test interval'
Error: Running Wasm tests with wasm-bindgen-test failed
Caused by: failed to execute `cargo test`: exited with exit status: 1
  full command: "cargo" "test" "--target" "wasm32-unknown-unknown" "--features" "tracing" "--test" "interval"

I've verified that this is in my local Cargo.lock:

[[package]]
name = "gloo-timers"
version = "0.2.4"
source = "git+https://github.com/ctm/gloo?branch=js_sys-experiment#b47469f731d2c9c141ade70e7dbfe65c3634a4d2"
dependencies = [
 "futures-channel",
 "futures-core",
 "js-sys",
 "wasm-bindgen",
]

and I'm using the latest wasm-pack (re-run the latest installer) and wasm-bindgen-test etc. (deleted Cargo.lock to start fresh).

Is there any way I could possibly help to debug this?

ctm · 2022-05-11T11:30:13Z

Thanks, @teohhanhui. Now that I have a failing test I can take a look and see if there's anything I can do. I don't want to get your hopes up though, this is not an area I am particularly knowledgable about. Additionally, today (and the next couple of days) are pretty busy for me.

I am, however, fairly persistent and do want a solution that works for everyone, so with both of us pulling in the same direction, we'll probably get one eventually.

Add support different WASM environments (such as workers, NodeJS) that don't have a `window` global by binding to the global `setInterval` and `clearInterval` functions directly. This uses the same approach as used by gloo-timers implemented in [rustwasm/gloo#185](rustwasm/gloo#185) and [rustwasm/gloo#283](rustwasm/gloo#283) given the `web-sys` lack of interes to support this (see discussion in [this issue](rustwasm/wasm-bindgen#1046)). Co-authored-by: sisou <[email protected]>

Add support different WASM environments (such as workers, NodeJS) that don't have a `window` global. This is done by binding to the global `setInterval` and `clearInterval` functions directly. This uses the same approach used by gloo-timers implemented in [rustwasm/gloo#185](rustwasm/gloo#185) and [rustwasm/gloo#283](rustwasm/gloo#283) given the `web-sys` lack of interes to support this (see discussion in [this issue](rustwasm/wasm-bindgen#1046)). Co-authored-by: sisou <[email protected]>

Add node.js support for timers

c798e33

Bind to global scope for all platforms

7c2fdb8

ranile merged commit f5761bf into rustwasm:master Jan 19, 2022

PhilippGackstatter deleted the feat/nodejs-timers branch January 19, 2022 12:43

PhilippGackstatter mentioned this pull request Jan 19, 2022

Remove web-sys dependency in gloo-timers #186

Merged

PhilippGackstatter mentioned this pull request Feb 9, 2022

protocols/gossipsub: Custom interval grows and is not performant libp2p/rust-libp2p#2497

Closed

PhilippGackstatter mentioned this pull request Mar 4, 2022

gloo-timers 0.2.3 can panic #187

Closed

flisky mentioned this pull request Jan 10, 2023

fix: clearTimeout illegal invocation with bundler (#187) #283

Merged

sisou mentioned this pull request Nov 18, 2023

feat(websocket-websys): add support for different WASM environments libp2p/rust-libp2p#4889

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add node.js support for timers #185

Add node.js support for timers #185

Uh oh!

PhilippGackstatter commented Jan 17, 2022

Uh oh!

PhilippGackstatter commented Jan 19, 2022

Uh oh!

ranile commented Jan 19, 2022

Uh oh!

PhilippGackstatter commented Jan 19, 2022

Uh oh!

PhilippGackstatter commented Jan 19, 2022

Uh oh!

ranile commented Jan 19, 2022

Uh oh!

teohhanhui commented Jan 30, 2022

Uh oh!

ranile commented Jan 30, 2022

Uh oh!

teohhanhui commented Jan 30, 2022 •

edited

Loading

Uh oh!

ctm commented Feb 5, 2022

Uh oh!

teohhanhui commented Feb 5, 2022

Uh oh!

ctm commented Feb 5, 2022

Uh oh!

ctm commented Feb 5, 2022

Uh oh!

teohhanhui commented May 11, 2022 •

edited

Loading

Uh oh!

ctm commented May 11, 2022

Uh oh!

Uh oh!

Add node.js support for timers #185

Add node.js support for timers #185

Uh oh!

Conversation

PhilippGackstatter commented Jan 17, 2022

Uh oh!

PhilippGackstatter commented Jan 19, 2022

Uh oh!

ranile commented Jan 19, 2022

Uh oh!

PhilippGackstatter commented Jan 19, 2022

Uh oh!

PhilippGackstatter commented Jan 19, 2022

Uh oh!

ranile commented Jan 19, 2022

Uh oh!

teohhanhui commented Jan 30, 2022

Uh oh!

ranile commented Jan 30, 2022

Uh oh!

teohhanhui commented Jan 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctm commented Feb 5, 2022

Uh oh!

teohhanhui commented Feb 5, 2022

Uh oh!

ctm commented Feb 5, 2022

Uh oh!

ctm commented Feb 5, 2022

Uh oh!

teohhanhui commented May 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctm commented May 11, 2022

Uh oh!

Uh oh!

teohhanhui commented Jan 30, 2022 •

edited

Loading

teohhanhui commented May 11, 2022 •

edited

Loading