Parse trailers even when an error occurs#3365
Conversation
| exception = response.grpcResponseToException() | ||
| } catch (e: IOException) { | ||
| exception = response.grpcResponseToException(e) | ||
| exception = response.grpcResponseToException(false, e) |
There was a problem hiding this comment.
intentionally only did this for unary calls because there's a specific test to disallow this for streaming (duplexSuspend_responseBodyThrows) - i suspect this is because when something goes wrong, the channel is explicitly closed, therefore making it not possible to read the trailers.
squarejesse
left a comment
There was a problem hiding this comment.
I think if we want to do this, we should detect an empty gRPC response, and not throw an exception in that case before we read the trailers
f86e697 to
1daf6d2
Compare
thanks - done. |
1daf6d2 to
3541dc4
Compare
|
@ahmedre sorry for the delay. Let's rebase and we should be good to go |
3541dc4 to
7ea3bf6
Compare
|
OkHttp bumped in between and some annotations don't need to be anymore |
It is possible for gRPC error responses to be empty and for servers to send the error information within trailers without also sending them in headers. In these cases, Wire will propagate an exception due to no response body and will not attempt to read trailers, relying only on headers instead. This patch allows Wire to attempt to parse the trailers for non-streaming requests instead of only relying on headers.
7ea3bf6 to
f9b777f
Compare
It is possible for gRPC error responses to be empty and for servers to send the error information within trailers without also sending them in headers. In these cases, Wire will propagate an exception due to no response body and will not attempt to read trailers, relying only on headers instead.
This patch allows Wire to attempt to parse the trailers for non-streaming requests instead of only relying on headers.