From c1be73122257712ee4e21803dc00cc19b3c41e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 12 Aug 2021 22:51:45 +0200 Subject: [PATCH] Simplify error messages using the default error text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E.g. use "authentication required" instead of "unauthorized: authentication required". See the tests for more examples. Signed-off-by: Miloslav Trmač --- docker/docker_image_dest.go | 4 ++-- docker/errors.go | 10 +++++++++- docker/errors_test.go | 19 ++++++++++--------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 8fe39b84d2..d02a11554a 100644 --- 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 index 750ef491ad..7680f309a1 100644 --- 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 index 199b07eb1d..cb6789cecb 100644 --- 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" + "JSON? What JSON?\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",