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” #7286

Open
jhump opened this issue May 30, 2024 · 6 comments
Open

Cardinality violations should use error code “unimplemented” #7286

jhump opened this issue May 30, 2024 · 6 comments
Labels
Area: RPC Features Includes Compression, Encoding, Attributes/Metadata, Interceptors. Status: Help Wanted Type: Bug

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 client and server in this Go module both return an unknown error in this situation instead of unimplemented.

@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)

@purnesh42H purnesh42H self-assigned this Jun 4, 2024
@purnesh42H purnesh42H added the P2 label Jun 14, 2024
@purnesh42H
Copy link
Contributor

@jhump we have to reconsider on deciding the error code for cardinality violations. For now, as long as an error code is being returned, we won't be making any changes.

@dfawley
Copy link
Member

dfawley commented Jun 21, 2024

I think we should leave this open until we decide what the behavior should be.

@dfawley dfawley reopened this Jun 21, 2024
@easwars
Copy link
Contributor

easwars commented Jun 26, 2024

Should we mark it blocked on the change to the spec, as promised in: grpc/grpc-java#11247 (comment)

@dfawley
Copy link
Member

dfawley commented Jun 26, 2024

I believe we've fully committed to making it INTERNAL. I don't think we need to wait on a doc update to implement it.

@ejona86 WDYT?

@dfawley
Copy link
Member

dfawley commented Jul 30, 2024

Per @ejona86 offline, it's fine to go ahead and make this change before the gRFC happens. We've agreed on that change, just need to go through the process to make it happen, and the current behavior doesn't match the spec already.

@purnesh42H purnesh42H added the Area: RPC Features Includes Compression, Encoding, Attributes/Metadata, Interceptors. label Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: RPC Features Includes Compression, Encoding, Attributes/Metadata, Interceptors. Status: Help Wanted Type: Bug
Projects
None yet
Development

No branches or pull requests

5 participants