Simplify error messages using the default error text

E.g. use "authentication required" instead of "unauthorized: authentication required". See the tests for more examples. Signed-off-by: Miloslav Trmač <[email protected]>
containers · Jun 17, 2022 · c1be731 · c1be731
1 parent b887aec
commit c1be731
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 12 deletions.
diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go
@@ -485,8 +485,8 @@ func successStatus(status int) bool {
 
 // isManifestInvalidError returns true iff err from registryHTTPResponseToError is a “manifest invalid” error.
 func isManifestInvalidError(err error) bool {
-	ec, ok := err.(errcode.ErrorCoder)
-	if !ok {
+	var ec errcode.ErrorCoder
+	if ok := errors.As(err, &ec); !ok {
 		return false
 	}
 

diff --git a/docker/errors.go b/docker/errors.go
@@ -78,13 +78,21 @@ func registryHTTPResponseToError(res *http.Response) error {
 		}
 		err = errs[0]
 	}
-	if e, ok := err.(*client.UnexpectedHTTPResponseError); ok {
+	switch e := err.(type) {
+	case *client.UnexpectedHTTPResponseError:
 		response := string(e.Response)
 		if len(response) > 50 {
 			response = response[:50] + "..."
 		}
 		// %.0w makes e visible to error.Unwrap() without including any text
 		err = fmt.Errorf("StatusCode: %d, %s%.0w", e.StatusCode, response, e)
+	case errcode.Error:
+		// e.Error() is fmt.Sprintf("%s: %s", e.Code.Error(), e.Message, which is usually
+		// rather redundant. So reword it without using e.Code.Error() if e.Message is the default.
+		if e.Message == e.Code.Message() {
+			// %.0w makes e visible to error.Unwrap() without including any text
+			err = fmt.Errorf("%s%.0w", e.Message, e)
+		}
 	}
 	return err
 }
diff --git a/docker/errors_test.go b/docker/errors_test.go
@@ -18,6 +18,7 @@ import (
 // they can change at any time for any reason.
 func TestRegistryHTTPResponseToError(t *testing.T) {
 	var unwrappedUnexpectedHTTPResponseError *client.UnexpectedHTTPResponseError
+	var unwrappedErrcodeError errcode.Error
 	for _, c := range []struct {
 		name              string
 		response          string
@@ -50,9 +51,9 @@ func TestRegistryHTTPResponseToError(t *testing.T) {
 				"Header1: Value1\r\n" +
 				"\r\n" +
 				"<html><body>JSON? What JSON?</body></html>\r\n",
-			errorString:       "unauthorized: authentication required",
-			errorType:         errcode.Error{},
-			unwrappedErrorPtr: nil,
+			errorString:       "authentication required",
+			errorType:         nil,
+			unwrappedErrorPtr: &unwrappedErrcodeError,
 		},
 		{ // docker.io when an image is not found
 			name: "GET https://registry-1.docker.io/v2/library/this-does-not-exist/manifests/latest",
@@ -66,9 +67,9 @@ func TestRegistryHTTPResponseToError(t *testing.T) {
 				"Www-Authenticate: Bearer realm=\"https://auth.docker.io/token\",service=\"registry.docker.io\",scope=\"repository:library/this-does-not-exist:pull\",error=\"insufficient_scope\"\r\n" +
 				"\r\n" +
 				"{\"errors\":[{\"code\":\"UNAUTHORIZED\",\"message\":\"authentication required\",\"detail\":[{\"Type\":\"repository\",\"Class\":\"\",\"Name\":\"library/this-does-not-exist\",\"Action\":\"pull\"}]}]}\n",
-			errorString:       "denied: requested access to the resource is denied",
-			errorType:         errcode.Error{},
-			unwrappedErrorPtr: nil,
+			errorString:       "requested access to the resource is denied",
+			errorType:         nil,
+			unwrappedErrorPtr: &unwrappedErrcodeError,
 		},
 		{ // docker.io when a tag is not found
 			name: "GET https://registry-1.docker.io/v2/library/busybox/manifests/this-does-not-exist",
@@ -83,9 +84,9 @@ func TestRegistryHTTPResponseToError(t *testing.T) {
 				"Strict-Transport-Security: max-age=31536000\r\n" +
 				"\r\n" +
 				"{\"errors\":[{\"code\":\"MANIFEST_UNKNOWN\",\"message\":\"manifest unknown\",\"detail\":{\"Tag\":\"this-does-not-exist\"}}]}\n",
-			errorString:       "manifest unknown: manifest unknown",
-			errorType:         errcode.Error{},
-			unwrappedErrorPtr: nil,
+			errorString:       "manifest unknown",
+			errorType:         nil,
+			unwrappedErrorPtr: &unwrappedErrcodeError,
 		},
 		{ // public.ecr.aws does not implement tag list
 			name: "GET https://public.ecr.aws/v2/nginx/nginx/tags/list",