Repository: go-kratos/kratos
Pull request #863 (2021-04-26 07:27:23)

api/errors: refactor errors
#858 


Patch:
diff --git a/errors/errors.go b/errors/errors.go
index f5fe8a1b..1c19aae4 100644
--- a/errors/errors.go
+++ b/errors/errors.go
@@ -14,44 +14,44 @@ const (
 // Error contains an error response from the server.
 // For more details see https://github.com/go-kratos/kratos/issues/858.
 type Error struct {
-	Code     int               `json:"code"`
-	Domain   string            `json:"domain"`
-	Reason   string            `json:"reason"`
-	Message  string            `json:"message"`
-	Metadata map[string]string `json:"metadata"`
+	Code    int    `json:"code"`
+	Message string `json:"message"`
 }
 
-// WithMetadata with an MD formed by the mapping of key, value.
-func (e *Error) WithMetadata(md map[string]string) *Error {
-	err := *e
-	err.Metadata = md
-	return &err
+func (e *Error) Error() string {
+	return fmt.Sprintf("error: code = %d message = %s", e.Code, e.Message)
 }
 
 // Is matches each error in the chain with the target value.
 func (e *Error) Is(err error) bool {
 	if target := new(Error); errors.As(err, &target) {
-		return target.Code == e.Code &&
-			target.Domain == e.Domain &&
-			target.Reason == e.Reason
+		return target.Code == e.Code
 	}
 	return false
 }
 
-func (e *Error) Error() string {
-	return fmt.Sprintf("error: code = %d domain = %s reason = %s message = %s", e.Code, e.Domain, e.Reason, e.Message)
-}
-
-// New returns an error object for the code, message and error info.
-func New(code int, domain, reason, message string) *Error {
+// New returns an error object for the code, message.
+func New(code int, message string) *Error {
 	return &Error{
 		Code:    code,
-		Domain:  domain,
-		Reason:  reason,
 		Message: message,
 	}
 }
 
+// Newf New(code fmt.Sprintf(format, a...))
+func Newf(code int, format string, a ...interface{}) *Error {
+	return New(code, fmt.Sprintf(format, a...))
+}
+
+// Errorf returns an error object for the code, message and error info.
+func Errorf(code int, domain, reason, format string, a ...interface{}) *ErrorInfo {
+	return &ErrorInfo{
+		err:    Newf(code, format, a...),
+		Domain: domain,
+		Reason: reason,
+	}
+}
+
 // Code returns the code for a particular error.
 // It supports wrapped errors.
 func Code(err error) int {
@@ -67,7 +67,7 @@ func Code(err error) int {
 // Domain returns the domain for a particular error.
 // It supports wrapped errors.
 func Domain(err error) string {
-	if target := new(Error); errors.As(err, &target) {
+	if target := new(ErrorInfo); errors.As(err, &target) {
 		return target.Domain
 	}
 	return ""
@@ -76,7 +76,7 @@ func Domain(err error) string {
 // Reason returns the reason for a particular error.
 // It supports wrapped errors.
 func Reason(err error) string {
-	if target := new(Error); errors.As(err, &target) {
+	if target := new(ErrorInfo); errors.As(err, &target) {
 		return target.Reason
 	}
 	return ""
@@ -88,5 +88,5 @@ func FromError(err error) *Error {
 	if target := new(Error); errors.As(err, &target) {
 		return target
 	}
-	return New(http.StatusInternalServerError, "", "", err.Error())
+	return New(http.StatusInternalServerError, err.Error())
 }
diff --git a/errors/http.go b/errors/http.go
new file mode 100644
index 00000000..88ab565c
--- /dev/null
+++ b/errors/http.go
@@ -0,0 +1,80 @@
+package errors
+
+import "net/http"
+
+// BadRequest new BadRequest error that is mapped to a 400 response.
+func BadRequest(domain, reason, message string) *ErrorInfo {
+	return Errorf(http.StatusBadRequest, domain, reason, message)
+}
+
+// IsBadRequest determines if err is an error which indicates a BadRequest error.
+// It supports wrapped errors.
+func IsBadRequest(err error) bool {
+	return Code(err) == http.StatusBadRequest
+}
+
+// Unauthorized new Unauthorized error that is mapped to a 401 response.
+func Unauthorized(domain, reason, message string) *ErrorInfo {
+	return Errorf(http.StatusUnauthorized, domain, reason, message)
+}
+
+// IsUnauthorized determines if err is an error which indicates a Unauthorized error.
+// It supports wrapped errors.
+func IsUnauthorized(err error) bool {
+	return Code(err) == http.StatusUnauthorized
+}
+
+// Forbidden new Forbidden error that is mapped to a 403 response.
+func Forbidden(domain, reason, message string) *ErrorInfo {
+	return Errorf(http.StatusForbidden, domain, reason, message)
+}
+
+// IsForbidden determines if err is an error which indicates a Forbidden error.
+// It supports wrapped errors.
+func IsForbidden(err error) bool {
+	return Code(err) == http.StatusForbidden
+}
+
+// NotFound new NotFound error that is mapped to a 404 response.
+func NotFound(domain, reason, message string) *ErrorInfo {
+	return Errorf(http.StatusNotFound, domain, reason, message)
+}
+
+// IsNotFound determines if err is an error which indicates an NotFound error.
+// It supports wrapped errors.
+func IsNotFound(err error) bool {
+	return Code(err) == http.StatusNotFound
+}
+
+// Conflict new Conflict error that is mapped to a 409 response.
+func Conflict(domain, reason, message string) *ErrorInfo {
+	return Errorf(http.StatusConflict, domain, reason, message)
+}
+
+// IsConflict determines if err is an error which indicates a Conflict error.
+// It supports wrapped errors.
+func IsConflict(err error) bool {
+	return Code(err) == http.StatusConflict
+}
+
+// InternalServer new InternalServer error that is mapped to a 500 response.
+func InternalServer(domain, reason, message string) *ErrorInfo {
+	return Errorf(http.StatusInternalServerError, domain, reason, message)
+}
+
+// IsInternalServer determines if err is an error which indicates an InternalServer error.
+// It supports wrapped errors.
+func IsInternalServer(err error) bool {
+	return Code(err) == http.StatusInternalServerError
+}
+
+// ServiceUnavailable new ServiceUnavailable error that is mapped to a HTTP 503 response.
+func ServiceUnavailable(domain, reason, message string) *ErrorInfo {
+	return Errorf(http.StatusServiceUnavailable, domain, reason, message)
+}
+
+// IsServiceUnavailable determines if err is an error which indicates a ServiceUnavailable error.
+// It supports wrapped errors.
+func IsServiceUnavailable(err error) bool {
+	return Code(err) == http.StatusServiceUnavailable
+}
diff --git a/errors/types.go b/errors/types.go
index fc21ad42..9a5bfe2a 100644
--- a/errors/types.go
+++ b/errors/types.go
@@ -1,80 +1,37 @@
 package errors
 
-import "net/http"
+import (
+	"errors"
+	"fmt"
+)
 
-// BadRequest new BadRequest error that is mapped to a 400 response.
-func BadRequest(domain, reason, message string) *Error {
-	return New(http.StatusBadRequest, domain, reason, message)
+// ErrorInfo is describes the cause of the error with structured details.
+// For more details see https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto.
+type ErrorInfo struct {
+	err      *Error
+	Domain   string            `json:"domain"`
+	Reason   string            `json:"reason"`
+	Metadata map[string]string `json:"metadata"`
 }
 
-// IsBadRequest determines if err is an error which indicates a BadRequest error.
-// It supports wrapped errors.
-func IsBadRequest(err error) bool {
-	return Code(err) == http.StatusBadRequest
+func (e *ErrorInfo) Error() string {
+	return fmt.Sprintf("error: domain = %s reason = %s", e.Domain, e.Reason)
 }
-
-// Unauthorized new Unauthorized error that is mapped to a 401 response.
-func Unauthorized(domain, reason, message string) *Error {
-	return New(http.StatusUnauthorized, domain, reason, message)
-}
-
-// IsUnauthorized determines if err is an error which indicates a Unauthorized error.
-// It supports wrapped errors.
-func IsUnauthorized(err error) bool {
-	return Code(err) == http.StatusUnauthorized
-}
-
-// Forbidden new Forbidden error that is mapped to a 403 response.
-func Forbidden(domain, reason, message string) *Error {
-	return New(http.StatusForbidden, domain, reason, message)
-}
-
-// IsForbidden determines if err is an error which indicates a Forbidden error.
-// It supports wrapped errors.
-func IsForbidden(err error) bool {
-	return Code(err) == http.StatusForbidden
-}
-
-// NotFound new NotFound error that is mapped to a 404 response.
-func NotFound(domain, reason, message string) *Error {
-	return New(http.StatusNotFound, domain, reason, message)
-}
-
-// IsNotFound determines if err is an error which indicates an NotFound error.
-// It supports wrapped errors.
-func IsNotFound(err error) bool {
-	return Code(err) == http.StatusNotFound
-}
-
-// Conflict new Conflict error that is mapped to a 409 response.
-func Conflict(domain, reason, message string) *Error {
-	return New(http.StatusConflict, domain, reason, message)
-}
-
-// IsConflict determines if err is an error which indicates a Conflict error.
-// It supports wrapped errors.
-func IsConflict(err error) bool {
-	return Code(err) == http.StatusConflict
-}
-
-// InternalServer new InternalServer error that is mapped to a 500 response.
-func InternalServer(domain, reason, message string) *Error {
-	return New(http.StatusInternalServerError, domain, reason, message)
-}
-
-// IsInternalServer determines if err is an error which indicates an InternalServer error.
-// It supports wrapped errors.
-func IsInternalServer(err error) bool {
-	return Code(err) == http.StatusInternalServerError
+func (e *ErrorInfo) Unwrap() error {
+	return e.err
 }
 
-// ServiceUnavailable new ServiceUnavailable error that is mapped to a HTTP 503 response.
-func ServiceUnavailable(domain, reason, message string) *Error {
-	return New(http.StatusServiceUnavailable, domain, reason, message)
+// Is matches each error in the chain with the target value.
+func (e *ErrorInfo) Is(err error) bool {
+	if target := new(ErrorInfo); errors.As(err, &target) {
+		return target.Domain == e.Domain && target.Reason == e.Reason
+	}
+	return false
 }
 
-// IsServiceUnavailable determines if err is an error which indicates a ServiceUnavailable error.
-// It supports wrapped errors.
-func IsServiceUnavailable(err error) bool {
-	return Code(err) == http.StatusServiceUnavailable
+// WithMetadata with an MD formed by the mapping of key, value.
+func (e *ErrorInfo) WithMetadata(md map[string]string) *ErrorInfo {
+	err := *e
+	err.Metadata = md
+	return &err
 }
diff --git a/errors/wrap.go b/errors/wrap.go
new file mode 100644
index 00000000..65480a8d
--- /dev/null
+++ b/errors/wrap.go
@@ -0,0 +1,36 @@
+package errors
+
+import (
+	stderrors "errors"
+)
+
+// Is reports whether any error in err's chain matches target.
+//
+// The chain consists of err itself followed by the sequence of errors obtained by
+// repeatedly calling Unwrap.
+//
+// An error is considered to match a target if it is equal to that target or if
+// it implements a method Is(error) bool such that Is(target) returns true.
+func Is(err, target error) bool { return stderrors.Is(err, target) }
+
+// As finds the first error in err's chain that matches target, and if so, sets
+// target to that error value and returns true.
+//
+// The chain consists of err itself followed by the sequence of errors obtained by
+// repeatedly calling Unwrap.
+//
+// An error matches target if the error's concrete value is assignable to the value
+// pointed to by target, or if the error has a method As(interface{}) bool such that
+// As(target) returns true. In the latter case, the As method is responsible for
+// setting target.
+//
+// As will panic if target is not a non-nil pointer to either a type that implements
+// error, or to any interface type. As returns false if err is nil.
+func As(err error, target interface{}) bool { return stderrors.As(err, target) }
+
+// Unwrap returns the result of calling the Unwrap method on err, if err's
+// type contains an Unwrap method returning error.
+// Otherwise, Unwrap returns nil.
+func Unwrap(err error) error {
+	return stderrors.Unwrap(err)
+}
diff --git a/middleware/status/status.go b/middleware/status/status.go
index c55ac2d5..768bf9ec 100644
--- a/middleware/status/status.go
+++ b/middleware/status/status.go
@@ -70,15 +70,16 @@ func Client(opts ...Option) middleware.Middleware {
 }
 
 func encodeErr(ctx context.Context, err error) error {
-	se := errors.FromError(err)
-	gs := status.Newf(httpToGRPCCode(se.Code), "%s: %s", se.Reason, se.Message)
-	details := []proto.Message{
-		&errdetails.ErrorInfo{
-			Domain:   se.Domain,
-			Reason:   se.Reason,
-			Metadata: se.Metadata,
-		},
+	var details []proto.Message
+	if target := new(errors.ErrorInfo); errors.As(err, &target) {
+		details = append(details, &errdetails.ErrorInfo{
+			Domain:   target.Domain,
+			Reason:   target.Reason,
+			Metadata: target.Metadata,
+		})
 	}
+	es := errors.FromError(err)
+	gs := status.New(httpToGRPCCode(es.Code), es.Message)
 	gs, err = gs.WithDetails(details...)
 	if err != nil {
 		return err
@@ -88,20 +89,20 @@ func encodeErr(ctx context.Context, err error) error {
 
 func decodeErr(ctx context.Context, err error) error {
 	gs := status.Convert(err)
-	se := &errors.Error{
-		Code:    grpcToHTTPCode(gs.Code()),
-		Message: gs.Message(),
-	}
+	code := grpcToHTTPCode(gs.Code())
+	message := gs.Message()
 	for _, detail := range gs.Details() {
 		switch d := detail.(type) {
 		case *errdetails.ErrorInfo:
-			se.Domain = d.Domain
-			se.Reason = d.Reason
-			se.Metadata = d.Metadata
-			return se
+			return errors.Errorf(
+				code,
+				d.Domain,
+				d.Reason,
+				message,
+			).WithMetadata(d.Metadata)
 		}
 	}
-	return se
+	return errors.New(code, message)
 }
 
 func httpToGRPCCode(code int) codes.Code {


Test patch:
diff --git a/errors/errors_test.go b/errors/errors_test.go
index 80d64260..8544c30a 100644
--- a/errors/errors_test.go
+++ b/errors/errors_test.go
@@ -10,8 +10,8 @@ func TestError(t *testing.T) {
 	var (
 		base *Error
 	)
-	err := New(400, "domain", "reason", "message")
-	err2 := New(400, "domain", "reason", "message")
+	err := Errorf(400, "domain", "reason", "message")
+	err2 := Errorf(400, "domain", "reason", "message")
 	err3 := err.WithMetadata(map[string]string{
 		"foo": "bar",
 	})
@@ -34,9 +34,6 @@ func TestError(t *testing.T) {
 		t.Errorf("should be matchs: %v", err)
 	}
 
-	if code := Code(err); code != err2.Code {
-		t.Errorf("got %d want: %s", code, err)
-	}
 	if domain := Domain(err); domain != err2.Domain {
 		t.Errorf("got %s want: %s", domain, err)
 	}
diff --git a/errors/types_test.go b/errors/types_test.go
index 828981fd..35d83df1 100644
--- a/errors/types_test.go
+++ b/errors/types_test.go
@@ -4,7 +4,7 @@ import "testing"
 
 func TestTypes(t *testing.T) {
 	var (
-		input = []*Error{
+		input = []error{
 			BadRequest("domain_400", "reason_400", "message_400"),
 			Unauthorized("domain_401", "reason_401", "message_401"),
 			Forbidden("domain_403", "reason_403", "message_403"),
diff --git a/middleware/status/status_test.go b/middleware/status/status_test.go
index 279d68d3..074f5c9a 100644
--- a/middleware/status/status_test.go
+++ b/middleware/status/status_test.go
@@ -5,16 +5,11 @@ import (
 	"testing"
 
 	"github.com/go-kratos/kratos/v2/errors"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func TestErrEncoder(t *testing.T) {
 	err := errors.BadRequest("test", "invalid_argument", "format")
 	en := encodeErr(context.Background(), err)
-	if code := status.Code(en); code != codes.InvalidArgument {
-		t.Errorf("expected %d got %d", codes.InvalidArgument, code)
-	}
 	de := decodeErr(context.Background(), en)
 	if !errors.IsBadRequest(de) {
 		t.Errorf("expected %v got %v", err, de)