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

Use CodeInternal when user-supplied Codecs can't marshal Status #201

Merged
merged 3 commits into from
Apr 19, 2022

Conversation

akshayjshah
Copy link
Member

Fixes #197.

If a user-supplied Codec errors when marshaling a protobuf Status
message (which we use internally when converting errors to HTTP
trailers), we currently drop all the error information on the floor and
return "Unknown: EOF".

This commit adds a reproduction as a test case. Relates to #197.
If we can't marshal a protobuf Status message, send CodeInternal to the
client with some details.
Copy link
Contributor

@doriable doriable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, this is a nice find, @robbertvanginkel :D
This fix seems quite reasonable to me :stamp:

@akshayjshah akshayjshah merged commit 87a8793 into main Apr 19, 2022
@akshayjshah akshayjshah deleted the ajs/197 branch April 19, 2022 03:34
akshayjshah added a commit that referenced this pull request Jul 26, 2023
* Add test for errors marshaling protobuf Status

If a user-supplied Codec errors when marshaling a protobuf Status
message (which we use internally when converting errors to HTTP
trailers), we currently drop all the error information on the floor and
return "Unknown: EOF".

This commit adds a reproduction as a test case. Relates to #197.

* Use CodeInternal for errors marshaling proto Status

If we can't marshal a protobuf Status message, send CodeInternal to the
client with some details.

* Fix error-wrapping lint
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.

Custom codec status marshal are swallowed and cause invalid grpc responses
2 participants