chore(ux): show registry error and hint for Dockerhub (oras-project#1222

)

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Feynman Zhou <feynmanzhou@microsoft.com>

cmd/oras/internal/errors/errors.go

@@ -17,11 +17,16 @@ package errors
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/spf13/cobra"
 	"oras.land/oras-go/v2/registry"
+	"oras.land/oras-go/v2/registry/remote/errcode"
 )
 
+// RegistryErrorPrefix is the commandline prefix for errors from registry.
+const RegistryErrorPrefix = "Error response from registry:"
+
 // Error is the error type for CLI error messaging.
 type Error struct {
 	Err            error
@@ -60,6 +65,60 @@ func CheckArgs(checker func(args []string) (bool, string), Usage string) cobra.P
 	}
 }
 
+// Modifier modifies the error during cmd execution.
+type Modifier interface {
+	Modify(cmd *cobra.Command, err error) (modifiedErr error, modified bool)
+}
+
+// Command returns an error-handled cobra command.
+func Command(cmd *cobra.Command, handler Modifier) *cobra.Command {
+	runE := cmd.RunE
+	cmd.RunE = func(cmd *cobra.Command, args []string) error {
+		err := runE(cmd, args)
+		if err != nil {
+			err, _ = handler.Modify(cmd, err)
+			return err
+		}
+		return nil
+	}
+	return cmd
+}
+
+// Trim tries to trim toTrim from err.
+func Trim(err error, toTrim error) error {
+	var inner error
+	if errResp, ok := toTrim.(*errcode.ErrorResponse); ok {
+		if len(errResp.Errors) == 0 {
+			return fmt.Errorf("recognizable error message not found: %w", toTrim)
+		}
+		inner = errResp.Errors
+	} else {
+		return err
+	}
+
+	if rewrapped := reWrap(err, toTrim, inner); rewrapped != nil {
+		return rewrapped
+	}
+	return inner
+}
+
+// reWrap re-wraps errA to errC and trims out errB, returns nil if scrub fails.
+// +---------- errA ----------+
+// |         +---- errB ----+ |      +---- errA ----+
+// |         |    errC      | |  =>  |     errC     |
+// |         +--------------+ |      +--------------+
+// +--------------------------+
+func reWrap(errA, errB, errC error) error {
+	// TODO: trim dedicated error type when
+	// https://github.com/oras-project/oras-go/issues/677 is done
+	contentA := errA.Error()
+	contentB := errB.Error()
+	if idx := strings.Index(contentA, contentB); idx > 0 {
+		return fmt.Errorf("%s%w", contentA[:idx], errC)
+	}
+	return nil
+}
+
 // NewErrEmptyTagOrDigest creates a new error based on the reference string.
 func NewErrEmptyTagOrDigest(ref registry.Reference) error {
 	return NewErrEmptyTagOrDigestStr(ref.String())

cmd/oras/internal/option/remote.go

@@ -30,19 +30,23 @@ import (
 
 	credentials "github.com/oras-project/oras-credentials-go"
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"oras.land/oras-go/v2/errdef"
 	"oras.land/oras-go/v2/registry/remote"
 	"oras.land/oras-go/v2/registry/remote/auth"
+	"oras.land/oras-go/v2/registry/remote/errcode"
 	"oras.land/oras-go/v2/registry/remote/retry"
+	oerrors "oras.land/oras/cmd/oras/internal/errors"
 	"oras.land/oras/internal/credential"
 	"oras.land/oras/internal/crypto"
 	onet "oras.land/oras/internal/net"
 	"oras.land/oras/internal/trace"
 	"oras.land/oras/internal/version"
 )
 
-// Remote options struct.
+// Remote options struct contains flags and arguments specifying one registry.
+// Remote implements oerrors.Handler and interface.
 type Remote struct {
 	DistributionSpec
 	CACertFilePath    string
@@ -322,3 +326,15 @@ func (opts *Remote) isPlainHttp(registry string) bool {
 	}
 	return plainHTTP
 }
+
+// Modify modifies error during cmd execution.
+func (opts *Remote) Modify(cmd *cobra.Command, err error) (error, bool) {
+	var errResp *errcode.ErrorResponse
+	if errors.As(err, &errResp) {
+		cmd.SetErrPrefix(oerrors.RegistryErrorPrefix)
+		return &oerrors.Error{
+			Err: oerrors.Trim(err, errResp),
+		}, true
+	}
+	return err, false
+}

cmd/oras/internal/option/target.go

@@ -21,17 +21,21 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"net/http"
 	"os"
 	"strings"
 	"sync"
 
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"oras.land/oras-go/v2"
 	"oras.land/oras-go/v2/content"
 	"oras.land/oras-go/v2/content/oci"
+	"oras.land/oras-go/v2/errdef"
 	"oras.land/oras-go/v2/registry"
 	"oras.land/oras-go/v2/registry/remote"
+	"oras.land/oras-go/v2/registry/remote/errcode"
 	oerrors "oras.land/oras/cmd/oras/internal/errors"
 	"oras.land/oras/cmd/oras/internal/fileref"
 )
