Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client feature flag to support wasm32 targets. #693

Closed
wants to merge 1 commit into from

Conversation

boxdot
Copy link

@boxdot boxdot commented Jun 26, 2021

When protobuf is generated without the transport feature, the generated client can be compiled to wasm32 targets and used in browser environment.

To be able to run the client in browser, a h2 version including the commit 5c72713 has to be used.

Motivation

This enables tonic clients to compile to wasm32-unknown-unknown target and run in a Browser environment. For more information, please see #491.

Solution

For this to work, we

  1. introduced a new feature flag, which only enables system independent parts of tonic transport,
  2. added spawn function which either uses tokio::spawn or wasm_bindgen_futures::spawn_local dependening on the compilation target,
  3. set http2_max_concurrent_reset_streams to 0 for wasm, to disable the Instant::now usage in h2 which is not available in Browser.

When protobuf is generated without the transport feature, the
generated client can be compiled to wasm32 targets and used in
browser environment.

To be able to run the client in browser, a `h2` version including
the commit 5c72713 has to be used.
Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for opening this, I am not 100% sure if this is the right approach but I need to spend sometime digging into the wasm stuff a bit. So I am not going to include this in 0.5 but will try to get it in for 0.6. Thanks for all the hard work on this!

@@ -39,6 +41,13 @@ tls-roots-common = ["tls"]
tls-roots = ["tls-roots-common", "rustls-native-certs"]
tls-webpki-roots = ["tls-roots-common", "webpki-roots"]
prost = ["prost1", "prost-derive"]
client = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this feature be called wasm?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, this is not about wasm but about having a client without relying on any OS features e.g. like sockets. But naming is hard and I agree client is somehow misleading.

@@ -241,6 +241,7 @@ impl Endpoint {
}

/// Create a channel from this config.
#[cfg(feature = "transport")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm this feels a bit funky to me

@vdods
Copy link

vdods commented Dec 5, 2021

I'm curious if there's been any progress on this front. I'm also quite in favor of tonic clients working in wasm.

@bobbbay
Copy link

bobbbay commented Dec 9, 2021

Bump. I'd like to use a client in WASM, and this seems like exactly what I need. Can we get this merged?

@boxdot
Copy link
Author

boxdot commented Dec 9, 2021

It got stale. I will rebase this PR on branch 0.6.

@vdods
Copy link

vdods commented Dec 10, 2021

I had done this merge locally and mostly got it working for my own purposes, but it seems like there might be a missing #[cfg in the generated Client code for XyzClient::connect, which prevents the same generated code from being used for both wasm and non-wasm builds. For reference, I haven't been using the tonic::include_proto macro but instead generating code into my crate which I check in, partially so it's easier to see what code is actually produced.

@vdods
Copy link

vdods commented Dec 20, 2021

Further notes:

  1. Originally I had all my .proto and tonic-build stuff in a single crate, but I found that it was impossible to (a) invoke tonic-build with and without the "transport" feature for different targets, and (b) get target-specific dependencies working for tonic. The solution for (b) was to use features (sort of ugly, since it really is target-specific, and therefore the features solution uses mutually-exclusive features), and the solution for (a) was to separate out the tonic-build for client and server code into separate client and server crates.
  2. I was having a panic crash in the wasm client that wasn't happening in the tonic-ws-transport wasm-client example, but after a bunch of frustrating debugging, I realized that I had neglected to ensure that the commit 5c72713 from h2 was included in the version of h2 that the boxdot/tonic branch depended upon. Commit 5c72713 is part of h2 v0.3.4, so bumping the h2 dependency in tonic to "0.3.4" fixed that. I'm guessing the original boxdot/tonic branch happened before h2 v0.3.4 was released, which is why it didn't simply just depend on 0.3.4.

@rmccrystal
Copy link

Bump. I would like to use tonic over WebSockets in webasm for my project

@aknowel
Copy link

aknowel commented Sep 15, 2022

Hi, gRPC tunneled over WebSocket seems perfect solution for my project.
I have spent many hours looking for the solution, I have tried capn proto, tarpc, but it seems no of them are as advanced and as well documented as gRPC.

Couldn't we create a server that can be tunneled by any kind of stream?
I think that tarpc does such thing

@rmccrystal
Copy link

Hi, gRPC tunneled over WebSocket seems perfect solution for my project. I have spent many hours looking for the solution, I have tried capn proto, tarpc, but it seems no of them are as advanced and as well documented as gRPC.

Couldn't we create a server that can be tunneled by any kind of stream? I think that tarpc does such thing

Tarpc already lets you use any transport that implements AsyncRead + AsyncWrite, it's just a matter of supporting the wasm32 target

@wngr
Copy link

wngr commented Feb 22, 2023

I'd like to warm this up, as this is vastly superior to the grpc-web approach (mostly about hitting the connection limit per host imposed by browsers for multiple streaming requests).

@LucioFranco what's your opinion on this PR? Which are the open issues to get this going again?

@lucasmerlin
Copy link

lucasmerlin commented Jan 7, 2024

I've created an updated version based on current tonic over here: https://github.com/lucasmerlin/tonic/tree/wasm-client-feature
I've also updated tonic-ws-transport to work with my changes: https://github.com/lucasmerlin/tonic-ws-transport/tree/updated-dependencies

Trying this in my project it works flawlessly!

I'm happy to create a new PR with my changes if @boxdot is not interested in working on this anymore

@boxdot
Copy link
Author

boxdot commented Jan 8, 2024

I'm happy to create a new PR with my changes if @boxdot is not interested in working on this anymore

@lucasmerlin Feel free to push to this branch, or open a new PR. Whatever works better for you. Since you did already all the necessary work, I would not wait for me and try to bring it in.

@pbower
Copy link

pbower commented Jun 14, 2024

Hi there, wondering if there's any plans to merge this one in ? Thanks

@flokli
Copy link

flokli commented Jun 14, 2024

I think this has moved to a new PR at #1594

@djc djc closed this Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.