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

Unexpected error type returned from ServerStream.SendMsg() #6548

Closed
ash2k opened this issue Aug 14, 2023 · 8 comments
Closed

Unexpected error type returned from ServerStream.SendMsg() #6548

ash2k opened this issue Aug 14, 2023 · 8 comments

Comments

@ash2k
Copy link
Contributor

ash2k commented Aug 14, 2023

What version of gRPC are you using?

v1.57.0

What version of Go are you using (go version)?

N/A

What operating system (Linux, Windows, …) and version?

N/A

What did you do?

Called SendMsg() on a ServerStream in a streaming RPC.

What did you expect to see?

On error I was expecting to get something that has a GRPCStatus() method i.e. can be converted to the Status type, etc.

What did you see instead?

Based on the error message (in logs and error tracking system), occasionally I get a transport.ConnectionError:

https://github.com/grpc/grpc-go/blob/v1.57.0/internal/transport/transport.go#L782

https://github.com/grpc/grpc-go/blob/v1.57.0/internal/transport/transport.go#L747-L778

This is an internal type and I cannot check for it in a nice way so that I can handle it properly. I also cannot handle it in a generic way (via GRPCStatus()) because it doesn't have that method.

I can see that this type is converted to a "proper" error via toRPCErr() e.g. when passed to stats handlers in https://github.com/grpc/grpc-go/blob/v1.57.0/server.go#L1227 and in other places.

Shouldn't gRPC convert this internal type to a "proper" (i.e. Status) error type before returning it to the user code?

p.s. This may be affecting other methods too, not only SendMsg(), I haven't audited the code.
p.p.s. It'd be very useful to document the contract on what errors can be returned from all public API methods on ServerStream and ClientStream (and anywhere else it makes sense).

@easwars
Copy link
Contributor

easwars commented Aug 15, 2023

The documentation on the ServerStream says the following:

// ServerStream defines the server-side behavior of a streaming RPC.
//
// Errors returned from ServerStream methods are compatible with the status
// package.  However, the status code will often not match the RPC status as
// seen by the client application, and therefore, should not be relied upon for
// this purpose.

Are you relying on the status returned from the ServerStream methods in your server application?

Also, could you please let us know the exact error message you are seeing, so that it will help us track down the exact error.

And is there a way we can reproduce this? Thanks.

@github-actions
Copy link

This issue is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions bot added the stale label Aug 21, 2023
@ash2k
Copy link
Contributor Author

ash2k commented Aug 23, 2023

Sorry for the delay, I'll provide more information soon.

@github-actions github-actions bot removed the stale label Aug 24, 2023
@github-actions
Copy link

This issue is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@timofurrer
Copy link

timofurrer commented Sep 6, 2023

Are you relying on the status returned from the ServerStream methods in your server application?

@easwars We are using it from within a grpc.StreamServerInterceptor. That is, converting the error returned from the handler of the stream server interceptor.

@easwars
Copy link
Contributor

easwars commented Sep 6, 2023

Could you please let us know the exact error message you are seeing, so that it will help us track down the exact error.

And is there a way we can reproduce this? Thanks.

@easwars easwars reopened this Sep 6, 2023
@easwars easwars removed the stale label Sep 6, 2023
@github-actions
Copy link

This issue is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@ash2k
Copy link
Contributor Author

ash2k commented Jan 24, 2024

I think #6891 partially addresses what I'm talking about. I'm not sure if it fully fixes the issue or not. In my case the ConnectionError was returned from SendMsg(). I want to get back to troubleshooting this but had no bandwidth for it so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants