Skip to content

Parse trailers even when an error occurs#3365

Merged
oldergod merged 1 commit intosquare:masterfrom
ahmedre:read_trailers_on_error
Feb 4, 2026
Merged

Parse trailers even when an error occurs#3365
oldergod merged 1 commit intosquare:masterfrom
ahmedre:read_trailers_on_error

Conversation

@ahmedre
Copy link
Contributor

@ahmedre ahmedre commented Aug 21, 2025

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.

exception = response.grpcResponseToException()
} catch (e: IOException) {
exception = response.grpcResponseToException(e)
exception = response.grpcResponseToException(false, e)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link

@squarejesse squarejesse left a comment

Choose a reason for hiding this comment

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

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

@ahmedre ahmedre force-pushed the read_trailers_on_error branch from f86e697 to 1daf6d2 Compare August 22, 2025 10:47
@ahmedre
Copy link
Contributor Author

ahmedre commented Aug 22, 2025

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

thanks - done.

@ahmedre ahmedre force-pushed the read_trailers_on_error branch from 1daf6d2 to 3541dc4 Compare August 22, 2025 10:54
@oldergod
Copy link
Member

oldergod commented Feb 3, 2026

@ahmedre sorry for the delay. Let's rebase and we should be good to go

@ahmedre ahmedre force-pushed the read_trailers_on_error branch from 3541dc4 to 7ea3bf6 Compare February 4, 2026 15:13
@oldergod
Copy link
Member

oldergod commented Feb 4, 2026

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.
@ahmedre ahmedre force-pushed the read_trailers_on_error branch from 7ea3bf6 to f9b777f Compare February 4, 2026 19:42
@oldergod oldergod enabled auto-merge February 4, 2026 21:21
@oldergod oldergod merged commit c799956 into square:master Feb 4, 2026
9 checks passed
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.

3 participants