Skip to content

Conversation

@JonathanArns
Copy link

Fixes an issue where grpcbox ignores errors that are returned from the callback function of a streaming RPC. This happened both on RPCs with streaming and non-streaming output, though the behaviour was different: For non-streaming output, grpcbox always returned a generic error, generated in grpcbox_stream.erl line 425 (on this commit).
For streaming output, errors were completely ignored and the stream was closed without an error indication to the client.

Also fixes a small issue where grpcbox passed incorrect ServerInfo to streaming interceptors for RPCs with only streaming output.

Fixes an issue where grpcbox ignores errors that are returned from
the callback function of a streaming RPC. This happened both on
RPCs with streaming and non-streaming output, though the behaviour
was different: For non-streaming output, grpcbox always returned
a generic error, generated in `grpcbox_stream.erl` line 425 (on this
commit).
For streaming output, errors were completely ignored and the
stream was closed without an error indication to the client.

Also fixes a small issue where grpcbox passed incorrect ServerInfo
to streaming interceptors for RPCs with only streaming output.
@tsloughter
Copy link
Owner

@JonathanArns sorry for not getting to this sooner. Is there a decent way to add a test for this by chance?

@JonathanArns
Copy link
Author

Hi, yes I can come up with a test.
We (@tomas-abrahamsson and @mzeuner) also have a handful of other small fixes for grpcbox and chatterbox that we have been working on over the last couple months. I think we would like to try to upstream all of those soon.

@tsloughter
Copy link
Owner

I just realized you are at Ericsson. Funny enough I just emailed Rickard Green to check if grpcbox was still in use :)

@tsloughter
Copy link
Owner

And I would love additional tests, thanks!

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.

2 participants