Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EOF when calling Send for client streams #961

Closed
gustavocovas opened this issue Jun 25, 2019 · 1 comment
Closed

EOF when calling Send for client streams #961

gustavocovas opened this issue Jun 25, 2019 · 1 comment
Labels

Comments

@gustavocovas
Copy link
Contributor

@jonasarilho and I are working on a service with a client-side streaming RPC. We noticed that grpc-gateway does not handle correctly errors that occur before it is done processing the request body. For instance:

  1. The gateway receives a request that will result in an UNAUTHENTICATED status from the back-end service, because of invalid credentials that will be forwarded via metadata.
  2. The back-end service receives the RPC call and returns the expected status.
  3. If the gateway is not done processing the request, the generated code calls stream.Send, which returns io.EOF, as described in the docs.
  4. The returned io.EOF results in a 500 status, instead of a 401.

We believe that adding a verification for io.EOF on the generated code template should fix this issue:

if err = stream.Send(&protoReq); err != nil {
	if err == io.EOF {
		break
	}
	grpclog.Infof("Failed to send request: %v", err)
	return nil, metadata, err
}

Is this solution in the right direction? Since this would be our first contribution, we appreciate help to turn this into a PR.
Thanks

@johanbrandhorst
Copy link
Collaborator

TIL we support client side streaming. Cool! This suggestion sounds good to me. Please feel free to open a PR, but we'll probably want to add some tests here as well. I think it'd be cool to add something to the integration tests. What do you think?

adasari pushed a commit to adasari/grpc-gateway that referenced this issue Apr 9, 2020
* Add integration test for grpc-ecosystem#961

Co-authored-by: Jonas Arilho <[email protected]>

* Add verification for io.EOF after stream.Send() on generated code template (grpc-ecosystem#961)

Co-authored-by: Jonas Arilho <[email protected]>

* Add more values on testABEBulkCreateWithError, run go mod tidy

Fixes grpc-ecosystem#961
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants