pubsub: Receive fails to hide context.Canceled error when using synchronous pull

[mgabeler-lee-6rs]

Client

PubSub

Environment

Various

Go Environment

$ go version
go version go1.16 linux/amd64

$ go env

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/mgl/.cache/go-build"
GOENV="/home/mgl/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/mgl/src/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/mgl/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.16"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.16/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build4089208195=/tmp/go-build -gno-record-gcc-switches"

Code

func foo(client *pubsub.Client) {
	sub := client.Subscription("foo")
	// enabling synchronous receive causes Receive to return a context cancelled error instead of nil
	sub.ReceiveSettings.Synchronous = true
	ctx, cancel := context.WithCancel(context.Background())
	err := sub.Receive(ctx, func(c context.Context, m *pubsub.Message) {
		m.Ack()
		cancel()
	})
	if err != nil {
		panic(err)
	}
}

Expected behavior

The call to sub.Receive should return a nil error when it is terminated through context cancellation.

Actual behavior

The call to sub.Receive returns a version of context.Canceled, wrapped/converted to a gRPC status error, if and only if Synchronous pull is enabled.

Additional context

I've traced this so far as this error handling logic in messageIterator.pullMessages:

google-cloud-go/pubsub/iterator.go

Lines 247 to 254 in 39517ae

	switch {
	case err == context.Canceled:
	return nil, nil
	case err != nil:
	return nil, err
	default:
	return res.ReceivedMessages, nil
	}

In this case, it's trying to replace the context cancellation error with nil, but that cancellation error has already been wrapped into a gRPC one at this point ... which isn't surprising, because it came from a gRPC client call :)

I believe this can be fixed by updating the switch thus:

	switch {
	case err == context.Canceled:
		return nil, nil
	case status.Code(err) == codes.Canceled: // ⇐ add this case
		return nil, nil
	case err != nil:
		return nil, err
	default:
		return res.ReceivedMessages, nil
	}

[hongalex]

Totally agreed, thanks for opening this issue and including your investigations! I opened a PR without your suggested change to fix this.