Move an error helper to registry/session.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

pkg/httputils/httputils.go

@@ -3,8 +3,6 @@ package httputils
 import (
 	"fmt"
 	"net/http"
-
-	"github.com/docker/docker/pkg/jsonmessage"
 )
 
 // Download requests a given URL and returns an io.Reader.
@@ -17,11 +15,3 @@ func Download(url string) (resp *http.Response, err error) {
 	}
 	return resp, nil
 }
-
-// NewHTTPRequestError returns a JSON response error.
-func NewHTTPRequestError(msg string, res *http.Response) error {
-	return &jsonmessage.JSONError{
-		Message: msg,
-		Code:    res.StatusCode,
-	}
-}

pkg/httputils/httputils_test.go

@@ -5,8 +5,11 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
+
+	"github.com/docker/docker/pkg/testutil"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDownload(t *testing.T) {
@@ -22,10 +25,8 @@ func TestDownload(t *testing.T) {
 
 	actual, err := ioutil.ReadAll(response.Body)
 	response.Body.Close()
-
-	if err != nil || string(actual) != expected {
-		t.Fatalf("Expected the response %q, got err:%q, actual:%q", expected, err, string(actual))
-	}
+	require.NoError(t, err)
+	assert.Equal(t, expected, string(actual))
 }
 
 func TestDownload400Errors(t *testing.T) {
@@ -36,29 +37,11 @@ func TestDownload400Errors(t *testing.T) {
 	}))
 	defer ts.Close()
 	// Expected status code = 403
-	if _, err := Download(ts.URL); err == nil || err.Error() != expectedError {
-		t.Fatalf("Expected the error %q, got %q", expectedError, err)
-	}
+	_, err := Download(ts.URL)
+	assert.EqualError(t, err, expectedError)
 }
 
 func TestDownloadOtherErrors(t *testing.T) {
-	if _, err := Download("I'm not an url.."); err == nil || !strings.Contains(err.Error(), "unsupported protocol scheme") {
-		t.Fatalf("Expected an error with 'unsupported protocol scheme', got %q", err)
-	}
-}
-
-func TestNewHTTPRequestError(t *testing.T) {
-	errorMessage := "Some error message"
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// 403
-		http.Error(w, errorMessage, http.StatusForbidden)
-	}))
-	defer ts.Close()
-	httpResponse, err := http.Get(ts.URL)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := NewHTTPRequestError(errorMessage, httpResponse); err.Error() != errorMessage {
-		t.Fatalf("Expected err to be %q, got %q", errorMessage, err)
-	}
+	_, err := Download("I'm not an url..")
+	testutil.ErrorContains(t, err, "unsupported protocol scheme")
 }

registry/session.go

@@ -23,8 +23,8 @@ import (
 	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/docker/api/types"
 	registrytypes "github.com/docker/docker/api/types/registry"
-	"github.com/docker/docker/pkg/httputils"
 	"github.com/docker/docker/pkg/ioutils"
+	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/tarsum"
 	"github.com/docker/docker/registry/resumable"
@@ -227,7 +227,7 @@ func (r *Session) GetRemoteHistory(imgID, registry string) ([]string, error) {
 		if res.StatusCode == 401 {
 			return nil, errcode.ErrorCodeUnauthorized.WithArgs()
 		}
-		return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Server error: %d trying to fetch remote history for %s", res.StatusCode, imgID), res)
+		return nil, newJSONError(fmt.Sprintf("Server error: %d trying to fetch remote history for %s", res.StatusCode, imgID), res)
 	}
 
 	var history []string
@@ -247,7 +247,7 @@ func (r *Session) LookupRemoteImage(imgID, registry string) error {
 	}
 	res.Body.Close()
 	if res.StatusCode != 200 {
-		return httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
+		return newJSONError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
 	}
 	return nil
 }
@@ -260,7 +260,7 @@ func (r *Session) GetRemoteImageJSON(imgID, registry string) ([]byte, int64, err
 	}
 	defer res.Body.Close()
 	if res.StatusCode != 200 {
-		return nil, -1, httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
+		return nil, -1, newJSONError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
 	}
 	// if the size header is not present, then set it to '-1'
 	imageSize := int64(-1)
