Support for wasm and Cloudflare Workers

[shiguma127]

I would like to use this library with Cloudflare Workers.

when I try to compile the following code

cargo check --target=wasm32-unknown-unknown

then it occurs this error

error: future cannot be sent between threads safely
   --> rspotify-http\src\reqwest.rs:110:38
    |
110 |       ) -> Result<String, Self::Error> {
    |  ______________________________________^
111 | |         self.request(Method::GET, url, headers, |req| req.query(payload))
112 | |             .await
113 | |     }
    | |_____^ future created by async block is not `Send`
    |
    = help: within `impl Future`, the trait `Send` is not implemented for `*mut u8`
note: future is not `Send` as this value is used across an await
   --> rspotify-http\src\reqwest.rs:93:13
    |
89  |         let response = request.send().await?;
    |             -------- has type `Response` which is not `Send`
...
93  |             response.text().await.map_err(Into::into)
    |             ^^^^^^^^^^^^^^^^^^^^^ await occurs here, with `response` maybe used later
...
97  |     }
    |     - `response` is later dropped here
    = note: required for the cast to the object type `dyn Future<Output = Result<std::string::String, ReqwestError>> + Send`

and when i use rspotify whth features=["client-ureq","ureq-rustls-tls"] on wrangler　project I can compile, but I can't deploy with this error.

Error: Something went wrong with the request to Cloudflare...
Uncaught Error: LinkError: WebAssembly.Instance(): Import #21 module="env" function="GFp_ChaCha20_ctr32" error: function import requires a callable
  at line 0
 [API code: 10021]

I don't think there is anything that can't be done since reqwest also supports wasm.

How do you think? Thaks.

[marioortizmanero]

Thanks for the report. We had never tried to compile to wasm so anything could happen. It would be great to make it possible as long as it's not super complicated. I have no idea how Cloudflare Workers work, though.

It seems like reqwest::Response is not Send on wasm, but it is for other targets. I think this makes sense because there is no support for multi-threading in wasm yet, so Send is not necessary. In reqwest, Response has a completely different implementation for wasm. I ran cargo doc --target=wasm32-unknown-unknown in order to see the source of the issue, and it's true:

Response is made up of a Box<Url>, which is Send, and a http::Response<web_sys::Response>. http::Response is Send, and finally, web_sys::Response is not Send. In fact, almost nothing in web_sys is Send, so it's not like we could just make a PR in order to make it Send upstream. And as I said, it wouldn't make sense to make it Send anyway because it's all single-threaded.

The solution to this would be to just not enforce Send or Sync in our traits, such as BaseHttpClient or BaseClient, when compiling to wasm. This could be done via feature flags, and I don't think it would be too complicated. We would also have to add wasm to our CI to make sure it's always working.

Unfortunately, I don't have enough free time right now for this myself right now. I'm on finals and working on my final year project. Maybe after New Years or so, I don't know. Supporting wasm seems viable, but it's definitely quite a bit of work. If you're interested in doing it yourself I could definitely review your changes from time to time or give you advice. You more or less know where to start now. Or maybe @ramsayleung wants to help out.

[ramsayleung]

It would be great to make this crate to compile to wasm, even though I had never tried to compile this project to wasm before either, and I also have no idea about how Cloudflare Workers work.

Is there a way to create an environment to reproduce this issue?

As Mario illustrating clearly, there are two ways to compile reqwest to wasm:

1. we could create a feature-request issue to the upstream, to make it Send
2. we control Send and Sync with feature flags, and omitting Send and Sync trait when compiling to wasm

Both solutions take time to work. If you can't wait to develop your project and make rspotify compile to wasm, I think the quick-fix solution:

when i use rspotify whth features=["client-ureq","ureq-rustls-tls"] on wrangler project I can compile, but I can't deploy with this error.

It seems the client-ureq can compile to wasm, but fails for some unknown reasons. Perhaps there is still a way to compile rspotify to Cloudflare Workers when we figure out the reason why it fails

[tomocrafter]

I've managed to compile it with removing all of Send markers from signatures and maybe_async on following commit and it works on Cloudflare Wokers as expected. https://github.com/shiguma127/rspotify/commit/33139259110fef181952739375b1001614faead9

also tried to make it switchable with feature but seems like cargo doesn't allow us to flag features depends on arch yet so I couldn't.

If you have any workaround or idea, please let me know!

[shiguma127]

After looking at the commit above, I figured I didn't need to delete all sends, so I created the following commit. https://github.com/shiguma127/rspotify/commit/2d5d5cabee41f13d227bb3f15251bb6561bdf005

However, there is no need to remove the Send boundary of the structure, and replacing

#[maybe_async]

with

#[maybe_async(?Send)]

as in the commit below compiles and seems to work with cloudflere-workers. https://github.com/shiguma127/rspotify/commit/2d5d5cabee41f13d227bb3f15251bb6561bdf005

I am not sure if this is the best solution. Thanks for your help.

[marioortizmanero]

Great! No need to add a feature, we can just use the target itself to configure whether it's send or not (e. g. #[cfg(target = ...)] or #[cfg_attr(target = ...)] ).

What could be a problem is trying to make that switch with all the impl IntoIterator, because there are no trait alias yet on rust. There is a known workaround for that, so it's not a problem, really: https://stackoverflow.com/a/26071172.

Could you perhaps make a PR with those changes?

[tomocrafter]

@marioortizmanero Thank you for your support! I created the PR that switches between maybe_async(?Send) and maybe_async depends on target_arch. but I'm not sure why some of it doesn't need to be maybe_async(?Send) because I'm not used to Rust so much. If you have time, Can you explain why it doesn't fail on compile without changing all of it and removing all of Send makers?

[marioortizmanero]

Notice that some of these Send errors may only appear when you use the data structure. So if you don't use e.g. the code auth client, you won't need maybe_async on that because it's not being used anyway. Try not adding these "unnecessary" switches and then running all the tests and examples, you might see how they don't compile after that.

Not 100% sure anyway, I'll have to try it myself when I have time.

[djedlajn]

Can we have #293 reviewed and merged to enable compiling to wasm and deploying to Cloudflare?

[djedlajn]

I have managed to make it work as per #293 will raise PR at some point this week I just need to address prior concerns related to CI and to make sure that all tests pass.

[marioortizmanero]

Hi @djedlajn, did you end up managing to fix #293?

[github-actions[bot]]

Message to comment on stale issues. If none provided, will not mark issues stale

[gelendir]

Hey, wanted to let you know that I tried finishing what @shiguma127 started, PR here: https://github.com/ramsayleung/rspotify/pull/458

[ramsayleung]

Closing this issue since the support for wasm and Cloudflare has been merged.