Skip to content

grpc: revert #8278: Fix cardinality violations in non-server streaming RPCs #8404

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.

Sign up for GitHub

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

Merged
arjan-bal merged 1 commit into from
Jun 23, 2025
Merged

grpc: revert #8278: Fix cardinality violations in non-server streaming RPCs #8404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions stream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1138,10 +1138,6 @@
if statusErr := a.transportStream.Status().Err(); statusErr != nil {
return statusErr
}
// Received no msg and status OK for non-server streaming rpcs.
if !cs.desc.ServerStreams {
return status.Error(codes.Internal, "cardinality violation: received no response message from non-streaming RPC")
}
return io.EOF // indicates successful end of stream.
}

Expand Down Expand Up @@ -1175,7 +1171,7 @@
} else if err != nil {
return toRPCErr(err)
}
return status.Error(codes.Internal, "cardinality violation: expected <EOF> for non server-streaming RPCs, but received another message")
return status.Errorf(codes.Internal, "cardinality violation: expected <EOF> for non server-streaming RPCs, but received another message")
}

func (a *csAttempt) finish(err error) {
Expand Down Expand Up @@ -1482,10 +1478,6 @@
if statusErr := as.transportStream.Status().Err(); statusErr != nil {
return statusErr
}
// Received no msg and status OK for non-server streaming rpcs.
if !as.desc.ServerStreams {
return status.Error(codes.Internal, "cardinality violation: received no response message from non-streaming RPC")
}
return io.EOF // indicates successful end of stream.
}
return toRPCErr(err)
Expand All @@ -1503,7 +1495,7 @@
} else if err != nil {
return toRPCErr(err)
}
return status.Error(codes.Internal, "cardinality violation: expected <EOF> for non server-streaming RPCs, but received another message")
return status.Errorf(codes.Internal, "cardinality violation: expected <EOF> for non server-streaming RPCs, but received another message")

Check warning on line 1498 in stream.go

View check run for this annotation

Codecov / codecov/patch

stream.go#L1498 

Added line #L1498 was not covered by tests
}

func (as *addrConnStream) finish(err error) {
Expand Down
110 changes: 4 additions & 106 deletions test/end2end_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3589,6 +3589,9 @@ func testClientStreamingError(t *testing.T, e env) {
// Tests that a client receives a cardinality violation error for client-streaming
// RPCs if the server doesn't send a message before returning status OK.
func (s) TestClientStreamingCardinalityViolation_ServerHandlerMissingSendAndClose(t *testing.T) {
// TODO : https://github.com/grpc/grpc-go/issues/8119 - remove `t.Skip()`
// after this is fixed.
t.Skip()
ss := &stubserver.StubServer{
StreamingInputCallF: func(_ testgrpc.TestService_StreamingInputCallServer) error {
// Returning status OK without sending a response message.This is a
Expand Down Expand Up @@ -3737,113 +3740,8 @@ func (s) TestClientStreaming_ReturnErrorAfterSendAndClose(t *testing.T) {
}
}

// Tests that a client receives a cardinality violation error for unary
// RPCs if the server doesn't send a message before returning status OK.
func (s) TestUnaryRPC_ServerSendsOnlyTrailersWithOK(t *testing.T) {
lis, err := testutils.LocalTCPListener()
if err != nil {
t.Fatal(err)
}
defer lis.Close()

ss := grpc.UnknownServiceHandler(func(any, grpc.ServerStream) error {
return nil
})

s := grpc.NewServer(ss)
go s.Serve(lis)
defer s.Stop()

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
cc, err := grpc.NewClient(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("grpc.NewClient(%q) failed unexpectedly: %v", lis.Addr(), err)
}
defer cc.Close()

client := testgrpc.NewTestServiceClient(cc)
if _, err = client.EmptyCall(ctx, &testpb.Empty{}); status.Code(err) != codes.Internal {
t.Errorf("stream.RecvMsg() = %v, want error %v", status.Code(err), codes.Internal)
}
}

// Tests that client will receive cardinality violations when calling
// RecvMsg() multiple times for non-streaming response streams.
func (s) TestUnaryRPC_ClientCallRecvMsgTwice(t *testing.T) {
e := tcpTLSEnv
te := newTest(t, e)
defer te.tearDown()

te.startServer(&testServer{security: e.security})

cc := te.clientConn()
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()

desc := &grpc.StreamDesc{
StreamName: "UnaryCall",
ServerStreams: false,
ClientStreams: false,
}
stream, err := cc.NewStream(ctx, desc, "/grpc.testing.TestService/UnaryCall")
if err != nil {
t.Fatalf("cc.NewStream() failed unexpectedly: %v", err)
}

if err := stream.SendMsg(&testpb.SimpleRequest{}); err != nil {
t.Fatalf("stream.SendMsg(_) = %v, want <nil>", err)
}

resp := &testpb.SimpleResponse{}
if err := stream.RecvMsg(resp); err != nil {
t.Fatalf("stream.RecvMsg() = %v , want <nil>", err)
}

if err = stream.RecvMsg(resp); status.Code(err) != codes.Internal {
t.Errorf("stream.RecvMsg() = %v, want error %v", status.Code(err), codes.Internal)
}
}

// Tests that client will receive cardinality violations when calling
// RecvMsg() multiple times for non-streaming response streams.
func (s) TestClientStreaming_ClientCallRecvMsgTwice(t *testing.T) {
ss := stubserver.StubServer{
StreamingInputCallF: func(stream testgrpc.TestService_StreamingInputCallServer) error {
if err := stream.SendAndClose(&testpb.StreamingInputCallResponse{}); err != nil {
t.Errorf("stream.SendAndClose(_) = %v, want <nil>", err)
}
return nil
},
}
if err := ss.Start(nil); err != nil {
t.Fatal("Error starting server:", err)
}
defer ss.Stop()

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
stream, err := ss.Client.StreamingInputCall(ctx)
if err != nil {
t.Fatalf(".StreamingInputCall(_) = _, %v, want <nil>", err)
}
if err := stream.Send(&testpb.StreamingInputCallRequest{}); err != nil {
t.Fatalf("stream.Send(_) = %v, want <nil>", err)
}
if err := stream.CloseSend(); err != nil {
t.Fatalf("stream.CloseSend() = %v, want <nil>", err)
}
resp := new(testpb.StreamingInputCallResponse)
if err := stream.RecvMsg(resp); err != nil {
t.Fatalf("stream.RecvMsg() = %v , want <nil>", err)
}
if err = stream.RecvMsg(resp); status.Code(err) != codes.Internal {
t.Errorf("stream.RecvMsg() = %v, want error %v", status.Code(err), codes.Internal)
}
}

// Tests that a client receives a cardinality violation error for client-streaming
// RPCs if the server call SendMsg() multiple times.
// RPCs if the server call SendMsg multiple times.
func (s) TestClientStreaming_ServerHandlerSendMsgAfterSendMsg(t *testing.T) {
ss := stubserver.StubServer{
StreamingInputCallF: func(stream testgrpc.TestService_StreamingInputCallServer) error {
Expand Down
Loading