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

Cardinality violations should use error code “unimplemented” #2768

Closed
jhump opened this issue May 30, 2024 · 3 comments
Closed

Cardinality violations should use error code “unimplemented” #2768

jhump opened this issue May 30, 2024 · 3 comments

Comments

@jhump
Copy link
Member

jhump commented May 30, 2024

The gRPC docs for error codes state that both client and server should use the unimplemented code for cardinality violations. See table at the bottom of this doc (you can search for “cardinality violation” in the doc): https://grpc.github.io/grpc/core/md_doc_statuscodes.html.

A cardinality violation is when a stream contains an incorrect number of messages. Specifically, when a response stream for a unary or client-stream RPC contains zero messages with an OK status or more than one message; or when a request stream for a unary or server-stream RPC contains zero or more than one messages.

The server in this repo conforms to the documented behavior, but the client does not.

  1. For unary and client-stream RPCs, if no response messages are present in the response, only an “OK” status, the client reports an internal error (with a message of “No message received”) instead of unimplemented.
  2. For unary and client-stream RPCs, if a second response message is erroneously sent by the server before the trailers/status, the operation appears to hang indefinitely (!!). A metadata event is triggered, but nothing else — no callback, no error event, no status event, and no end event.
@ejona86
Copy link
Member

ejona86 commented May 31, 2024

For the Java issue I mentioned this change to the spec was made without any real notification/discussion. So we may want to re-consider the spec in this case. grpc/grpc-java#11247 (comment)

@ejona86
Copy link
Member

ejona86 commented Jun 5, 2024

To be clear: we should definitely fix the hang, and we don't need agreement on the status code before doing so.

@murgatroid99
Copy link
Member

This is fixed in version 1.10.10.

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

No branches or pull requests

3 participants