From 009514ecc1243379530349bd7cc03af46012af92 Mon Sep 17 00:00:00 2001
From: Quan Tian <quan.tian@broadcom.com>
Date: Mon, 22 Jul 2024 17:13:43 +0800
Subject: [PATCH] Enrich the error returned from Request.Watch method

The Error method of the error returned from Request.Watch was "unknown"
even the server returned clear message in the Status struct. It was
because Request.Watch used the Result's err member directly, which is an
unstructured error from the response which the Result object may use if
the caller did not return a structured error.

The patch fixes it by calling the Result's Error method instead, which
returns the structured error when it's present.

It also removes the wrong expectation about events.

Kubernetes-commit: 596c5696c64023808af164284263647d795b0ac2
---
 rest/request.go      |  5 +++--
 rest/request_test.go | 40 +++++++++++++---------------------------
 2 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/rest/request.go b/rest/request.go
index 25f3a059..dfe5e0ad 100644
--- a/rest/request.go
+++ b/rest/request.go
@@ -752,8 +752,9 @@ func (r *Request) Watch(ctx context.Context) (watch.Interface, error) {
 				// the server must have sent us an error in 'err'
 				return true, nil
 			}
-			if result := r.transformResponse(resp, req); result.err != nil {
-				return true, result.err
+			result := r.transformResponse(resp, req)
+			if err := result.Error(); err != nil {
+				return true, err
 			}
 			return true, fmt.Errorf("for request %s, got status: %v", url, resp.StatusCode)
 		}()
diff --git a/rest/request_test.go b/rest/request_test.go
index 747de7cb..38c95c99 100644
--- a/rest/request_test.go
+++ b/rest/request_test.go
@@ -977,7 +977,7 @@ func TestRequestWatch(t *testing.T) {
 			Err:              true,
 		},
 		{
-			name: "server returns forbidden",
+			name: "server returns forbidden with json content",
 			Request: &Request{
 				c: &RESTClient{
 					content: defaultContentConfig(),
@@ -986,41 +986,27 @@ func TestRequestWatch(t *testing.T) {
 			},
 			serverReturns: []responseErr{
 				{response: &http.Response{
+					Header:     http.Header{"Content-Type": []string{"application/json"}},
 					StatusCode: http.StatusForbidden,
-					Body:       io.NopCloser(bytes.NewReader([]byte{})),
+					Body: io.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(scheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), &metav1.Status{
+						Status:  metav1.StatusFailure,
+						Message: "secrets is forbidden",
+						Reason:  metav1.StatusReasonForbidden,
+						Code:    http.StatusForbidden,
+					})))),
 				}, err: nil},
 			},
 			attemptsExpected: 1,
-			Expect: []watch.Event{
-				{
-					Type: watch.Error,
-					Object: &metav1.Status{
-						Status:  "Failure",
-						Code:    500,
-						Reason:  "InternalError",
-						Message: `an error on the server ("unable to decode an event from the watch stream: test error") has prevented the request from succeeding`,
-						Details: &metav1.StatusDetails{
-							Causes: []metav1.StatusCause{
-								{
-									Type:    "UnexpectedServerResponse",
-									Message: "unable to decode an event from the watch stream: test error",
-								},
-								{
-									Type:    "ClientWatchDecoding",
-									Message: "unable to decode an event from the watch stream: test error",
-								},
-							},
-						},
-					},
-				},
-			},
-			Err: true,
+			Err:              true,
 			ErrFn: func(err error) bool {
+				if err.Error() != "secrets is forbidden" {
+					return false
+				}
 				return apierrors.IsForbidden(err)
 			},
 		},
 		{
-			name: "server returns forbidden",
+			name: "server returns forbidden without content",
 			Request: &Request{
 				c: &RESTClient{
 					content: defaultContentConfig(),