search

http-rs / surf

Fast and friendly HTTP client framework for async Rust
https://docs.rs/surf
Apache License 2.0
1.45k stars 119 forks source link

wasm feature flags & getrandom dependency #315

Closed ufoscout closed 2 years ago

ufoscout commented 2 years ago

I created a new empty cargo project with this Cargo.toml file:

[package]
name = "test_surf_wasm"
version = "0.1.0"
edition = "2018"

[dependencies]
surf = { version = "2.2", default-features = false, features = ["wasm-client"] }

Trying to compile it with target wasm32-unknown-unknown fails with:

$ cargo build --target wasm32-unknown-unknown
    Updating crates.io index
   Compiling version_check v0.9.3
   Compiling proc-macro2 v1.0.27
   Compiling unicode-xid v0.2.2
   Compiling syn v1.0.73
   Compiling cfg-if v1.0.0
   Compiling typenum v1.13.0
   Compiling log v0.4.14
   Compiling serde v1.0.126
   Compiling wasm-bindgen-shared v0.2.74
   Compiling serde_derive v1.0.126
   Compiling lazy_static v1.4.0
   Compiling bumpalo v3.7.0
   Compiling ryu v1.0.5
   Compiling wasm-bindgen v0.2.74
   Compiling proc-macro-hack v0.5.19
   Compiling futures-core v0.3.15
   Compiling subtle v2.4.1
   Compiling serde_json v1.0.64
   Compiling opaque-debug v0.3.0
   Compiling futures-channel v0.3.15
   Compiling autocfg v1.0.1
   Compiling memchr v2.4.0
   Compiling semver-parser v0.7.0
   Compiling futures-sink v0.3.15
   Compiling matches v0.1.8
   Compiling libc v0.2.98
   Compiling futures-io v0.3.15
   Compiling stdweb-internal-runtime v0.1.5
   Compiling itoa v0.4.7
   Compiling percent-encoding v2.1.0
   Compiling ppv-lite86 v0.2.10
   Compiling getrandom v0.1.16
   Compiling pin-project-lite v0.2.7
   Compiling sha1 v0.6.0
   Compiling tinyvec_macros v0.1.0
   Compiling proc-macro-nested v0.1.7
   Compiling slab v0.4.3
   Compiling futures-task v0.3.15
   Compiling pin-utils v0.1.0
   Compiling base-x v0.2.8
   Compiling const_fn v0.4.8
   Compiling cache-padded v1.1.1
   Compiling discard v1.0.4
   Compiling crossbeam-utils v0.8.5
   Compiling event-listener v2.5.1
   Compiling anyhow v1.0.42
   Compiling async-trait v0.1.50
   Compiling http-types v2.11.1
   Compiling parking v2.0.0
   Compiling waker-fn v1.1.0
   Compiling base64 v0.13.0
   Compiling data-encoding v2.3.2
   Compiling once_cell v1.8.0
   Compiling infer v0.2.3
   Compiling mime v0.3.16
   Compiling getrandom v0.2.3
   Compiling instant v0.1.10
error: the wasm32-unknown-unknown target is not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support
   --> /home/ufo/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.3/src/lib.rs:219:9
    |
219 | /         compile_error!("the wasm32-unknown-unknown target is not supported by \
220 | |                         default, you may need to enable the \"js\" feature. \
221 | |                         For more information see: \
222 | |                         https://docs.rs/getrandom/#webassembly-support");
    | |_________________________________________________________________________^

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
   --> /home/ufo/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.3/src/lib.rs:246:5
    |
