From 7c6f53132f4a407886670a7b870bade58dd73316 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Wed, 28 Apr 2021 23:09:27 +0800 Subject: [PATCH] api/errors: refactor to grpc statas (#880) * refactor to grpc status --- errors/errors.go | 73 +++++++++++------- errors/errors_test.go | 6 +- errors/http.go | 80 -------------------- errors/types.go | 95 +++++++++++++++++------ examples/blog/internal/server/grpc.go | 2 - examples/blog/internal/server/http.go | 2 - examples/helloworld/client/main.go | 3 - examples/helloworld/server/main.go | 3 - middleware/logging/logging.go | 9 ++- middleware/metrics/metrics.go | 8 +- middleware/status/status.go | 105 -------------------------- middleware/status/status_test.go | 17 ----- transport/http/client.go | 9 +-- 13 files changed, 133 insertions(+), 279 deletions(-) delete mode 100644 errors/http.go delete mode 100644 middleware/status/status.go delete mode 100644 middleware/status/status_test.go diff --git a/errors/errors.go b/errors/errors.go index 1c19aae4f..1fdb44ac6 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -4,6 +4,10 @@ import ( "errors" "fmt" "net/http" + + "google.golang.org/genproto/googleapis/rpc/errdetails" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) const ( @@ -11,42 +15,66 @@ const ( SupportPackageIsVersion1 = true ) -// Error contains an error response from the server. -// For more details see https://github.com/go-kratos/kratos/issues/858. +// Error 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 Error struct { - Code int `json:"code"` - Message string `json:"message"` + s *status.Status + + Domain string `json:"domain"` + Reason string `json:"reason"` + Metadata map[string]string `json:"metadata"` } func (e *Error) Error() string { - return fmt.Sprintf("error: code = %d message = %s", e.Code, e.Message) + return fmt.Sprintf("error: domain = %s reason = %s", e.Domain, e.Reason) +} + +// GRPCStatus returns the Status represented by se. +func (e *Error) GRPCStatus() *status.Status { + s, err := e.s.WithDetails(&errdetails.ErrorInfo{ + Domain: e.Domain, + Reason: e.Reason, + Metadata: e.Metadata, + }) + if err != nil { + return e.s + } + return s } // 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 + return target.Domain == e.Domain && target.Reason == e.Reason } return false } +// 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 +} + // New returns an error object for the code, message. -func New(code int, message string) *Error { +func New(code codes.Code, domain, reason, message string) *Error { return &Error{ - Code: code, - Message: message, + s: status.New(codes.Code(code), message), + Domain: domain, + Reason: reason, } } // Newf New(code fmt.Sprintf(format, a...)) -func Newf(code int, format string, a ...interface{}) *Error { - return New(code, fmt.Sprintf(format, a...)) +func Newf(code codes.Code, domain, reason, format string, a ...interface{}) *Error { + return New(code, domain, reason, 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...), +func Errorf(code codes.Code, domain, reason, format string, a ...interface{}) error { + return &Error{ + s: status.New(codes.Code(code), fmt.Sprintf(format, a...)), Domain: domain, Reason: reason, } @@ -54,12 +82,12 @@ func Errorf(code int, domain, reason, format string, a ...interface{}) *ErrorInf // Code returns the code for a particular error. // It supports wrapped errors. -func Code(err error) int { +func Code(err error) codes.Code { if err == nil { return http.StatusOK } if target := new(Error); errors.As(err, &target) { - return target.Code + return target.s.Code() } return http.StatusInternalServerError } @@ -67,7 +95,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(ErrorInfo); errors.As(err, &target) { + if target := new(Error); errors.As(err, &target) { return target.Domain } return "" @@ -76,17 +104,8 @@ 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(ErrorInfo); errors.As(err, &target) { + if target := new(Error); errors.As(err, &target) { return target.Reason } return "" } - -// FromError try to convert an error to *Error. -// It supports wrapped errors. -func FromError(err error) *Error { - if target := new(Error); errors.As(err, &target) { - return target - } - return New(http.StatusInternalServerError, err.Error()) -} diff --git a/errors/errors_test.go b/errors/errors_test.go index 8544c30af..3255a6835 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -4,14 +4,16 @@ import ( "errors" "fmt" "testing" + + "google.golang.org/grpc/codes" ) func TestError(t *testing.T) { var ( base *Error ) - err := Errorf(400, "domain", "reason", "message") - err2 := Errorf(400, "domain", "reason", "message") + err := Newf(codes.InvalidArgument, "domain", "reason", "message") + err2 := Newf(codes.InvalidArgument, "domain", "reason", "message") err3 := err.WithMetadata(map[string]string{ "foo": "bar", }) diff --git a/errors/http.go b/errors/http.go deleted file mode 100644 index 88ab565c1..000000000 --- a/errors/http.go +++ /dev/null @@ -1,80 +0,0 @@ -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 9a5bfe2a0..b96b8f899 100644 --- a/errors/types.go +++ b/errors/types.go @@ -1,37 +1,82 @@ package errors import ( - "errors" - "fmt" + "google.golang.org/grpc/codes" ) -// 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"` +// BadRequest new BadRequest error that is mapped to a 400 response. +func BadRequest(domain, reason, message string) *Error { + return Newf(codes.InvalidArgument, domain, reason, message) } -func (e *ErrorInfo) Error() string { - return fmt.Sprintf("error: domain = %s reason = %s", e.Domain, e.Reason) -} -func (e *ErrorInfo) Unwrap() error { - return e.err +// IsBadRequest determines if err is an error which indicates a BadRequest error. +// It supports wrapped errors. +func IsBadRequest(err error) bool { + return Code(err) == codes.InvalidArgument } -// 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 +// Unauthorized new Unauthorized error that is mapped to a 401 response. +func Unauthorized(domain, reason, message string) *Error { + return Newf(codes.Unauthenticated, domain, reason, message) } -// 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 +// IsUnauthorized determines if err is an error which indicates a Unauthorized error. +// It supports wrapped errors. +func IsUnauthorized(err error) bool { + return Code(err) == codes.Unauthenticated +} + +// Forbidden new Forbidden error that is mapped to a 403 response. +func Forbidden(domain, reason, message string) *Error { + return Newf(codes.PermissionDenied, 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) == codes.PermissionDenied +} + +// NotFound new NotFound error that is mapped to a 404 response. +func NotFound(domain, reason, message string) *Error { + return Newf(codes.NotFound, 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) == codes.NotFound +} + +// Conflict new Conflict error that is mapped to a 409 response. +func Conflict(domain, reason, message string) *Error { + return Newf(codes.Aborted, 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) == codes.Aborted +} + +// InternalServer new InternalServer error that is mapped to a 500 response. +func InternalServer(domain, reason, message string) *Error { + return Newf(codes.Internal, domain, reason, message) +} + +// IsInternalServer determines if err is an error which indicates an Internal error. +// It supports wrapped errors. +func IsInternalServer(err error) bool { + return Code(err) == codes.Internal +} + +// ServiceUnavailable new ServiceUnavailable error that is mapped to a HTTP 503 response. +func ServiceUnavailable(domain, reason, message string) *Error { + return Newf(codes.Unavailable, domain, reason, message) +} + +// IsServiceUnavailable determines if err is an error which indicates a Unavailable error. +// It supports wrapped errors. +func IsServiceUnavailable(err error) bool { + return Code(err) == codes.Unavailable } diff --git a/examples/blog/internal/server/grpc.go b/examples/blog/internal/server/grpc.go index 454b02b54..17a1ba6fb 100644 --- a/examples/blog/internal/server/grpc.go +++ b/examples/blog/internal/server/grpc.go @@ -8,7 +8,6 @@ import ( "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/middleware/logging" "github.com/go-kratos/kratos/v2/middleware/recovery" - "github.com/go-kratos/kratos/v2/middleware/status" "github.com/go-kratos/kratos/v2/middleware/tracing" "github.com/go-kratos/kratos/v2/transport/grpc" "go.opentelemetry.io/otel/trace" @@ -19,7 +18,6 @@ func NewGRPCServer(c *conf.Server, tracer trace.TracerProvider, blog *service.Bl var opts = []grpc.ServerOption{ grpc.Middleware( middleware.Chain( - status.Server(), tracing.Server(tracing.WithTracerProvider(tracer)), logging.Server(log.DefaultLogger), recovery.Recovery(), diff --git a/examples/blog/internal/server/http.go b/examples/blog/internal/server/http.go index 68f4a5913..c991bab1c 100644 --- a/examples/blog/internal/server/http.go +++ b/examples/blog/internal/server/http.go @@ -8,7 +8,6 @@ import ( "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/middleware/logging" "github.com/go-kratos/kratos/v2/middleware/recovery" - "github.com/go-kratos/kratos/v2/middleware/status" "github.com/go-kratos/kratos/v2/middleware/tracing" "github.com/go-kratos/kratos/v2/transport/http" "go.opentelemetry.io/otel/trace" @@ -28,7 +27,6 @@ func NewHTTPServer(c *conf.Server, tracer trace.TracerProvider, blog *service.Bl } m := http.Middleware( middleware.Chain( - status.Server(), tracing.Server(tracing.WithTracerProvider(tracer)), logging.Server(log.DefaultLogger), recovery.Recovery(), diff --git a/examples/helloworld/client/main.go b/examples/helloworld/client/main.go index bd4ab79c9..6b23eec44 100644 --- a/examples/helloworld/client/main.go +++ b/examples/helloworld/client/main.go @@ -9,7 +9,6 @@ import ( "github.com/go-kratos/kratos/v2/errors" "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/middleware/recovery" - "github.com/go-kratos/kratos/v2/middleware/status" transgrpc "github.com/go-kratos/kratos/v2/transport/grpc" transhttp "github.com/go-kratos/kratos/v2/transport/http" ) @@ -25,7 +24,6 @@ func callHTTP() { transhttp.WithMiddleware( middleware.Chain( recovery.Recovery(), - status.Client(), ), ), ) @@ -59,7 +57,6 @@ func callGRPC() { transgrpc.WithEndpoint("127.0.0.1:9000"), transgrpc.WithMiddleware( middleware.Chain( - status.Client(), recovery.Recovery(), ), ), diff --git a/examples/helloworld/server/main.go b/examples/helloworld/server/main.go index 8ed226b2b..33e11de3f 100644 --- a/examples/helloworld/server/main.go +++ b/examples/helloworld/server/main.go @@ -12,7 +12,6 @@ import ( "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/middleware/logging" "github.com/go-kratos/kratos/v2/middleware/recovery" - "github.com/go-kratos/kratos/v2/middleware/status" "github.com/go-kratos/kratos/v2/transport/grpc" "github.com/go-kratos/kratos/v2/transport/http" ) @@ -50,7 +49,6 @@ func main() { grpc.Address(":9000"), grpc.Middleware( middleware.Chain( - status.Server(), logging.Server(logger), recovery.Recovery(), ), @@ -63,7 +61,6 @@ func main() { httpSrv.HandlePrefix("/", pb.NewGreeterHandler(s, http.Middleware( middleware.Chain( - status.Server(), logging.Server(logger), recovery.Recovery(), ), diff --git a/middleware/logging/logging.go b/middleware/logging/logging.go index 001d4c008..bbeee50e0 100644 --- a/middleware/logging/logging.go +++ b/middleware/logging/logging.go @@ -3,6 +3,7 @@ package logging import ( "context" "fmt" + "go.opentelemetry.io/otel/trace" "github.com/go-kratos/kratos/v2/errors" @@ -49,7 +50,7 @@ func Server(l log.Logger) middleware.Middleware { "method", method, "args", args, "query", query, - "code", errors.Code(err), + "code", uint32(errors.Code(err)), "error", err.Error(), ) return nil, err @@ -73,7 +74,7 @@ func Server(l log.Logger) middleware.Middleware { "path", path, "method", method, "args", args, - "code", errors.Code(err), + "code", uint32(errors.Code(err)), "error", err.Error(), ) return nil, err @@ -130,7 +131,7 @@ func Client(l log.Logger) middleware.Middleware { "method", method, "args", args, "query", query, - "code", errors.Code(err), + "code", uint32(errors.Code(err)), "error", err.Error(), ) return nil, err @@ -154,7 +155,7 @@ func Client(l log.Logger) middleware.Middleware { "path", path, "method", method, "args", args, - "code", errors.Code(err), + "code", uint32(errors.Code(err)), "error", err.Error(), ) return nil, err diff --git a/middleware/metrics/metrics.go b/middleware/metrics/metrics.go index bc99dc5a1..a269f6929 100644 --- a/middleware/metrics/metrics.go +++ b/middleware/metrics/metrics.go @@ -48,7 +48,7 @@ func Server(opts ...Option) middleware.Middleware { var ( method string path string - code int + code uint32 ) if info, ok := grpc.FromServerContext(ctx); ok { method = "POST" @@ -65,7 +65,7 @@ func Server(opts ...Option) middleware.Middleware { startTime := time.Now() reply, err := handler(ctx, req) if err != nil { - code = errors.Code(err) + code = uint32(errors.Code(err)) } if options.requests != nil { options.requests.With(method, path, strconv.Itoa(int(code))).Inc() @@ -90,7 +90,7 @@ func Client(opts ...Option) middleware.Middleware { var ( method string path string - code int + code uint32 ) if info, ok := grpc.FromClientContext(ctx); ok { method = "POST" @@ -102,7 +102,7 @@ func Client(opts ...Option) middleware.Middleware { startTime := time.Now() reply, err := handler(ctx, req) if err != nil { - code = errors.Code(err) + code = uint32(errors.Code(err)) } if options.requests != nil { options.requests.With(method, path, strconv.Itoa(int(code))).Inc() diff --git a/middleware/status/status.go b/middleware/status/status.go deleted file mode 100644 index 5d4311faa..000000000 --- a/middleware/status/status.go +++ /dev/null @@ -1,105 +0,0 @@ -package status - -import ( - "context" - - "github.com/go-kratos/kratos/v2/errors" - "github.com/go-kratos/kratos/v2/internal/http" - "github.com/go-kratos/kratos/v2/middleware" - - //lint:ignore SA1019 grpc - "github.com/golang/protobuf/proto" - "google.golang.org/genproto/googleapis/rpc/errdetails" - "google.golang.org/grpc/status" -) - -// HandlerFunc is middleware error handler. -type HandlerFunc func(context.Context, error) error - -// Option is recovery option. -type Option func(*options) - -type options struct { - handler HandlerFunc -} - -// WithHandler with status handler. -func WithHandler(h HandlerFunc) Option { - return func(o *options) { - o.handler = h - } -} - -// Server is an error middleware. -func Server(opts ...Option) middleware.Middleware { - options := options{ - handler: encodeError, - } - for _, o := range opts { - o(&options) - } - return func(handler middleware.Handler) middleware.Handler { - return func(ctx context.Context, req interface{}) (interface{}, error) { - reply, err := handler(ctx, req) - if err != nil { - return nil, options.handler(ctx, err) - } - return reply, nil - } - } -} - -// Client is an error middleware. -func Client(opts ...Option) middleware.Middleware { - options := options{ - handler: decodeError, - } - for _, o := range opts { - o(&options) - } - return func(handler middleware.Handler) middleware.Handler { - return func(ctx context.Context, req interface{}) (interface{}, error) { - reply, err := handler(ctx, req) - if err != nil { - return nil, options.handler(ctx, err) - } - return reply, nil - } - } -} - -func encodeError(ctx context.Context, err error) error { - 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(http.GRPCCodeFromStatus(es.Code), es.Message) - gs, err = gs.WithDetails(details...) - if err != nil { - return err - } - return gs.Err() -} - -func decodeError(ctx context.Context, err error) error { - gs := status.Convert(err) - code := http.StatusFromGRPCCode(gs.Code()) - message := gs.Message() - for _, detail := range gs.Details() { - switch d := detail.(type) { - case *errdetails.ErrorInfo: - return errors.Errorf( - code, - d.Domain, - d.Reason, - message, - ).WithMetadata(d.Metadata) - } - } - return errors.New(code, message) -} diff --git a/middleware/status/status_test.go b/middleware/status/status_test.go deleted file mode 100644 index a974e09ee..000000000 --- a/middleware/status/status_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package status - -import ( - "context" - "testing" - - "github.com/go-kratos/kratos/v2/errors" -) - -func TestErrEncoder(t *testing.T) { - err := errors.BadRequest("test", "invalid_argument", "format") - en := encodeError(context.Background(), err) - de := decodeError(context.Background(), en) - if !errors.IsBadRequest(de) { - t.Errorf("expected %v got %v", err, de) - } -} diff --git a/transport/http/client.go b/transport/http/client.go index 9c987757b..b4303c66b 100644 --- a/transport/http/client.go +++ b/transport/http/client.go @@ -7,7 +7,6 @@ import ( "time" "github.com/go-kratos/kratos/v2/encoding" - "github.com/go-kratos/kratos/v2/errors" xhttp "github.com/go-kratos/kratos/v2/internal/http" "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/transport" @@ -75,7 +74,7 @@ func NewTransport(ctx context.Context, opts ...ClientOption) (http.RoundTripper, ctx: ctx, timeout: 500 * time.Millisecond, transport: http.DefaultTransport, - errorDecoder: checkResponse, + errorDecoder: CheckResponse, } for _, o := range opts { o(options) @@ -145,9 +144,9 @@ func Do(client *http.Client, req *http.Request, target interface{}) error { return codec.Unmarshal(data, target) } -// checkResponse returns an error (of type *Error) if the response +// CheckResponse returns an error (of type *Error) if the response // status code is not 2xx. -func checkResponse(ctx context.Context, res *http.Response) error { +func CheckResponse(ctx context.Context, res *http.Response) error { if res.StatusCode >= 200 && res.StatusCode <= 299 { return nil } @@ -158,5 +157,5 @@ func checkResponse(ctx context.Context, res *http.Response) error { return status.ErrorProto(st) } } - return errors.New(res.StatusCode, "") + return status.Error(xhttp.GRPCCodeFromStatus(res.StatusCode), res.Status) }