Decode errors returned to clients by the Watch interface

pkg/client/request.go

@@ -449,11 +449,10 @@ func (r *Request) Watch() (watch.Interface, error) {
 		return nil, err
 	}
 	if resp.StatusCode != http.StatusOK {
-		var body []byte
+		if _, _, err := r.transformResponse(resp, req, nil); err != nil {
-		if resp.Body != nil {
+			return nil, err
-			body, _ = ioutil.ReadAll(resp.Body)
 		}
-		return nil, fmt.Errorf("for request '%+v', got status: %v\nbody: %v", req.URL, resp.StatusCode, string(body))
+		return nil, fmt.Errorf("for request '%+v', got status: %v", req.URL, resp.StatusCode)
 	}
 	return watch.NewStreamWatcher(watchjson.NewDecoder(resp.Body, r.codec)), nil
 }
@@ -604,12 +603,18 @@ func (r *Request) Do() Result {
 	if err != nil {
 		return Result{err: err}
 	}
-	respBody, created, err := r.transformResponse(body, r.resp, r.req)
+	respBody, created, err := r.transformResponse(r.resp, r.req, body)
 	return Result{respBody, created, err, r.codec}
 }
 
-// transformResponse converts an API response into a structured API object.
+// transformResponse converts an API response into a structured API object. If body is nil, the response
-func (r *Request) transformResponse(body []byte, resp *http.Response, req *http.Request) ([]byte, bool, error) {
+// body will be read to try and gather more response data.
+func (r *Request) transformResponse(resp *http.Response, req *http.Request, body []byte) ([]byte, bool, error) {
+	if body == nil && resp.Body != nil {
+		if data, err := ioutil.ReadAll(resp.Body); err == nil {
+			body = data
+		}
+	}
 	// Did the server give us a status response?
 	isStatusResponse := false
 	var status api.Status
@@ -622,43 +627,7 @@ func (r *Request) transformResponse(body []byte, resp *http.Response, req *http.
 		// no-op, we've been upgraded
 	case resp.StatusCode < http.StatusOK || resp.StatusCode > http.StatusPartialContent:
 		if !isStatusResponse {
-			var err error = &UnexpectedStatusError{
+			return nil, false, r.transformUnstructuredResponseError(resp, req, body)
-				Request:  req,
-				Response: resp,
-				Body:     string(body),
-			}
-			message := "unknown"
-			if isTextResponse(resp) {
-				message = strings.TrimSpace(string(body))
-			}
-			// TODO: handle other error classes we know about
-			switch resp.StatusCode {
-			case http.StatusConflict:
-				if req.Method == "POST" {
-					err = errors.NewAlreadyExists(r.resource, r.resourceName)
-				} else {
-					err = errors.NewConflict(r.resource, r.resourceName, err)
-				}
-			case http.StatusNotFound:
-				err = errors.NewNotFound(r.resource, r.resourceName)
-			case http.StatusBadRequest:
-				err = errors.NewBadRequest(message)
-			case http.StatusUnauthorized:
-				err = errors.NewUnauthorized(message)
-			case http.StatusForbidden:
-				err = errors.NewForbidden(r.resource, r.resourceName, err)
-			case errors.StatusUnprocessableEntity:
-				err = errors.NewInvalid(r.resource, r.resourceName, nil)
-			case errors.StatusServerTimeout:
-				retryAfter, _ := retryAfter(resp)
-				err = errors.NewServerTimeout(r.resource, r.verb, retryAfter)
-			case errors.StatusTooManyRequests:
-				retryAfter, _ := retryAfter(resp)
-				err = errors.NewServerTimeout(r.resource, r.verb, retryAfter)
-			case http.StatusInternalServerError:
-				err = errors.NewInternalError(fmt.Errorf(message))
-			}
-			return nil, false, err
 		}
 		return nil, false, errors.FromObject(&status)
 	}
@@ -675,6 +644,52 @@ func (r *Request) transformResponse(body []byte, resp *http.Response, req *http.
 	return body, created, nil
 }
 
+// transformUnstructuredResponseError handles an error from the server that is not in a structured form.
+func (r *Request) transformUnstructuredResponseError(resp *http.Response, req *http.Request, body []byte) error {
+	if body == nil && resp.Body != nil {
+		if data, err := ioutil.ReadAll(resp.Body); err == nil {
+			body = data
+		}
+	}
+	var err error = &UnexpectedStatusError{
+		Request:  req,
+		Response: resp,
+		Body:     string(body),
+	}
+	message := "unknown"
+	if isTextResponse(resp) {
+		message = strings.TrimSpace(string(body))
+	}
+	// TODO: handle other error classes we know about
+	switch resp.StatusCode {
+	case http.StatusConflict:
+		if req.Method == "POST" {
+			err = errors.NewAlreadyExists(r.resource, r.resourceName)
+		} else {
+			err = errors.NewConflict(r.resource, r.resourceName, err)
+		}
+	case http.StatusNotFound:
+		err = errors.NewNotFound(r.resource, r.resourceName)
+	case http.StatusBadRequest:
+		err = errors.NewBadRequest(message)
+	case http.StatusUnauthorized:
+		err = errors.NewUnauthorized(message)
+	case http.StatusForbidden:
+		err = errors.NewForbidden(r.resource, r.resourceName, err)
+	case errors.StatusUnprocessableEntity:
+		err = errors.NewInvalid(r.resource, r.resourceName, nil)
+	case errors.StatusServerTimeout:
+		retryAfter, _ := retryAfter(resp)
+		err = errors.NewServerTimeout(r.resource, r.verb, retryAfter)
+	case errors.StatusTooManyRequests:
+		retryAfter, _ := retryAfter(resp)
+		err = errors.NewServerTimeout(r.resource, r.verb, retryAfter)
+	case http.StatusInternalServerError:
+		err = errors.NewInternalError(fmt.Errorf(message))
+	}
+	return err
+}
+
 // isTextResponse returns true if the response appears to be a textual media type.
 func isTextResponse(resp *http.Response) bool {
 	contentType := resp.Header.Get("Content-Type")

pkg/client/request_test.go

@@ -267,7 +267,7 @@ func TestTransformResponse(t *testing.T) {
 		if err != nil {
 			t.Errorf("failed to read body of response: %v", err)
 		}
-		response, created, err := r.transformResponse(body, test.Response, &http.Request{})
+		response, created, err := r.transformResponse(test.Response, &http.Request{}, body)
 		hasErr := err != nil
 		if hasErr != test.Error {
 			t.Errorf("%d: unexpected error: %t %v", i, test.Error, err)
@@ -357,7 +357,7 @@ func TestTransformUnstructuredError(t *testing.T) {
 		if err != nil {
 			t.Errorf("failed to read body: %v", err)
 		}
-		_, _, err = r.transformResponse(body, testCase.Res, testCase.Req)
+		_, _, err = r.transformResponse(testCase.Res, testCase.Req, body)
 		if !testCase.ErrFn(err) {
 			t.Errorf("unexpected error: %v", err)
 			continue
@@ -381,6 +381,7 @@ func TestRequestWatch(t *testing.T) {
 	testCases := []struct {
 		Request *Request
 		Err     bool
+		ErrFn   func(error) bool
 		Empty   bool
 	}{
 		{
@@ -402,12 +403,48 @@ func TestRequestWatch(t *testing.T) {
 		},
 		{
 			Request: &Request{
+				codec: testapi.Codec(),
 				client: clientFunc(func(req *http.Request) (*http.Response, error) {
 					return &http.Response{StatusCode: http.StatusForbidden}, nil
 				}),
 				baseURL: &url.URL{},
 			},
 			Err: true,
+			ErrFn: func(err error) bool {
+				return apierrors.IsForbidden(err)
+			},
+		},
+		{
+			Request: &Request{
+				codec: testapi.Codec(),
+				client: clientFunc(func(req *http.Request) (*http.Response, error) {
+					return &http.Response{StatusCode: http.StatusUnauthorized}, nil
+				}),
+				baseURL: &url.URL{},
+			},
+			Err: true,
+			ErrFn: func(err error) bool {
+				return apierrors.IsUnauthorized(err)
+			},
+		},
+		{
+			Request: &Request{
+				codec: testapi.Codec(),
+				client: clientFunc(func(req *http.Request) (*http.Response, error) {
+					return &http.Response{
+						StatusCode: http.StatusUnauthorized,
+						Body: ioutil.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(testapi.Codec(), &api.Status{
+							Status: api.StatusFailure,
+							Reason: api.StatusReasonUnauthorized,
+						})))),
+					}, nil
+				}),
+				baseURL: &url.URL{},
+			},
+			Err: true,
+			ErrFn: func(err error) bool {
+				return apierrors.IsUnauthorized(err)
+			},
 		},
 		{
 			Request: &Request{
@@ -453,6 +490,9 @@ func TestRequestWatch(t *testing.T) {
 			t.Errorf("%d: expected %t, got %t: %v", i, testCase.Err, hasErr, err)
 			continue
 		}
+		if testCase.ErrFn != nil && !testCase.ErrFn(err) {
+			t.Errorf("%d: error not valid: %v", i, err)
+		}
 		if hasErr && watch != nil {
 			t.Errorf("%d: watch should be nil when error is returned", i)
 			continue