Error message on server streaming endpoint does not get delimiter appended

[davidedmonds]

🐛 Bug Report

When calling a Server Streaming grpc endpoint through grpc-gateway, when an error is returned, it does not have a delimiter appended (by default, \n) unlike other streaming messages.

To Reproduce

1. Call an endpoint that maps onto a Server Streaming grpc endpoint, that returns an error.

Expected behavior

grpc-gateway returns {"error":....}\n (or other configured delimiter)

Actual Behavior

grpc-gateway returns {"error":....} (without the configured delimiter)

Your Environment

Mac OS 12.2.1, Go 1.17.6, grpc-gateway v2.8.0, google.golang.org/grpc v1.45.0

Note

Am aware this might not be a "bug" - the behaviour is always the same when passing errors, so it might be as designed. If that's the case, please don't hesitate to tell me so :D

Should this be considered a bug, am happy to try to get a PR in as well.

[johanbrandhorst]

I'm kinda leaning towards this not being a bug, but what is the behavior for successful streams? Does the last message include the delimiter at the end? If so, we should probably include it in errors as well.

[davidedmonds]

Yes - the last message includes a delimiter if it is not an error response.

When an error is returned:

❯ curl --request GET  --url http://localhost/with-error -iv --raw
Note: Unnecessary use of -X or --request, GET is already inferred.
*   Trying 127.0.0.1:80...
* Connected to localhost (127.0.0.1) port 80 (#0)
> GET /match-request/abc-123 HTTP/1.1
> Host: localhost
> User-Agent: curl/7.77.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
HTTP/1.1 200 OK
< Content-Type: application/json
Content-Type: application/json
< Grpc-Metadata-Content-Type: application/grpc
Grpc-Metadata-Content-Type: application/grpc
< Date: Fri, 18 Mar 2022 16:45:16 GMT
Date: Fri, 18 Mar 2022 16:45:16 GMT
< Transfer-Encoding: chunked
Transfer-Encoding: chunked

<
4d
{"result":{/*...*/}}

4b
{"result":{/*...*/}}

4c
{"result":{/*...*/}}

38
{"error":{"code":2,"message":"test error","details":[]}}
0

* Connection #0 to host localhost left intact

When no error is returned

❯ curl --request GET  --url http://localhost/no-error -iv --raw
Note: Unnecessary use of -X or --request, GET is already inferred.
*   Trying 127.0.0.1:80...
* Connected to localhost (127.0.0.1) port 80 (#0)
> GET /match-request/abc-123 HTTP/1.1
> Host: localhost
> User-Agent: curl/7.77.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
HTTP/1.1 200 OK
< Content-Type: application/json
Content-Type: application/json
< Grpc-Metadata-Content-Type: application/grpc
Grpc-Metadata-Content-Type: application/grpc
< Date: Fri, 18 Mar 2022 16:45:47 GMT
Date: Fri, 18 Mar 2022 16:45:47 GMT
< Transfer-Encoding: chunked
Transfer-Encoding: chunked

<
4d
{"result":{/*...*/}}

4b
{"result":{/*...*/}}

4c
{"result":{/*...*/}}

0

* Connection #0 to host localhost left intact

[johanbrandhorst]

In that case I think you're right, we should add the delimiter after the error. It hopefully shouldn't break any clients. Would you be willing to contribute this fix?

[stelcodes]

Hi @johanbrandhorst, I'm trying to make my first contribution and I think I solved the issue. Just posted PR #2596

[davidedmonds]

@stelcodes looks like you beat me to it :) TYVM <3