From 005d2dfb6b68bf5a35bfb8db449d3f0084b34d6e Mon Sep 17 00:00:00 2001
From: Antoine Tollenaere <atollena@gmail.com>
Date: Sat, 17 Jun 2023 02:24:13 +0200
Subject: [PATCH] fix(internal/retry): never return nil from GRPCStatus()
 (#8128)

An error's GRPCStatus() method should never returns nil, which maps to a status with Code.OK (https://togithub.com/grpc/grpc-go/blob/642dd63a85275a96d172f446911fd04111e2c74c/internal/status/status.go#L71-L76). Instead it should return an actual error code, such as Status.Unknown.

This addresses https://togithub.com/googleapis/google-cloud-go/issues/8127.
---
 internal/retry.go      |  3 ++-
 internal/retry_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/internal/retry.go b/internal/retry.go
index 2943a6d0b457..290e744b7d36 100644
--- a/internal/retry.go
+++ b/internal/retry.go
@@ -20,6 +20,7 @@ import (
 	"time"
 
 	gax "github.com/googleapis/gax-go/v2"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
@@ -81,5 +82,5 @@ func (e wrappedCallErr) GRPCStatus() *status.Status {
 	if s, ok := status.FromError(e.wrappedErr); ok {
 		return s
 	}
-	return nil
+	return status.New(codes.Unknown, e.Error())
 }
diff --git a/internal/retry_test.go b/internal/retry_test.go
index 771cb26ffca4..1c26dcbcc394 100644
--- a/internal/retry_test.go
+++ b/internal/retry_test.go
@@ -93,3 +93,29 @@ func TestRetryPreserveError(t *testing.T) {
 		t.Errorf("got message %q, want %q", g, w)
 	}
 }
+
+func TestRetryWrapsErrorWithStatusUnknown(t *testing.T) {
+	// When retrying on an error that is not a grpc error, make sure to return
+	// a valid gRPC status.
+	err := retry(context.Background(), gax.Backoff{},
+		func() (bool, error) {
+			return false, errors.New("test error")
+		},
+		func(context.Context, time.Duration) error {
+			return context.DeadlineExceeded
+		})
+	if err == nil {
+		t.Fatalf("unexpectedly got nil error")
+	}
+	wantError := "retry failed with context deadline exceeded; last error: test error"
+	if g, w := err.Error(), wantError; g != w {
+		t.Errorf("got error %q, want %q", g, w)
+	}
+	got, ok := status.FromError(err)
+	if !ok {
+		t.Fatal("expected error to implement a gRPC status")
+	}
+	if g, w := got.Code(), codes.Unknown; g != w {
+		t.Errorf("got code %v, want %v", g, w)
+	}
+}