Merge pull request #1531 from aaronlehmann/empty-errors-slice

Don't return empty errcode.Errors slices

docs/client/errors.go

@@ -2,6 +2,7 @@ package client
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -10,6 +11,10 @@ import (
 	"github.com/docker/distribution/registry/api/errcode"
 )
 
+// ErrNoErrorsInBody is returned when a HTTP response body parses to an empty
+// errcode.Errors slice.
+var ErrNoErrorsInBody = errors.New("no error details found in HTTP response body")
+
 // UnexpectedHTTPStatusError is returned when an unexpected HTTP status is
 // returned when making a registry api call.
 type UnexpectedHTTPStatusError struct {
@@ -17,18 +22,19 @@ type UnexpectedHTTPStatusError struct {
 }
 
 func (e *UnexpectedHTTPStatusError) Error() string {
-	return fmt.Sprintf("Received unexpected HTTP status: %s", e.Status)
+	return fmt.Sprintf("received unexpected HTTP status: %s", e.Status)
 }
 
 // UnexpectedHTTPResponseError is returned when an expected HTTP status code
 // is returned, but the content was unexpected and failed to be parsed.
 type UnexpectedHTTPResponseError struct {
-	ParseErr error
+	ParseErr   error
-	Response []byte
+	StatusCode int
+	Response   []byte
 }
 
 func (e *UnexpectedHTTPResponseError) Error() string {
-	return fmt.Sprintf("Error parsing HTTP response: %s: %q", e.ParseErr.Error(), string(e.Response))
+	return fmt.Sprintf("error parsing HTTP %d response body: %s: %q", e.StatusCode, e.ParseErr.Error(), string(e.Response))
 }
 
 func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
@@ -53,10 +59,22 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
 
 	if err := json.Unmarshal(body, &errors); err != nil {
 		return &UnexpectedHTTPResponseError{
-			ParseErr: err,
+			ParseErr:   err,
-			Response: body,
+			StatusCode: statusCode,
+			Response:   body,
 		}
 	}
+
+	if len(errors) == 0 {
+		// If there was no error specified in the body, return
+		// UnexpectedHTTPResponseError.
+		return &UnexpectedHTTPResponseError{
+			ParseErr:   ErrNoErrorsInBody,
+			StatusCode: statusCode,
+			Response:   body,
+		}
+	}
+
 	return errors
 }
 

docs/client/errors_test.go

@@ -59,6 +59,21 @@ func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) {
 	}
 }
 
+func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) {
+	json := `{"randomkey": "randomvalue"}`
+	response := &http.Response{
+		Status:     "404 Not Found",
+		StatusCode: 404,
+		Body:       nopCloser{bytes.NewBufferString(json)},
+	}
+	err := HandleErrorResponse(response)
+
+	expectedMsg := `error parsing HTTP 404 response body: no error details found in HTTP response body: "{\"randomkey\": \"randomvalue\"}"`
+	if !strings.Contains(err.Error(), expectedMsg) {
+		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
+	}
+}
+
 func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
 	json := "{invalid json}"
 	response := &http.Response{
@@ -68,7 +83,7 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
 	}
 	err := HandleErrorResponse(response)
 
-	expectedMsg := "Error parsing HTTP response: invalid character 'i' looking for beginning of object key string: \"{invalid json}\""
+	expectedMsg := "error parsing HTTP 404 response body: invalid character 'i' looking for beginning of object key string: \"{invalid json}\""
 	if !strings.Contains(err.Error(), expectedMsg) {
 		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 	}
@@ -82,7 +97,7 @@ func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) {
 	}
 	err := HandleErrorResponse(response)
 
-	expectedMsg := "Received unexpected HTTP status: 501 Not Implemented"
+	expectedMsg := "received unexpected HTTP status: 501 Not Implemented"
 	if !strings.Contains(err.Error(), expectedMsg) {
 		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 	}