Merge pull request #11599 from wy65701436/revise-base-error

update internal error output format

src/lib/errors/const.go

@@ -27,40 +27,50 @@ const (
 
 // NotFoundError is error for the case of object not found
 func NotFoundError(err error) *Error {
-	return New(err).WithCode(NotFoundCode).WithMessage("resource not found")
+	return New("resource not found").WithCode(NotFoundCode).WithCause(err)
 }
 
 // ConflictError is error for the case of object conflict
 func ConflictError(err error) *Error {
-	return New(err).WithCode(ConflictCode).WithMessage("resource conflict")
+	return New("resource conflict").WithCode(ConflictCode).WithCause(err)
 }
 
 // DeniedError is error for the case of denied
 func DeniedError(err error) *Error {
-	return New(err).WithCode(DENIED).WithMessage("denied")
+	return New("denied").WithCode(DENIED).WithCause(err)
 }
 
 // UnauthorizedError is error for the case of unauthorized accessing
 func UnauthorizedError(err error) *Error {
-	return New(err).WithCode(UnAuthorizedCode).WithMessage("unauthorized")
+	return New("unauthorized").WithCode(UnAuthorizedCode).WithCause(err)
 }
 
 // BadRequestError is error for the case of bad request
 func BadRequestError(err error) *Error {
-	return New(err).WithCode(BadRequestCode).WithMessage("bad request")
+	return New("bad request").WithCode(BadRequestCode).WithCause(err)
 }
 
 // ForbiddenError is error for the case of forbidden
 func ForbiddenError(err error) *Error {
-	return New(err).WithCode(ForbiddenCode).WithMessage("forbidden")
+	return New("forbidden").WithCode(ForbiddenCode).WithCause(err)
 }
 
 // PreconditionFailedError is error for the case of precondition failed
 func PreconditionFailedError(err error) *Error {
-	return New(err).WithCode(PreconditionCode).WithMessage("precondition failed")
+	return New("precondition failed").WithCode(PreconditionCode).WithCause(err)
 }
 
 // UnknownError ...
 func UnknownError(err error) *Error {
-	return New(err).WithCode(GeneralCode).WithMessage("unknown")
+	return New("unknown").WithCode(GeneralCode).WithCause(err)
+}
+
+// IsNotFoundErr returns true when the error is NotFoundError
+func IsNotFoundErr(err error) bool {
+	return IsErr(err, NotFoundCode)
+}
+
+// IsConflictErr checks whether the err chain contains conflict error
+func IsConflictErr(err error) bool {
+	return IsErr(err, ConflictCode)
 }

src/lib/errors/errors.go

@@ -18,8 +18,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"strings"
-
 	"github.com/goharbor/harbor/src/lib/log"
 )
 
@@ -39,24 +37,13 @@ type Error struct {
 }
 
 // Error returns a human readable error, error.Error() will not contains the track information. Needs it? just call error.StackTrace()
+// Code will not be in the error output.
 func (e *Error) Error() string {
-	var parts []string
-
-	var causeStr string
+	out := e.Message
 	if e.Cause != nil {
-		causeStr = e.Cause.Error()
-		parts = append(parts, causeStr)
+		out = out + ": " + e.Cause.Error()
 	}
-
-	if e.Code != "" {
-		parts = append(parts, e.Code)
-	}
-
-	if e.Message != causeStr {
-		parts = append(parts, e.Message)
-	}
-
-	return strings.Join(parts, ", ")
+	return out
 }
 
 // StackTrace ...
@@ -137,8 +124,8 @@ func New(in interface{}) *Error {
 	default:
 		err = fmt.Errorf("%v", in)
 	}
+
 	return &Error{
-		Cause:   err,
 		Message: err.Error(),
 		Stack:   newStack(),
 	}
@@ -163,16 +150,18 @@ func Wrapf(err error, format string, args ...interface{}) *Error {
 		return nil
 	}
 	e := &Error{
-		Cause: err,
-		Stack: newStack(),
+		Cause:   err,
+		Message: fmt.Sprintf(format, args...),
+		Stack:   newStack(),
 	}
