-
Notifications
You must be signed in to change notification settings - Fork 764
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
GRPC within service worker / web worker #587
Comments
same problem here |
We have a patch in google-closure to support service workers (via fetch) and will try to get the change to github soon. |
@wenbozhu has this been resolved yet? |
@travikk if it's any use to hear, we've been using grpc-web in web workers without a problem. |
Same problem, I moved forward a bit with using but I'm getting:
ugh an no way of knowing how else to proceed |
It looks like the gRPC client accepts an instance of
Which accepts an It seems to me that if one can get an instance of grpc-web/javascript/net/grpc/web/clientoptions.js Lines 48 to 54 in 0350052
After finishing the implementation so that it reads However, I haven't tested server-side streaming using this mode. If I open a PR to upstream this change, would that be helpful to push this along? Even if there are some features not supported using fetch, those could be noted as a caveat and with helpful error messages explaining why. |
@tjhorner Hi! Thanks for your interest and digging here! Much appreciated! Actually, i've realized that this feature was implemented on the internal fork but not open sourced.. It should actually be quite an easy change!
There's another option called grpc-web/javascript/net/grpc/web/clientoptions.js Lines 57 to 62 in 0350052
Our internal implementation is quite simple too. Basically replacing the following line:
with something like this: newXhr(isUnary) {
const useBinaryChunks = this.chunkedServerStreaming_ && !isUnary;
if (this.workerScope_ || useBinaryChunks) {
const xmlHttpFactory = new FetchXmlHttpFactory({
worker: this.workerScope_,
streamBinaryChunks: useBinaryChunks,
});
return new XhrIo(xmlHttpFactory);
}
return new XhrIo();
}
Thank you for the offering here! Appreciate it! I think it can certainly help move things along here!! 😃 The only thing is, we're currently in the process of doing a Typescript migration, so it might not be a great timing to take PRs if this takes an extended amount of time. In that case it would be better to wait until the migration is over before we do this. But if it's a really simple change, i don't mind taking it up sooner too! 😃 |
@sampajano Thank you for the very quick response! My solution was remarkably similar, minus the grpc-web/javascript/net/grpc/web/grpcwebclientbase.js Lines 184 to 191 in 6c41064
I can definitely submit a PR if you think it's a small enough change. But if I do, something I would like more info on is testing strategy. Not sure how thorough you'd want the tests for this (e.g., should There's no rush on my end, however. For the project we're using this for at work, we are vendoring the library with these changes, so I'm ok with waiting for a more official release in the meantime. Let me know what you think! |
Oh great! Glad to note! :) Could you try passing the
Really appreciate the considerations! Yes def agreed that we should ensure it's tested 😃 Luckily we have an interop test already i think you can probably easily reuse! (no need for mocking! :) I think you can just:
if (argv.use_fetch) { ... } Where you'd set a global constant or something to pass the new fetch option to all If you don't mind, could you try the above and see how it works? Lemme know if you run into any issues. Thanks! 😃 |
Im trying to run GRPC from inside a service worker. It works fine from inside the main application thread and only throws an error from inside a service worker.
Error:
Im assuming this is due to how GRPC communicates to the backend services and assuming this is not possible to do at the moment?
Thanks
The text was updated successfully, but these errors were encountered: