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

grpc-web filter not setting http status code from grpc status #5331

Closed
jwahlin opened this issue Dec 17, 2018 · 7 comments
Closed

grpc-web filter not setting http status code from grpc status #5331

jwahlin opened this issue Dec 17, 2018 · 7 comments
Labels
question Questions that are neither investigations, bugs, nor enhancements

Comments

@jwahlin
Copy link

jwahlin commented Dec 17, 2018

I'm using the grpc-web filter and it isn't setting the http status appropriately from the gRPC status. The gRPC status is coming through the response in the trailers since we're using http2.

To replicate the issue, use the grpc-web filter and send a grpc-web request through Envoy that should fail. The grpc-status will be non-zero, but the http status will show 200. I expect envoy to translate the grpc-status trailer to an http status.

@qiwzhang
Copy link
Contributor

@fengli79 ?

@alyssawilk alyssawilk added the help wanted Needs help! label Dec 17, 2018
@fengli79
Copy link
Contributor

fengli79 commented Dec 17, 2018

@jwahlin, the grpc status is encoded as response body instead of the http/http2 response code.

https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md

HTTP/2 related behavior (specified in gRPC over HTTP2)

  1. stream-id is not supported or used
  2. go-away is not supported or used

@alyssawilk alyssawilk added question Questions that are neither investigations, bugs, nor enhancements and removed help wanted Needs help! labels Dec 17, 2018
@jwahlin
Copy link
Author

jwahlin commented Dec 17, 2018

Thanks for taking a look at this. I do see the grpc-status included in the response trailers. Just to clarify, you're saying that regardless of the grpc-status, the http status is intended to stay at 200, is that right?

I'm not sure how the stream-id and go-away changes are related.

@fengli79
Copy link
Contributor

Thanks for taking a look at this. I do see the grpc-status included in the response trailers. Just to clarify, you're saying that regardless of the grpc-status, the http status is intended to stay at 200, is that right?
Correct.

I'm not sure how the stream-id and go-away changes are related.
They only exist between envoy and your grpc server. Your downstream is using http to talk with envoy, although its can be http2, but semantically it's http in grpc-web, stream id and go-away is not visible there.

@jwahlin
Copy link
Author

jwahlin commented Dec 17, 2018

Thanks for the information. When we use http (and the grpc_http1_bridge), the grpc-status does determine the http status. When we use http2, the grpc-status does not affect the http status. Are there any plans to add a grpc_http2_bridge that sets the http2 status based on the grpc-status?

@fengli79
Copy link
Contributor

grpc_http1_bridge is not considered as a grpc-web compatible filter.
Yes, someone may add a grpc_http2_bridge as needed, but it's pretty much a different mapping from http/http2 to grpc, different from the grpc-web spec.
With grpc-web, you will use the grpc-web js client to handle status and error, so you don't need to worry about the underlying http status code anymore.

@jwahlin
Copy link
Author

jwahlin commented Dec 17, 2018

Makes sense. Thanks for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Questions that are neither investigations, bugs, nor enhancements
Projects
None yet
Development

No branches or pull requests

4 participants