streaming: should return cancel error instead of Internal error with unexpected EOF

[fuweid]

What version of gRPC are you using?

v1.72.0

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

v1.24.2

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

Linux

What did you do?

In the ETCD integration test, there is a case that verifies the client receives a context canceled error when reading data from a streaming RPC and the context is subsequently canceled. The steps would be like

• Create a context with a one-second timeout (ctx).
• Use that ctx to initiate the streaming RPC, and start a new goroutine to read the stream and write received data into a pipe.
• The client sleeps for two seconds and then attempts to read from the pipe.
  • NOTE: The background goroutine may read one message before timeout. https://github.com/etcd-io/etcd/blob/0a0fde11ace45eaf596862df35aae833b8e6d649/client/v3/maintenance.go#L284-L293
• The read should return a context canceled error.

# in etcd main branch
$ cd ./tests/integration/clientv3
$ go test -v ./ -count=10 --failfast -run TestMaintenanceSnapshotTimeout

What did you expect to see?

I should receice context canceled error

What did you see instead?

received rpc error: code = Internal desc = unexpected EOF

I can't reproduce this issue with v1.71.0 version. Right now, I found that

grpc-go/rpc_util.go

Line 676 in a43eba6

if err == io.EOF {

- the p.r.Read returns io.EOF and then convert it into unexpectedEOF.

REF: etcd-io/etcd#19833 (comment)

cc @ahrtr @dfawley

[ahrtr]

To add more context to clarify this issue from users perspective.

When etcd calls the Recv method (generated gRPC code, see below, also see here), when the context (the context used when creating the gRPC stream, see here ) timeout,

• previously (on grpc-go 1.71), the Recv always returned a rpc error: code = DeadlineExceeded desc = context deadline exceeded error.
• After we bumped to grpc-go v1.72, the Recv sometimes returns the same expected error, but sometimes return unexpected error rpc error: code = Internal desc = unexpected EOF. This changed behaviour makes our test flaky.

func (x *maintenanceSnapshotClient) Recv() (*SnapshotResponse, error) {
	m := new(SnapshotResponse)
	if err := x.ClientStream.RecvMsg(m); err != nil {
		return nil, err
	}
	return m, nil
}

I believe this should be a generic problem for all grpc-go's users, not specific to etcd.

[arjan-bal]

I suspect this is a bug in the gRPC client-side stream handling that was exposed by changes in #8071. This PR made gRPC Go servers cancel the stream on seeing the deadline being exceeded by sending an RST_STREAM with status code CANCELLED. The client monitors the deadline too, so it's a race between the client cancelling the stream and the server cancelling the stream. Usually clients would cancel the RPC before receiving the cancellation from the server.

On the client side, the handleRSTStream method which converts the CANCELLED status to a DEADLINE_EXCEEDED and calls closeStream() with an io.EOF error.

grpc-go/internal/transport/http2_client.go

Lines 1257 to 1265 in 080f956

	if statusCode == codes.Canceled {
	if d, ok := s.ctx.Deadline(); ok && !d.After(time.Now()) {
	// Our deadline was already exceeded, and that was likely the cause
	// of this cancellation. Alter the status code accordingly.
	statusCode = codes.DeadlineExceeded
	}
	}
	t.closeStream(s, io.EOF, false, http2.ErrCodeNo, status.Newf(statusCode, "stream terminated by RST_STREAM with error code: %v", f.ErrCode), nil, false)
	}

closeStream() writes this io.EOF error for the reading goroutine to stop gracefully.

grpc-go/internal/transport/http2_client.go

Lines 942 to 945 in 080f956

	if err != nil {
	// This will unblock reads eventually.
	s.write(recvMsg{err: err})
	}

As pointed out in the issue description, this io.EOF gets converted to an io.ErrUnexpectedEOF if the reader was in the middle of reading a gRPC message. This could happen when a gRPC message is split across multiple HTTP/2 data frames.

grpc-go/rpc_util.go

Lines 674 to 680 in a43eba6

	data, err := p.r.Read(int(length))
	if err != nil {
	if err == io.EOF {
	err = io.ErrUnexpectedEOF
	}
	return 0, nil, err
	}

In stream.go, we have multiple places where we return the RPC status when we encounter an io.EOF while reading the stream, example

grpc-go/stream.go

Lines 1134 to 1139 in 080f956

	if err == io.EOF {
	if statusErr := a.transportStream.Status().Err(); statusErr != nil {
	return statusErr
	}
	return io.EOF // indicates successful end of stream.
	}

As the io.EOF is converted to an io.ErrUnexpectedEOF, the RPC status is not returned.

[dfawley]

Good find! That sounds like exactly what's happening.

Is there any value in the io.UnexpectedEOF? Or can we use the stream's status (if there is one, which I think there always should be) in this case?

[arjan-bal]

Is there any value in the io.UnexpectedEOF?

An io.UnexpectedEOF may be an internal error when we're trying to read for example the 5 byte gRPC message header, but receive an io.EOF before reading the required number of bytes.
On re-checking, an io.UnexpectedEOF could be due to different status codes. For example when a incorrectly structured HTTP/2 frame is read by the framer, it would result in http2Client.closeStream() being called with an Unavailable gRPC status.

Or can we use the stream's status (if there is one, which I think there always should be) in this case?

This seems like the safer change to make. I'm not a 100% sure if there will always be an RPC status available when we encounter and io.UnexpectedEOF, I couldn't think of a case when the stream's status is not available. I've checked for a non-nil status in the fix, keeping the existing behaviour in case of a nil status.