@@ -445,13 +445,13 @@ func (r *Session) GetRepositoryData(name reference.Named) (*RepositoryData, erro
 	// TODO: Right now we're ignoring checksums in the response body.
 	// In the future, we need to use them to check image validity.
 	if res.StatusCode == 404 {
-		return nil, httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code: %d", res.StatusCode), res)
+		return nil, newJSONError(fmt.Sprintf("HTTP code: %d", res.StatusCode), res)
 	} else if res.StatusCode != 200 {
 		errBody, err := ioutil.ReadAll(res.Body)
 		if err != nil {
 			logrus.Debugf("Error reading response body: %s", err)
 		}
-		return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Error: Status %d trying to pull repository %s: %q", res.StatusCode, reference.Path(name), errBody), res)
+		return nil, newJSONError(fmt.Sprintf("Error: Status %d trying to pull repository %s: %q", res.StatusCode, reference.Path(name), errBody), res)
 	}
 
 	var endpoints []string
@@ -538,12 +538,12 @@ func (r *Session) PushImageJSONRegistry(imgData *ImgData, jsonRaw []byte, regist
 	}
 	defer res.Body.Close()
 	if res.StatusCode == 401 && strings.HasPrefix(registry, "http://") {
-		return httputils.NewHTTPRequestError("HTTP code 401, Docker will not send auth headers over HTTP.", res)
+		return newJSONError("HTTP code 401, Docker will not send auth headers over HTTP.", res)
 	}
 	if res.StatusCode != 200 {
 		errBody, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
+			return newJSONError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
 		}
 		var jsonBody map[string]string
 		if err := json.Unmarshal(errBody, &jsonBody); err != nil {
@@ -551,7 +551,7 @@ func (r *Session) PushImageJSONRegistry(imgData *ImgData, jsonRaw []byte, regist
 		} else if jsonBody["error"] == "Image already exists" {
 			return ErrAlreadyExists
 		}
-		return httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d while uploading metadata: %q", res.StatusCode, errBody), res)
+		return newJSONError(fmt.Sprintf("HTTP code %d while uploading metadata: %q", res.StatusCode, errBody), res)
 	}
 	return nil
 }
@@ -592,9 +592,9 @@ func (r *Session) PushImageLayerRegistry(imgID string, layer io.Reader, registry
 	if res.StatusCode != 200 {
 		errBody, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return "", "", httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
+			return "", "", newJSONError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
 		}
-		return "", "", httputils.NewHTTPRequestError(fmt.Sprintf("Received HTTP code %d while uploading layer: %q", res.StatusCode, errBody), res)
+		return "", "", newJSONError(fmt.Sprintf("Received HTTP code %d while uploading layer: %q", res.StatusCode, errBody), res)
 	}
 
 	checksumPayload = "sha256:" + hex.EncodeToString(h.Sum(nil))
@@ -620,7 +620,7 @@ func (r *Session) PushRegistryTag(remote reference.Named, revision, tag, registr
 	}
 	res.Body.Close()
 	if res.StatusCode != 200 && res.StatusCode != 201 {
-		return httputils.NewHTTPRequestError(fmt.Sprintf("Internal server error: %d trying to push tag %s on %s", res.StatusCode, tag, reference.Path(remote)), res)
+		return newJSONError(fmt.Sprintf("Internal server error: %d trying to push tag %s on %s", res.StatusCode, tag, reference.Path(remote)), res)
 	}
 	return nil
 }
@@ -684,7 +684,7 @@ func (r *Session) PushImageJSONIndex(remote reference.Named, imgList []*ImgData,
 			if err != nil {
 				logrus.Debugf("Error reading response body: %s", err)
 			}
-			return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Error: Status %d trying to push repository %s: %q", res.StatusCode, reference.Path(remote), errBody), res)
+			return nil, newJSONError(fmt.Sprintf("Error: Status %d trying to push repository %s: %q", res.StatusCode, reference.Path(remote), errBody), res)
 		}
 		tokens = res.Header["X-Docker-Token"]
 		logrus.Debugf("Auth token: %v", tokens)
@@ -702,7 +702,7 @@ func (r *Session) PushImageJSONIndex(remote reference.Named, imgList []*ImgData,
 			if err != nil {
 				logrus.Debugf("Error reading response body: %s", err)
 			}
-			return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Error: Status %d trying to push checksums %s: %q", res.StatusCode, reference.Path(remote), errBody), res)
+			return nil, newJSONError(fmt.Sprintf("Error: Status %d trying to push checksums %s: %q", res.StatusCode, reference.Path(remote), errBody), res)
 		}
 	}
 
@@ -751,7 +751,7 @@ func (r *Session) SearchRepositories(term string, limit int) (*registrytypes.Sea
 	}
 	defer res.Body.Close()
 	if res.StatusCode != 200 {
-		return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Unexpected status code %d", res.StatusCode), res)
+		return nil, newJSONError(fmt.Sprintf("Unexpected status code %d", res.StatusCode), res)
 	}
 	result := new(registrytypes.SearchResults)
 	return result, json.NewDecoder(res.Body).Decode(result)
@@ -782,3 +782,10 @@ func isTimeout(err error) bool {
 	t, ok := e.(timeout)
 	return ok && t.Timeout()
 }
+
+func newJSONError(msg string, res *http.Response) error {
+	return &jsonmessage.JSONError{
+		Message: msg,
+		Code:    res.StatusCode,
+	}
+}