From f427ee3edede981524c2ffb57fd2d8981f8cf8b4 Mon Sep 17 00:00:00 2001
From: Matheus Pimenta <matheuscscp@gmail.com>
Date: Tue, 20 Sep 2022 22:15:09 +0100
Subject: [PATCH] feat(internal/gensupport): wrap retry failures with context
 and prev error (#1684)

Previously, if a call to sendAndRetry ran with retries until the context deadline, the error returned would be the final error from the service rather than the context expiration error. This change uses error wrapping to wrap the error from the service, while also preserving the context error in the message and with errors.Is.

Fixes #1685
---
 internal/gensupport/send.go | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/internal/gensupport/send.go b/internal/gensupport/send.go
index 70a8e01c1b3..dd24139b366 100644
--- a/internal/gensupport/send.go
+++ b/internal/gensupport/send.go
@@ -17,6 +17,27 @@ import (
 	"github.com/googleapis/gax-go/v2"
 )
 
+// Use this error type to return an error which allows introspection of both
+// the context error and the error from the service.
+type wrappedCallErr struct {
+	ctxErr     error
+	wrappedErr error
+}
+
+func (e wrappedCallErr) Error() string {
+	return fmt.Sprintf("retry failed with %v; last error: %v", e.ctxErr, e.wrappedErr)
+}
+
+func (e wrappedCallErr) Unwrap() error {
+	return e.wrappedErr
+}
+
+// Is allows errors.Is to match the error from the call as well as context
+// sentinel errors.
+func (e wrappedCallErr) Is(target error) bool {
+	return errors.Is(e.ctxErr, target) || errors.Is(e.wrappedErr, target)
+}
+
 // SendRequest sends a single HTTP request using the given client.
 // If ctx is non-nil, it calls all hooks, then sends the request with
 // req.WithContext, then calls any functions returned by the hooks in
@@ -96,12 +117,12 @@ func sendAndRetry(ctx context.Context, client *http.Client, req *http.Request, r
 	for {
 		select {
 		case <-ctx.Done():
-			// If we got an error, and the context has been canceled,
-			// the context's error is probably more useful.
-			if err == nil {
-				err = ctx.Err()
+			// If we got an error and the context has been canceled, return an error acknowledging
+			// both the context cancelation and the service error.
+			if err != nil {
+				return resp, wrappedCallErr{ctx.Err(), err}
 			}
-			return resp, err
+			return resp, ctx.Err()
 		case <-time.After(pause):
 		}
 
@@ -110,10 +131,10 @@ func sendAndRetry(ctx context.Context, client *http.Client, req *http.Request, r
 			// select is satisfied at the same time, Go will choose one arbitrarily.
 			// That can cause an operation to go through even if the context was
 			// canceled before.
-			if err == nil {
-				err = ctx.Err()
+			if err != nil {
+				return resp, wrappedCallErr{ctx.Err(), err}
 			}
-			return resp, err
+			return resp, ctx.Err()
 		}
 		invocationHeader := fmt.Sprintf("gccl-invocation-id/%s gccl-attempt-count/%d", invocationID, attempts)
 		xGoogHeader := strings.Join([]string{invocationHeader, baseXGoogHeader}, " ")