-
Notifications
You must be signed in to change notification settings - Fork 791
Update document to mention Nginx server-streaming support #1403
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
Conversation
@@ -13,7 +13,7 @@ frameworks for languages such as Python, Java, and Node. For details, see the | |||
## Streaming Support | |||
gRPC-web currently supports 2 RPC modes: | |||
- Unary RPCs ([example](#make-a-unary-rpc-call)) | |||
- Server-side Streaming RPCs ([example](#server-side-streaming)) (NOTE: Only when [`grpcwebtext`](#wire-format-mode) mode is used.) | |||
- Server-side Streaming RPCs ([example](#server-side-streaming)) (NOTE: Only when [`grpcwebtext`](#wire-format-mode) mode is used, or when [Nginx](#ecosystem) is used) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a surprising / confusing note. Why does Envoy, the gRPC-web recommended proxy, not support server-side streaming in binary, if Nginx is able to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i can agree.. I was just stating the information i gathered, but not trying to reason if it's "expected" or not.. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrasing here is still confusing, even if it's true - it says only the text format supports streaming, but also that Nginx actually supports streaming regardless of format. If true it would be clearer to say that streaming conceptually works with both formats but the Envoy implementation specifically only supports streaming with the text format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah i agree with you.. i didn't call out Envoy specifically because it's not the only proxy that supports grpc web.
Let's get a conclusion on the other comment you have, and then i'll update this text.
@@ -337,15 +337,16 @@ Multiple proxies support the gRPC-web protocol. | |||
$ docker-compose up -d node-server envoy commonjs-client | |||
``` | |||
|
|||
2. You can also try the [gRPC-web Go proxy][]. | |||
2. [Nginx](https://www.nginx.com/) has a grpc-web module ([doc](https://nginx.org/en/docs/http/ngx_http_grpc_module.html), [announcement](https://www.nginx.com/blog/nginx-1-13-10-grpc/)). It seems to work with simple configs, according to [user feedbacks](https://github.com/grpc/grpc-web/discussions/1322)), even with server-streaming support ([user feedback](https://github.com/grpc/grpc-web/issues/1391)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither of these nginx pages mention gRPC-web - are we sure they are referring to gRPC-web and not gRPC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, you're right.. i think that's a very good point.. Somehow i thought they were — I probably misread initially. Maybe they are not..
In any case, there are 2 users reporting grpc-web is working, including the OP and this confirmation, and hence i changed the doc.
Maybe i should wait for one extra confirmation on the binary streaming case, but the OP sounded pretty sure in #1391 so i trusted his judgements :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty suspicious these threads are not consistently distinguishing gRPC traffic from gRPC-web. For example application/grpc
is clearly not gRPC-web. Furthermore, it seems unlikely that Nginx has somehow managed to enable streaming support for the gRPC-web binary format when your understanding was the binary format is incapable of supporting server-side streaming.
More to the point, it's impossible for Nginx to have enabled streaming support for the gRPC-web binary format without collaborating with the gRPC-web project, because the protocol spec directly says this is not supported. So if they have implemented some sort of streaming support it's not to spec.
But my bet (especially given the lack of any mention of gRPC-web in their documentation) is Nginx supports gRPC, and people are conflating it with gRPC-web because it's a "web" server that's able to proxy gRPC traffic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty suspicious these threads are not consistently distinguishing gRPC traffic from gRPC-web. For example
application/grpc
is clearly not gRPC-web.
Yeah i can see your suspicion. I don't deny them.
Furthermore, it seems unlikely that Nginx has somehow managed to enable streaming support for the gRPC-web binary format when your understanding was the binary format is incapable of supporting server-side streaming.
This is a very good point.. thanks for pointing it out :)
More to the point, it's impossible for Nginx to have enabled streaming support for the gRPC-web binary format without collaborating with the gRPC-web project, because the protocol spec directly says this is not supported. So if they have implemented some sort of streaming support it's not to spec.
Yes another very good point.
But my bet (especially given the lack of any mention of gRPC-web in their documentation) is Nginx supports gRPC, and people are conflating it with gRPC-web because it's a "web" server that's able to proxy gRPC traffic.
Yes it's definitely a possibility. Let me loop in the original commenters.
@jmzwcn @zhumazhenis Hi! Thanks for previously participating in the discussion on nginx support.
There's a question now on the validity of the conclusions as above.
Could you help responding to it if there are some misunderstandings?
Thanks in advance!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sampajano any objection to reverting these notes? At best they're unclear and IMO it would be better to remove them and hash out exactly what is known before re-adding them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK! Since the original commenters are not active at this point, i guess we can revert until upon further discussions :)
Do you want to create a PR to revert this one? Or if you prefer i can do it too.
Thanks! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, just cut #1408
Discussed in following threads:
#1391
#1322