246 |     imp::getrandom_inner(dest)
    |     ^^^ use of undeclared crate or module `imp`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0433`.
error: could not compile `getrandom`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

The problem is fixed by adding this to the Cargo.toml file:

getrandom = { version = "0.2", default-features = false, features = ["js"] }

I suppose that the js feature of getrandom should be enabled by default if the wasm-client is used

Fishrock123 commented 2 years ago

You have already found the correct answer. We can't really know if you want to build for JS wasm or not the way feature flags are currently set up.

The correct(?) future answer for this would be a proper wasm32-emscripten-unknown target, I believe.

ufoscout commented 2 years ago

Hi @Fishrock123 I don't know exactly what you mean by "JS wasm", I simply tried to build to a cargo wasm target with the surf wasm-client feature enabled. Is there another way to build to wasm that I am missing?

Fishrock123 commented 2 years ago

Please refer to the docs sections from getrandom which is linked in the error message, since it explains this: https://docs.rs/getrandom/0.2.3/getrandom/#webassembly-support

tl;dr: Yes there are multiple possible wasm targets, and wasm32-unknown-unknown is (as per it's name) ambiguous.

ngryman commented 2 years ago

Hi,

I'm facing the same issue as @ufoscout.

When I explicitly add the js feature for getrandom I'm getting the following error:

error: cyclic package dependency: package `ahash v0.7.4` depends on itself. Cycle:
package `ahash v0.7.4`
    ... which is depended on by `hashbrown v0.11.2`
    ... which is depended on by `indexmap v1.7.0`
    ... which is depended on by `serde_json v1.0.64`
    ... which is depended on by `wasm-bindgen v0.2.74`
    ... which is depended on by `js-sys v0.3.51`
    ... which is depended on by `getrandom v0.2.3`
    ... which is depended on by `ahash v0.7.4`

I'm pretty new to Rust. This might seem obvious but do you know how to troubleshoot and resolve this?

The documentation mentions this:

wasm-client: use window.fetch as the HTTP backend.

When I read this, I'm assuming that surf supports wasm32-unknown-unknown out-of-the-box as it seems to be a pretty popular target in other libraries that advertise WASM support in JavaScript runtimes. I didn't expect having to dive into the documentation of a transitive dependency to make it work.

One can expect an increase of isomorphic Rust usages in the future. Being able to easily use surf both server-side and client-side would be greatly beneficial for the library. I believe having a clear, frictionless, path to use the wasm-client with wasm32-unknown-unknown would definitely help with that.

I suppose that the js feature of getrandom should be enabled by default if the wasm-client is used

@Fishrock123 I agree with this proposition. My questions are:

  1. Is there something preventing surf to do this by default?
  2. Otherwise, do you know of alternatives that would allow supporting wasm32-unknown-unknown out-of-the-box?

Thank you!

Fishrock123 commented 2 years ago

If you care about this please tell your wasm packing tool to switch to wasm32-unknown-emscripten.

Surf could introduce a wasm-web-client feature or something to cover this up but it really is not this library’s problem, IMO.

Fishrock123 commented 2 years ago

(By “could” I mean I’ll accept a PR for something like that if someone makes one.)

jkelleyrtp commented 2 years ago

If you care about this please tell your wasm packing tool to switch to wasm32-unknown-emscripten.

Surf could introduce a wasm-web-client feature or something to cover this up but it really is not this library’s problem, IMO.

Sorry - but isn't that what the feature "wasm-client" should mean? Shouldn't the proper form of get-random be selected if compiling for WASM? It's super confusing to have a "wasm-client" feature that doesn't compile on wasm... I think this feature is broken.

Edit, yeah, it should work, there's a crate called "wasm-test" in the workspace.

@ufoscout

the secret to getting it to work is to pin getrandom to the "js" feature

[dependencies.getrandom]
version = "0.2"
features = ["js"]

This is a thing that can be done within surf, so I'll make a pr to fix this.

Fishrock123 commented 2 years ago

I was not involved with the development of the WASM feature within Surf and I don't know the full original intentions.

Non-web wasm runtimes do exist, particularly ones based on WASI, but there could potentially be others as well.

Fishrock123 commented 2 years ago

(Perhaps someone could confirm if it it is even possible to compile Surf to WASI at all. If not, then we might as well just support web-only WASM for now.)

jkelleyrtp commented 2 years ago

(Perhaps someone could confirm if it it is even possible to compile Surf to WASI at all. If not, then we might as well just support web-only WASM for now.)

I believe when compiling for WASI you target wasm32-wasi. We could either target-select on wasi, or enable "wasi" as a feature in surf. WASI essentially uses the same sys-calls as regular code, so I think it might work fine without selecting the wasm-client.

I'm not sure what WASI people use for making web requests today.

I just tried compiling surf for wasi and the socket2 dependency fails because it doesn't support wasi as a target. Today at least, surf is not supported on WASI.

Fishrock123 commented 2 years ago

I believe this was fixed in #316 and should be released in a 2.3.0 soonish