-	return e.WithMessage(format, args...)
+	return e
 }
 
 // Errorf ...
 func Errorf(format string, args ...interface{}) *Error {
 	return &Error{
 		Message: fmt.Sprintf(format, args...),
+		Stack:   newStack(),
 	}
 }
 
@@ -183,6 +172,9 @@ func Cause(err error) error {
 		if !ok {
 			break
 		}
+		if cause.Cause == nil {
+			break
+		}
 		err = cause.Cause
 	}
 	return err
@@ -191,22 +183,12 @@ func Cause(err error) error {
 // IsErr checks whether the err chain contains error matches the code
 func IsErr(err error, code string) bool {
 	var e *Error
-	if errors.As(err, &e) {
+	if As(err, &e) {
 		return e.Code == code
 	}
 	return false
 }
 
-// IsNotFoundErr returns true when the error is NotFoundError
-func IsNotFoundErr(err error) bool {
-	return IsErr(err, NotFoundCode)
-}
-
-// IsConflictErr checks whether the err chain contains conflict error
-func IsConflictErr(err error) bool {
-	return IsErr(err, ConflictCode)
-}
-
 // ErrCode returns code of err
 func ErrCode(err error) string {
 	if err == nil {
@@ -214,7 +196,7 @@ func ErrCode(err error) string {
 	}
 
 	var e *Error
-	if ok := errors.As(err, &e); ok && e.Code != "" {
+	if ok := As(err, &e); ok && e.Code != "" {
 		return e.Code
 	} else if ok && e.Cause != nil {
 		return ErrCode(e.Cause)

src/lib/errors/errors_test.go

@@ -62,7 +62,7 @@ func caller2() error {
 
 func caller3() error {
 	err := caller4()
-	return New(err).WithMessage("it's caller 3.")
+	return New(nil).WithMessage("it's caller 3.").WithCause(err)
 }
 
 func caller4() error {
@@ -89,6 +89,169 @@ func (suite *ErrorTestSuite) TestStackTrace() {
 	suite.Contains(err.Error(), "it's caller 4")
 }
 
+func (suite *ErrorTestSuite) TestNew() {
+	cause := New("root")
+	suite.Equal("root", cause.Error())
+
+	err := New(cause)
+	suite.Equal("root", err.Error())
+
+	err = New(cause.WithCause(errors.New("stdErr")))
+	suite.Equal("root: stdErr", err.Error())
+
+	errStd := errors.New("stdErr")
+	err = New(errStd)
+	suite.Equal("stdErr", err.Error())
+
+	err = New(nil)
+	suite.Equal("<nil>", err.Error())
+
+	err = New("")
+	suite.Equal("", err.Error())
+}
+
+func (suite *ErrorTestSuite) TestWithMessage() {
+	cause := New("root")
+	err := cause.WithMessage("append message").WithMessage("append message2")
+	suite.Equal("append message2", err.Error())
+}
+
+func (suite *ErrorTestSuite) TestWithCause() {
+	cause := errors.New("stdErr")
+	err := New("root").WithCause(cause)
+	suite.Equal("root: stdErr", err.Error())
+}
+
+func (suite *ErrorTestSuite) TestWithCauseMessage() {
+	cause := errors.New("stdErr")
+	err := New("root").WithCause(cause).WithMessage("With Message")
+	suite.Equal("With Message: stdErr", err.Error())
+}
+
+func (suite *ErrorTestSuite) TestWrap() {
+	cause := New("root")
+	cause = Wrap(cause, "err1")
+	cause = Wrap(cause, "err2")
+	cause = Wrap(cause, "err3")
+	suite.Equal("err3: err2: err1: root", cause.Error())
+}
+
+func (suite *ErrorTestSuite) TestWrapf() {
+	cause := New("root")
+	cause = Wrapf(cause, "err%d", 1)
+	cause = Wrapf(cause, "err%d", 2)
+	cause = Wrapf(cause, "err%d", 3)
+	suite.Equal("err3: err2: err1: root", cause.Error())
+}
+
+func (suite *ErrorTestSuite) TestErrof() {
+	err := Errorf("it's err%d", 1)
+	suite.Equal("it's err1", err.Error())
+}
+
+func (suite *ErrorTestSuite) TestErrofWithMessage() {
+	err := Errorf("it's err%d", 1)
+	suite.Equal("it's err1", err.Error())
+}
+
+func (suite *ErrorTestSuite) TestWrapStdErr() {
+	cause := errors.New("stdErr")
+	err := Wrap(cause, "wrap stdErr")
+	suite.Equal("wrap stdErr: stdErr", err.Error())
+}
+
+func (suite *ErrorTestSuite) TestNilErr() {
+	nilErr := New(nil)
+	suite.Equal("<nil>", nilErr.Error())
+}
+
+func (suite *ErrorTestSuite) TestNilWithMessage() {
+	nilErr := New(nil).WithMessage("it's a nil error")
+	suite.Equal("it's a nil error", nilErr.Error())
+}
+
+func (suite *ErrorTestSuite) TestIsNotFoundErr() {
+	err := New(nil).WithCode(NotFoundCode)
+	suite.True(IsNotFoundErr(err))
+
+	err = New(nil).WithCode(PreconditionCode)
+	suite.False(IsNotFoundErr(err))
+}
+
+func (suite *ErrorTestSuite) TestIsConflictErrErr() {
+	err := New(nil).WithCode(ConflictCode)
+	suite.True(IsConflictErr(err))
+
+	err = New(nil).WithCode(PreconditionCode)
+	suite.False(IsConflictErr(err))
+}
+
+func (suite *ErrorTestSuite) TestErrCode() {
+	err := New(nil).WithCode(ConflictCode)
+	suite.Equal(ErrCode(err), ConflictCode)
+
+	err = New("root")
+	suite.Equal(ErrCode(err), GeneralCode)
+
+	err = New("root")
+	suite.Equal(ErrCode(err), GeneralCode)
+	err.WithCode(PreconditionCode)
+	suite.Equal(ErrCode(err), PreconditionCode)
+	err.WithCode(DENIED)
+	suite.Equal(ErrCode(err), DENIED)
+
+	stdErr := errors.New("stdErr")
+	suite.Equal(ErrCode(stdErr), GeneralCode)
+	err = Wrap(stdErr, "wrap stdErr")
+	suite.Equal(ErrCode(stdErr), GeneralCode)
+}
+
+func (suite *ErrorTestSuite) TestErrs() {
+	err := New("root").WithCode(ConflictCode)
+	suite.Equal(`{"errors":[{"code":"CONFLICT","message":"root"}]}`, NewErrs(err).Error())
+}
+
+func (suite *ErrorTestSuite) TestNotFoundError() {
+	root := errors.New("something is not found")
+	err := NotFoundError(root)
+	suite.Equal(`resource not found: something is not found`, err.Error())
+
+	root = errors.New("something is not found")
+	err = NotFoundError(root).WithMessage("asset not found")
+	suite.Equal(`asset not found: something is not found`, err.Error())
+}
+
+func (suite *ErrorTestSuite) TestIsErr() {
+	stdErr := errors.New("stdErr")
+	err := Wrap(stdErr, "root")
+	targetErr := Wrap(err, "target err").WithCode(ConflictCode)
+
+	suite.True(IsErr(targetErr, ConflictCode))
+}
+
+func (suite *ErrorTestSuite) TestCause() {
+	// self
+	root := New("root")
+	suite.Equal(root, Cause(root))
+
+	root = New("root")
+	cause1 := Wrap(root, "err1")
+	cause2 := Wrap(cause1, "err2")
+	cause3 := Wrap(cause2, "err3")
+	suite.Equal(root, Cause(cause3))
+}
+
+func (suite *ErrorTestSuite) TestCauseStd() {
+	root := errors.New("stdErr")
+	suite.Equal(root, Cause(root))
+
+	root = errors.New("stdErr")
+	cause1 := Wrap(root, "err1")
+	cause2 := Wrap(cause1, "err2")
+	cause3 := Wrap(cause2, "err3")
+	suite.Equal(root, Cause(cause3))
+}
+
 func TestErrorTestSuite(t *testing.T) {
 	suite.Run(t, &ErrorTestSuite{})
 }