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

Unary response with message and status in the trailers #786

Closed
JamesNK opened this issue Apr 21, 2020 · 3 comments · Fixed by #847
Closed

Unary response with message and status in the trailers #786

JamesNK opened this issue Apr 21, 2020 · 3 comments · Fixed by #847

Comments

@JamesNK
Copy link
Member

JamesNK commented Apr 21, 2020

I believe there is an issue in the gRPC-Web JavaScript client. When a unary response is returned with a message AND a non-OK status in the trailers then the JavaScript client assumes the call was successful. The JavaScript client assumes the call is successful because there is a message and doesn't check for a status in trailers.

The client should be updated to check for a non-OK status in trailers when processing unary calls.

@JamesNK JamesNK changed the title gRPC-Web unary response with message and status in the trailers Unary response with message and status in the trailers Apr 21, 2020
@stanley-cheung
Copy link
Collaborator

Reference thread: grpc/grpc#12824 (comment)

@wenbozhu
Copy link
Member

wenbozhu commented Jun 9, 2020

Should the message be discarded too, in this case?

I would consider such a response being generated from an ill-behaved server or is there any legit case for servers to generate the error following the response message?

@stanley-cheung
Copy link
Collaborator

Yes the message should be discarded if the status is non-OK, according to the link above. I can't quite actually do it though, (i.e. have a server return both a message and a non-OK status) outside of a unit-test setting. But I only tried Ruby and Node server - there seems to be no way to send both a message and a non-OK status. It may be possible for other languages though. I approached this from the unit-test perspective.

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 a pull request may close this issue.

3 participants