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

Fix grpc transcoding content type not matching response body #8312

Merged
merged 4 commits into from
Sep 24, 2019

Conversation

nareddyt
Copy link
Contributor

@nareddyt nareddyt commented Sep 20, 2019

For a trailer-only gRPC streaming response, ensure that the returned content type is application/json. This is required because the response body is an empty JSON array.

Risk Level: Low
Testing: Added an integration test to catch this bug.
Docs Changes: None
Release Notes: None

Fixes: #5011
Ref: #8158
Ref: #8009

Please note this fixes issues for responses that are:

  • Trailer-only (either an empty body or a gRPC error)
  • Streaming (not unary)

Signed-off-by: Teju Nareddy [email protected]

…e that the returned content type is `application/json`. This is required because the response body is an empty JSON body by default.

Risk Level: Low
Testing: Modified integration test to catch this bug.
Docs Changes: None
Release Notes: None

Fixes: envoyproxy#5011
Ref: envoyproxy#8158
Ref: envoyproxy#8009
Signed-off-by: Teju Nareddy <[email protected]>
@lizan
Copy link
Member

lizan commented Sep 20, 2019

@nareddyt does this PR supersede #8158?

@lizan lizan self-assigned this Sep 20, 2019
@lizan lizan added the waiting label Sep 20, 2019
@nareddyt
Copy link
Contributor Author

Yes, @qiwzhang asked me take it over.

We may have misunderstood the original issue, it seems like the error is only for streaming backends. I need to look into this further (and perhaps add a new integration test)

@nareddyt
Copy link
Contributor Author

Ready for review 👍

Signed-off-by: Teju Nareddy <[email protected]>
Signed-off-by: Teju Nareddy <[email protected]>
@lizan lizan merged commit ccb303f into envoyproxy:master Sep 24, 2019
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
…oxy#8312)

* When trailer indicates a gRPC error and there was no HTTP body, ensure that the returned content type is `application/json`. This is required because the response body is an empty JSON body by default.

Risk Level: Low
Testing: Modified integration test to catch this bug.
Docs Changes: None
Release Notes: None

Fixes: envoyproxy#5011
Ref: envoyproxy#8158
Ref: envoyproxy#8009

Signed-off-by: Teju Nareddy <[email protected]>
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.

grpc-json transcoder: stream return type always returns 200 even in case of errors
3 participants