@@ -43,6 +47,7 @@ const (
 
 // Target struct contains flags and arguments specifying one registry or image
 // layout.
+// Target implements oerrors.Handler interface.
 type Target struct {
 	Remote
 	RawReference string
@@ -239,8 +244,54 @@ func (opts *Target) EnsureReferenceNotEmpty() error {
 	return nil
 }
 
+// Modify handles error during cmd execution.
+func (opts *Target) Modify(cmd *cobra.Command, err error) (error, bool) {
+	if opts.IsOCILayout {
+		return err, false
+	}
+
+	if errors.Is(err, errdef.ErrNotFound) {
+		cmd.SetErrPrefix(oerrors.RegistryErrorPrefix)
+		return err, true
+	}
+
+	var errResp *errcode.ErrorResponse
+	if errors.As(err, &errResp) {
+		ref := registry.Reference{Registry: opts.RawReference}
+		if errResp.URL.Host != ref.Host() {
+			// raw reference is not registry host
+			var parseErr error
+			ref, parseErr = registry.ParseReference(opts.RawReference)
+			if parseErr != nil {
+				// this should not happen
+				return err, false
+			}
+			if errResp.URL.Host != ref.Host() {
+				// not handle if the error is not from the target
+				return err, false
+			}
+		}
+
+		cmd.SetErrPrefix(oerrors.RegistryErrorPrefix)
+		ret := &oerrors.Error{
+			Err: oerrors.Trim(err, errResp),
+		}
+
+		if ref.Registry == "docker.io" && errResp.StatusCode == http.StatusUnauthorized {
+			if ref.Repository != "" && !strings.Contains(ref.Repository, "/") {
+				// docker.io/xxx -> docker.io/library/xxx
+				ref.Repository = "library/" + ref.Repository
+				ret.Recommendation = fmt.Sprintf("Namespace seems missing. Do you mean `%s %s`?", cmd.CommandPath(), ref)
+			}
+		}
+		return ret, true
+	}
+	return err, false
+}
+
 // BinaryTarget struct contains flags and arguments specifying two registries or
 // image layouts.
+// BinaryTarget implements errors.Handler interface.
 type BinaryTarget struct {
 	From        Target
 	To          Target
@@ -269,3 +320,11 @@ func (opts *BinaryTarget) Parse() error {
 	opts.To.resolveFlag = append(opts.resolveFlag, opts.To.resolveFlag...)
 	return Parse(opts)
 }
+
+// Modify handles error during cmd execution.
+func (opts *BinaryTarget) Modify(cmd *cobra.Command, err error) (error, bool) {
+	if modifiedErr, modified := opts.From.Modify(cmd, err); modified {
+		return modifiedErr, modified
+	}
+	return opts.To.Modify(cmd, err)
+}

cmd/oras/internal/option/target_test.go

@@ -16,7 +16,15 @@ limitations under the License.
 package option
 
 import (
+	"errors"
+	"net/http"
+	"net/url"
+	"reflect"
 	"testing"
+
+	"github.com/spf13/cobra"
+	"oras.land/oras-go/v2/registry/remote/errcode"
+	oerrors "oras.land/oras/cmd/oras/internal/errors"
 )
 
 func TestTarget_Parse_oci(t *testing.T) {
@@ -75,3 +83,159 @@ func Test_parseOCILayoutReference(t *testing.T) {
 		})
 	}
 }
+
+func TestTarget_Modify_ociLayout(t *testing.T) {
+	errClient := errors.New("client error")
+	opts := &Target{}
+	got, modified := opts.Modify(&cobra.Command{}, errClient)
+
+	if modified {
+		t.Errorf("expect error not to be modified but received true")
+	}
+	if got != errClient {
+		t.Errorf("unexpected output from Target.Process() = %v", got)
+	}
+}
+
+func TestTarget_Modify_errInvalidReference(t *testing.T) {
+	errResp := &errcode.ErrorResponse{
+		URL:        &url.URL{Host: "registry-1.docker.io"},
+		StatusCode: http.StatusUnauthorized,
+		Errors: errcode.Errors{
+			errcode.Error{
+				Code:    "000",
+				Message: "mocked message",
+				Detail:  map[string]string{"mocked key": "mocked value"},
+			},
+		},
+	}
+	opts := &Target{
+		RawReference: "invalid-reference",
+	}
+	got, modified := opts.Modify(&cobra.Command{}, errResp)
+
+	if modified {
+		t.Errorf("expect error not to be modified but received true")
+	}
+	if got != errResp {
+		t.Errorf("unexpected output from Target.Process() = %v", got)
+	}
+}
+
+func TestTarget_Modify_errHostNotMatching(t *testing.T) {
+	errResp := &errcode.ErrorResponse{
+		URL:        &url.URL{Host: "registry-1.docker.io"},
+		StatusCode: http.StatusUnauthorized,
+		Errors: errcode.Errors{
+			errcode.Error{
+				Code:    "000",
+				Message: "mocked message",
+				Detail:  map[string]string{"mocked key": "mocked value"},
+			},
+		},
+	}
+
+	opts := &Target{
+		RawReference: "registry-2.docker.io/test:tag",
+	}
+	_, modified := opts.Modify(&cobra.Command{}, errResp)
+	if modified {
+		t.Errorf("expect error not to be modified but received true")
+	}
+}
+
+func TestTarget_Modify_dockerHint(t *testing.T) {
+	type fields struct {
+		Remote       Remote
+		RawReference string
+		Type         string
+		Reference    string
+		Path         string
+		IsOCILayout  bool
+	}
+	errs := errcode.Errors{
+		errcode.Error{
+			Code:    "000",
+			Message: "mocked message",
+			Detail:  map[string]string{"mocked key": "mocked value"},
+		},
+	}
+	tests := []struct {
+		name        string
+		fields      fields
+		err         error
+		modifiedErr *oerrors.Error
+	}{
+		{
+			"namespace already exists",
+			fields{RawReference: "docker.io/library/alpine:latest"},
+			&errcode.ErrorResponse{
+				URL:        &url.URL{Host: "registry-1.docker.io"},
+				StatusCode: http.StatusUnauthorized,
+				Errors:     errs,
+			},
+			&oerrors.Error{Err: errs},
+		},
+		{
+			"no namespace",
+			fields{RawReference: "docker.io"},
+			&errcode.ErrorResponse{
+				URL:        &url.URL{Host: "registry-1.docker.io"},
+				StatusCode: http.StatusUnauthorized,
+				Errors:     errs,
+			},
+			&oerrors.Error{Err: errs},
+		},
+		{
+			"not 401",
+			fields{RawReference: "docker.io"},
+			&errcode.ErrorResponse{
+				URL:        &url.URL{Host: "registry-1.docker.io"},
+				StatusCode: http.StatusConflict,
+				Errors:     errs,
+			},
+			&oerrors.Error{Err: errs},
+		},
+		{
+			"should hint",
+			fields{
+				RawReference: "docker.io/alpine",
+				Path:         "oras test",
+			},
+			&errcode.ErrorResponse{
+				URL:        &url.URL{Host: "registry-1.docker.io"},
+				StatusCode: http.StatusUnauthorized,
+				Errors:     errs,
+			},
+			&oerrors.Error{
+				Err:            errs,
+				Recommendation: "Namespace seems missing. Do you mean ` docker.io/library/alpine`?",
+			},
+		},
+	}
+
+	cmd := &cobra.Command{}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			opts := &Target{
+				Remote:       tt.fields.Remote,
+				RawReference: tt.fields.RawReference,
+				Type:         tt.fields.Type,
+				Reference:    tt.fields.Reference,
+				Path:         tt.fields.Path,
+				IsOCILayout:  tt.fields.IsOCILayout,
+			}
+			got, modified := opts.Modify(cmd, tt.err)
+			gotErr, ok := got.(*oerrors.Error)
+			if !ok {
+				t.Errorf("expecting error to be *oerrors.Error but received %T", got)
+			}
+			if !reflect.DeepEqual(gotErr.Err, tt.modifiedErr.Err) || gotErr.Usage != tt.modifiedErr.Usage || gotErr.Recommendation != tt.modifiedErr.Recommendation {
+				t.Errorf("Target.Modify() error = %v, wantErr %v", gotErr, tt.modifiedErr)
+			}
+			if !modified {
+				t.Errorf("Failed to modify %v", tt.err)
+			}
+		})
+	}
+}