Conflicting gRPC Response

[michaelharrigan]

• [x] Searched the repo's previous issues for similar reports.
• [x] Searched the internet for any similar reports.

Issue Summary

I am encountering an unusual issue with the response object for a gRPC message. While the statusCode indicates ok and result returns success, the headers provide contradictory information. Specifically, the headers contain a status code of 2, and the grpc-message reports an error.

This conflicting information is making it challenging to determine whether an operation was successful or not.

Detailed Description

I've confirmed that this issue originates from the client side by using Postman to access the same endpoint, which returns the correct information.

iOS Response:

In the iOS response, the following points illustrate the inconsistency:

1. The status code is displayed as ok/0.
2. The headers show a statusCode of 2/unknown error.
3. The grpc-message in the headers also reports an error.

Postman Response:

Contrastingly, when using Postman, I receive the correct response with no contradictory information.

Possible Cause

Through my investigation, I believe I've identified a potential cause of this issue. In the ProtocolClient, specifically in the unary(path:request:headers:completion:) function, the onResponse block checks response.code != .ok to throw an error. However, in this case, response.code is 0, while the headers indicate 2. Consequently, success is returned instead of raising a ConnectError with .failure.

[rebello95]

Thanks for filing such a detailed issue @michaelharrigan!

I think you're on the right track with the Possible Cause - the GRPCInterceptor is responsible from mapping trailers into appropriate status codes for the gRPC protocol: https://github.com/bufbuild/connect-swift/blob/c041679d67ff88ce4dcf1d38efa48470087af07c/Libraries/ConnectNIO/Internal/GRPCInterceptor.swift#L53

Based on the screenshot of the response you shared, it looks like trailers is empty while headers contains the grpc-status header, which I think is unexpected since the gRPC protocol spec states:

Most responses are expected to have both headers and trailers but Trailers-Only is permitted for calls that produce an immediate error. Status must be sent in Trailers even if the status code is OK.

This is why the code I linked above is only looking at trailers for the status code. Is it possible that the service is returning an invalid response by not putting grpc-status in the trailers?

I'd also like to confirm that you're using the NIOHTTPClient with this, not URLSessionHTTPClient?

[timostamm]

This is most likely a "trailers-only" response, which - contrary to what one might think - does not use trailers.

Response-Headers & Trailers-Only are each delivered in a single HTTP2 HEADERS frame block. Most responses are expected to have both headers and trailers but Trailers-Only is permitted for calls that produce an immediate error.

Here is an example for such a response:

< HTTP/2 200 
< content-type: application/grpc
< grpc-status: 3
< grpc-message: invalid argument

It does not have a body and no trailers either, but it is valid in gRPC, although poorly documented.

[aymericbeaumet]

Hello @rebello95, I'm working on the backend @michaelharrigan is querying.

Is it possible that the service is returning an invalid response by not putting grpc-status in the trailers?

We use the https://github.com/connectrpc/connect-go package. I confirm I don't see grpc-status in the trailers.

This is an example of a request returning an error, with a status code 2.

[michaelharrigan]

I'd also like to confirm that you're using the NIOHTTPClient with this, not URLSessionHTTPClient?

@rebello95 confirming that we do use the NIOHTTPClient.

[rebello95]

Thanks for those details @aymericbeaumet @michaelharrigan. It looks like @timostamm is correct and that a couple of changes need to be made to the library to handle those responses. I'll make the changes later today!

[michaelharrigan]

Thanks for those details @aymericbeaumet @michaelharrigan. It looks like @timostamm is correct and that a couple of changes need to be made to the library to handle those responses. I'll make the changes later today!

@rebello95 thank you for the quick responses & upcoming fix!

[rebello95]

No worries 👍🏽

I'm still trying to see if we have a test case available for this, but would you mind giving the branch on #174 a try to see if that solves your issue @michaelharrigan?

[michaelharrigan]

No worries 👍🏽

I'm still trying to see if we have a test case available for this, but would you mind giving the branch on #174 a try to see if that solves your issue @michaelharrigan?

@rebello95 good news, it 100% solves my issue! 🎉 Appreciate the quick turnaround here.

[rebello95]

Awesome! I'll have someone from our team review the PR and land it as soon as possible

[rebello95]

Ended up adding some additional logic and tests to that PR, and it's available on main now 😃