gloo icon indicating copy to clipboard operation
gloo copied to clipboard

gloo-net: incorrect access of global when used in a worker

Open cdata opened this issue 2 years ago • 8 comments

Describe the Bug

gloo-net HTTP features cannot be used in a worker because the library always tries to access the Fetch API from Window, which will fail when running in a worker: https://github.com/rustwasm/gloo/blob/5fc0a8ffbed68c9a5ce76e9a90bafa204720717a/crates/net/src/http/mod.rs#L189

Steps to Reproduce

  1. Design a WASM-compatible program that uses gloo-net to make HTTP requests
  2. Load and run it in a Web Worker

Expected Behavior

Expected HTTP requests are made.

Actual Behavior

An error, as the library attempts to access the wrong kind of global.

Additional Context

In digging around, I noticed there is some relevant history from gloo contributors: https://github.com/rustwasm/wasm-bindgen/issues/2148#issuecomment-638606446.

Following that comment thread, I discovered that gloo previously incorporated a limited workaround for accessing global properties in a scope-agnostic way: https://github.com/rustwasm/gloo/blob/2c9e776701ecb90c53e62dec1abd19c2b70e47c7/crates/timers/src/callback.rs#L8-L40

In some cases, it may feasible to take a simpler approach, such as this patch I applied to another crate that has this same issue: https://github.com/devashishdxt/rexie/commit/8637e4ebc2d2dfc6b33cd60dc85b99e46d6afa96

cdata avatar Mar 24 '22 22:03 cdata

I have created a fairly ugly patch to make gloo-net usable in DedicatedWorkerGlobalScope contexts: https://github.com/rustwasm/gloo/compare/master...cdata:master

cdata avatar Mar 24 '22 22:03 cdata

I have created a fairly ugly patch to make gloo-net usable in DedicatedWorkerGlobalScope contexts: master...cdata:master

I wonder if a better solution would be to solve this at the wasm-bindgen level with a Global trait that is returned by js_sys::global and that covers all of the common APIs between Window and DedicatedWorkerGlobalScope. Note that there is also ServiceWorkerGlobalScope that should be considered in the design.

Alternatively we could have an enum returned from js_sys::global such as:

enum Global {
   Window(Window),
   DedicatedWorkerGlobalScope(DedicatedWorkerGlobalScope),
   ServiceWorkerGlobalScope(ServiceWorkerGlobalScope),
}

which could again cover all the common APIs between these types. Perhaps @alexcrichton has some thoughts on this?

allsey87 avatar Jun 08 '22 09:06 allsey87

Given how many contexts JS can run in I don't think that's a great idea for wasm-bindgen itself, but as a helper crate somewhere I think it definitely makes sense.

alexcrichton avatar Jun 08 '22 14:06 alexcrichton

Thanks for the attention to this issue. Perhaps a gloo-global helper crate is in order?

cdata avatar Jun 08 '22 15:06 cdata

My intuition would be to say that it belongs to gloo-util. @alexcrichton when you say that it's not a great idea for wasm-bindgen itself, are you also mean that it is not a good fit for js_sys?

allsey87 avatar Jun 08 '22 15:06 allsey87

@cdata Thank you very much for your fork. I assume it's quite out of date now, but it still managed to resolve my issue.

Zageron avatar Aug 05 '22 03:08 Zageron

@Zageron I don't have time to look at it right now, but if you happen to rebase the change I will happily accept a PR from you :beers: cheers!

cdata avatar Aug 05 '22 03:08 cdata

@cdata I rebased your commit and created a PR to hopefully have an upstream solution soon.

flosse avatar Sep 07 '22 14:09 flosse