diff --git a/pkg/error_code/codes.go b/pkg/error_code/codes.go index a9ce721365c1..a5bbcf9c64c3 100644 --- a/pkg/error_code/codes.go +++ b/pkg/error_code/codes.go @@ -39,14 +39,16 @@ func NewInvalidInputErr(err error) ErrorCode { var _ ErrorCode = (*invalidInputErr)(nil) // assert implements interface var _ HasClientData = (*invalidInputErr)(nil) // assert implements interface +var _ Causer = (*invalidInputErr)(nil) // assert implements interface -// internalError gives the code InvalidInputCode -type internalErr struct{ CodedError } +// internalError gives the code InternalCode +type internalErr struct{ StackCode } // NewInternalErr creates an internalError from an err // If the given err is an ErrorCode that is a descendant of InternalCode, // its code will be used. // This ensures the intention of sending an HTTP 50x. +// This function also records a stack trace. func NewInternalErr(err error) ErrorCode { code := InternalCode if errcode, ok := err.(ErrorCode); ok { @@ -55,11 +57,12 @@ func NewInternalErr(err error) ErrorCode { code = errCode } } - return internalErr{CodedError{GetCode: code, Err: err}} + return internalErr{NewStackCode(CodedError{GetCode: code, Err: err}, 2)} } var _ ErrorCode = (*internalErr)(nil) // assert implements interface var _ HasClientData = (*internalErr)(nil) // assert implements interface +var _ Causer = (*internalErr)(nil) // assert implements interface // notFound gives the code NotFoundCode type notFoundErr struct{ CodedError } @@ -73,6 +76,7 @@ func NewNotFoundErr(err error) ErrorCode { var _ ErrorCode = (*notFoundErr)(nil) // assert implements interface var _ HasClientData = (*notFoundErr)(nil) // assert implements interface +var _ Causer = (*notFoundErr)(nil) // assert implements interface // CodedError is a convenience to attach a code to an error and already satisfy the ErrorCode interface. // If the error is a struct, that struct will get preseneted as data to the client. @@ -101,11 +105,17 @@ func NewCodedError(err error, code Code) CodedError { var _ ErrorCode = (*CodedError)(nil) // assert implements interface var _ HasClientData = (*CodedError)(nil) // assert implements interface +var _ Causer = (*CodedError)(nil) // assert implements interface func (e CodedError) Error() string { return e.Err.Error() } +// Cause satisfies the Causer interface. +func (e CodedError) Cause() error { + return e.Err +} + // Code returns the GetCode field func (e CodedError) Code() Code { return e.GetCode diff --git a/pkg/error_code/error_code.go b/pkg/error_code/error_code.go index 76df51d73f71..31949ef74765 100644 --- a/pkg/error_code/error_code.go +++ b/pkg/error_code/error_code.go @@ -21,7 +21,7 @@ // This package is designed to have few opinions and be a starting point for how you want to do errors in your project. // The main requirement is to satisfy the ErrorCode interface by attaching a Code to an Error. // See the documentation of ErrorCode. -// Additional optional interfaces HasClientData and HasOperation are provided for extensibility +// Additional optional interfaces HasClientData, HasOperation, Causer, and StackTracer are provided for extensibility // in creating structured error data representations. // // Hierarchies are supported: a Code can point to a parent. @@ -31,16 +31,20 @@ // A few generic top-level error codes are provided here. // You are encouraged to create your own application customized error codes rather than just using generic errors. // -// See JSONFormat for an opinion on how to send back meta data about errors with the error data to a client. +// See NewJSONFormat for an opinion on how to send back meta data about errors with the error data to a client. // JSONFormat includes a body of response data (the "data field") that is by default the data from the Error // serialized to JSON. -// This package provides no help on versioning error data. +// +// Errors are traced via PreviousErrorCode() which shows up as the Trace field in JSONFormat +// Stack traces are automatically added by NewInternalErr Internal errors StackTrace package errcode import ( "fmt" "net/http" "strings" + + "github.com/pkg/errors" ) // CodeStr is a representation of the type of a particular error. @@ -163,7 +167,7 @@ func (code Code) HTTPCode() int { // For an application specific error with a 1:1 mapping between a go error structure and a RegisteredCode, // You probably want to use this interface directly. Example: // -// // First define a normal error type +// // First define a normal error type // type PathBlocked struct { // start uint64 `json:"start"` // end uint64 `json:"end"` @@ -177,7 +181,7 @@ func (code Code) HTTPCode() int { // // Now define the code // var PathBlockedCode = errcode.StateCode.Child("state.blocked") // -// // Now attach the code to the error type +// // Now attach the code to the error type // func (e PathBlocked) Code() Code { // return PathBlockedCode // } @@ -206,13 +210,22 @@ func ClientData(errCode ErrorCode) interface{} { } // JSONFormat is an opinion on how to serialize an ErrorCode to JSON. -// Msg is the string from Error(). -// The Data field is filled in by GetClientData +// * Code is the error code string (CodeStr) +// * Msg is the string from Error() and should be friendly to end users. +// * Data is the ad-hoc data filled in by GetClientData and should be consumable by clients. +// * Operation is the high-level operation that was happening at the time of the error. +// The Operation field may be missing, and the Data field may be empty. +// +// The rest of the fields may be populated sparsely depending on the application: +// * Previous gives JSONFormat data for an ErrorCode that was wrapped by this one. It relies on the PreviousErrorCode function. +// * Stack is a stack trace. Usually only internal errors populate this. type JSONFormat struct { - Data interface{} `json:"data"` - Msg string `json:"msg"` - Code CodeStr `json:"code"` - Operation string `json:"operation,omitempty"` + Code CodeStr `json:"code"` + Msg string `json:"msg"` + Data interface{} `json:"data"` + Operation string `json:"operation,omitempty"` + Previous *JSONFormat `json:"previous,omitempty"` + Stack errors.StackTrace `json:"stack,omitempty"` } // OperationClientData gives the results of both the ClientData and Operation functions. @@ -231,11 +244,20 @@ func OperationClientData(errCode ErrorCode) (string, interface{}) { // NewJSONFormat turns an ErrorCode into a JSONFormat func NewJSONFormat(errCode ErrorCode) JSONFormat { op, data := OperationClientData(errCode) + + var previous *JSONFormat + if prevCode := PreviousErrorCode(errCode); prevCode != nil { + ptrVar := NewJSONFormat(prevCode) + previous = &ptrVar + } + return JSONFormat{ Data: data, Msg: errCode.Error(), Code: errCode.Code().CodeStr(), Operation: op, + Stack: StackTrace(errCode), + Previous: previous, } } diff --git a/pkg/error_code/error_code_test.go b/pkg/error_code/error_code_test.go index 7ea62dc03ed0..ae45e6374a3c 100644 --- a/pkg/error_code/error_code_test.go +++ b/pkg/error_code/error_code_test.go @@ -14,12 +14,13 @@ package errcode_test import ( - "errors" + "encoding/json" "fmt" "reflect" "testing" "github.com/pingcap/pd/pkg/error_code" + "github.com/pkg/errors" ) // Test setting the HTTP code @@ -202,6 +203,14 @@ func TestNewInvalidInputErr(t *testing.T) { ErrorEquals(t, err, "error") ClientDataEquals(t, err, MinimalError{}, internalCodeStr) + wrappedInternalErr := errcode.NewInternalErr(internalErr) + AssertCode(t, err, internalCodeStr) + AssertHTTPCode(t, err, 500) + ErrorEquals(t, err, "error") + ClientDataEquals(t, wrappedInternalErr, MinimalError{}, internalCodeStr) + // It should use the original stack trace, not the wrapped + AssertStackEquals(t, wrappedInternalErr, errcode.StackTrace(internalErr)) + err = errcode.NewInvalidInputErr(InternalChild{}) AssertCode(t, err, internalChildCodeStr) AssertHTTPCode(t, err, 503) @@ -209,6 +218,18 @@ func TestNewInvalidInputErr(t *testing.T) { ClientDataEquals(t, err, InternalChild{}, internalChildCodeStr) } +func TestStackTrace(t *testing.T) { + internalCodeStr := errcode.CodeStr("internal") + err := errors.New("errors stack") + wrappedInternalErr := errcode.NewInternalErr(err) + AssertCode(t, wrappedInternalErr, internalCodeStr) + AssertHTTPCode(t, wrappedInternalErr, 500) + ErrorEquals(t, err, "errors stack") + ClientDataEquals(t, wrappedInternalErr, err, internalCodeStr) + // It should use the original stack trace, not the wrapped + AssertStackEquals(t, wrappedInternalErr, errcode.StackTrace(err)) +} + func TestNewInternalErr(t *testing.T) { internalCodeStr := errcode.CodeStr("internal") err := errcode.NewInternalErr(errors.New("new error")) @@ -314,17 +335,30 @@ func ClientDataEquals(t *testing.T, code errcode.ErrorCode, data interface{}, co codeStr = codeStrs[0] } t.Helper() - if !reflect.DeepEqual(errcode.ClientData(code), data) { - t.Errorf("\nClientData expected: %#v\n ClientData but got: %#v", data, errcode.ClientData(code)) - } + + jsonEquals(t, "ClientData", data, errcode.ClientData(code)) + jsonExpected := errcode.JSONFormat{ Data: data, Msg: code.Error(), Code: codeStr, Operation: errcode.Operation(data), + Stack: errcode.StackTrace(code), + } + newJSON := errcode.NewJSONFormat(code) + newJSON.Previous = nil + jsonEquals(t, "JSONFormat", jsonExpected, newJSON) +} + +func jsonEquals(t *testing.T, errPrefix string, expectedIn interface{}, gotIn interface{}) { + t.Helper() + got, err1 := json.Marshal(gotIn) + expected, err2 := json.Marshal(expectedIn) + if err1 != nil || err2 != nil { + t.Errorf("%v could not serialize to json", errPrefix) } - if !reflect.DeepEqual(errcode.NewJSONFormat(code), jsonExpected) { - t.Errorf("\nJSON expected: %+v\n JSON but got: %+v", jsonExpected, errcode.NewJSONFormat(code)) + if !reflect.DeepEqual(expected, got) { + t.Errorf("%v\nClientData expected: %#v\n ClientData but got: %#v", errPrefix, expected, got) } } @@ -343,3 +377,11 @@ func AssertOperation(t *testing.T, v interface{}, op string) { t.Errorf("\nOp expected: %#v\n Op but got: %#v", op, opGot) } } + +func AssertStackEquals(t *testing.T, given errcode.ErrorCode, stExpected errors.StackTrace) { + t.Helper() + stGiven := errcode.StackTrace(given) + if stGiven == nil || stExpected == nil || stGiven[0] != stExpected[0] { + t.Errorf("\nStack expected: %#v\n Stack but got: %#v", stExpected[0], stGiven[0]) + } +} diff --git a/pkg/error_code/operation.go b/pkg/error_code/operation.go index 7e796d2ef253..14a119bdc20b 100644 --- a/pkg/error_code/operation.go +++ b/pkg/error_code/operation.go @@ -56,6 +56,11 @@ type OpErrCode struct { Err ErrorCode } +// Cause satisfies the Causer interface +func (e OpErrCode) Cause() error { + return e.Err +} + // Error prefixes the operation to the underlying Err Error. func (e OpErrCode) Error() string { return e.Operation + ": " + e.Err.Error() @@ -79,6 +84,7 @@ func (e OpErrCode) GetClientData() interface{} { var _ ErrorCode = (*OpErrCode)(nil) // assert implements interface var _ HasClientData = (*OpErrCode)(nil) // assert implements interface var _ HasOperation = (*OpErrCode)(nil) // assert implements interface +var _ Causer = (*OpErrCode)(nil) // assert implements interface // AddOp is constructed by Op. It allows method chaining with AddTo. type AddOp func(ErrorCode) OpErrCode @@ -94,7 +100,7 @@ func (addOp AddOp) AddTo(err ErrorCode) OpErrCode { // op := errcode.Op("path.move.x") // if start < obstable && obstacle < end { // return op.AddTo(PathBlocked{start, end, obstacle}) -// } +// } // func Op(operation string) AddOp { return func(err ErrorCode) OpErrCode { diff --git a/pkg/error_code/stack.go b/pkg/error_code/stack.go new file mode 100644 index 000000000000..caa2142e2851 --- /dev/null +++ b/pkg/error_code/stack.go @@ -0,0 +1,107 @@ +// Copyright 2018 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package errcode + +import ( + "github.com/pkg/errors" +) + +// StackTracer is the interface defined but not exported from pkg/errors +// The StackTrace() function (not method) is a preferred way to access the StackTrace +// +// Generally you should only bother with stack traces for internal errors. +type StackTracer interface { + StackTrace() errors.StackTrace +} + +// StackTrace retrieves the errors.StackTrace from the error if it is present. +// If there is not StackTrace it will return nil +// +// StackTrace looks to see if the error is a StackTracer of a Causer that is a StackTracer. +func StackTrace(err error) errors.StackTrace { + if stackTraceErr, ok := err.(StackTracer); ok { + return stackTraceErr.StackTrace() + } + if prev := WrappedError(err); prev != nil { + return StackTrace(prev) + } + return nil +} + +// StackCode is an ErrorCode with stack trace information by attached. +// This may be used as a convenience to record the strack trace information for the error. +// Generally stack traces aren't needed for user errors, but they are provided by NewInternalErr. +// Its also possible to define your own structures that satisfy the StackTracer interface. +type StackCode struct { + Err ErrorCode + GetStackTrace errors.StackTrace +} + +// StackTrace fulfills the StackTracer interface +func (e StackCode) StackTrace() errors.StackTrace { + return e.GetStackTrace +} + +// NewStackCode constrcuts a StackCode, which is an ErrorCode with stack trace information +// The second variable is an optional stack position gets rid of information about function calls to construct the stack trace. +// It is defaulted to 1 to remove this function call. +// +// NewStackCode first looks at the underlying error via WrappedError to see if it already has a StackTrace. +// If so, that StackTrace is used. +func NewStackCode(err ErrorCode, position ...int) StackCode { + stackPosition := 1 + if len(position) > 0 { + stackPosition = position[0] + } + + // if there is an existing trace, take that: it should be deeper + var prev error = err + for prev != nil { + if stackTraceErr, ok := prev.(StackTracer); ok { + return StackCode{Err: err, GetStackTrace: stackTraceErr.StackTrace()} + } + prev = WrappedError(prev) + } + + // we must go through some contortions to get a stack trace from pkg/errors + stackedErr := errors.WithStack(err) + if stackTraceErr, ok := stackedErr.(StackTracer); ok { + return StackCode{Err: err, GetStackTrace: stackTraceErr.StackTrace()[stackPosition:]} + } + panic("NewStackCode: pkg/errors WithStack StackTrace interface changed") +} + +// Cause satisfies the Causer interface +func (e StackCode) Cause() error { + return e.Err +} + +// Error ignores the stack and gives the underlying Err Error. +func (e StackCode) Error() string { + return e.Err.Error() +} + +// Code returns the unerlying Code of Err. +func (e StackCode) Code() Code { + return e.Err.Code() +} + +// GetClientData returns the ClientData of the underlying Err. +func (e StackCode) GetClientData() interface{} { + return ClientData(e.Err) +} + +var _ ErrorCode = (*StackCode)(nil) // assert implements interface +var _ HasClientData = (*StackCode)(nil) // assert implements interface +var _ Causer = (*StackCode)(nil) // assert implements interface diff --git a/pkg/error_code/wrapped.go b/pkg/error_code/wrapped.go new file mode 100644 index 000000000000..079cf38f041d --- /dev/null +++ b/pkg/error_code/wrapped.go @@ -0,0 +1,120 @@ +// Copyright 2018 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package errcode + +// Causer allows the abstract retrieval of the underlying error. +// This is the interface that pkg/errors does not export but is considered part of the stable public API. +// +// Types that wrap errors should implement this to allow viewing of the underlying error. +// Generally you would use this via pkg/errors.Cause or WrappedError. +// PreviousErrorCode and StackTrace use Causer to check if the underlying error is an ErrorCode or a StackTracer. +type Causer interface { + Cause() error +} + +// WrappedError retrieves the wrapped error via Causer. +// Unlike pkg/errors.Cause, it goes just one level and does not recursively traverse. +// Return nil is there is no wrapped error. +func WrappedError(err error) error { + if hasCause, ok := err.(Causer); ok { + return hasCause.Cause() + } + return nil +} + +// EmbedErr is designed to be embedded into your existing error structs. +// It provides the Causer interface already, which can reduce your boilerplate. +type EmbedErr struct { + Err error +} + +// Cause implements the Causer interface +func (e EmbedErr) Cause() error { + return e.Err +} + +var _ Causer = (*EmbedErr)(nil) // assert implements interface + +// PreviousErrorCode looks for a previous ErrorCode that has a different code. +// This helps construct a trace of all previous errors. +// It will return nil if no previous ErrorCode is found. +// +// To look for a previous ErrorCode it looks at Causer to see if they are an ErrorCode. +// Wrappers of errors like OpErrCode and CodedError implement Causer. +func PreviousErrorCode(err ErrorCode) ErrorCode { + return previousErrorCodeCompare(err.Code(), err) +} + +func previousErrorCodeCompare(code Code, err error) ErrorCode { + prev := WrappedError(err) + if prev == nil { + return nil + } + if errcode, ok := prev.(ErrorCode); ok { + if errcode.Code() != code { + return errcode + } + } + return previousErrorCodeCompare(code, prev) +} + +// GetErrorCode tries to convert an error to an ErrorCode. +// It will lookS for the first ErrorCode it can find via Causer. +// In that case it will retain the message of the original error by returning a WrappedErrorCode. +func GetErrorCode(err error) ErrorCode { + if errcode, ok := err.(ErrorCode); ok { + return errcode + } + prev := WrappedError(err) + for prev != nil { + if errcode, ok := prev.(ErrorCode); ok { + return WrappedErrorCode{errcode, err} + } + prev = WrappedError(err) + } + return nil +} + +// WrappedErrorCode is returned by GetErrorCode to retain the full wrapped error message +type WrappedErrorCode struct { + ErrCode ErrorCode + Top error +} + +// Code satisfies the ErrorCode interface +func (err WrappedErrorCode) Code() Code { + return err.ErrCode.Code() +} + +// Error satisfies the Error interface +func (err WrappedErrorCode) Error() string { + return err.Top.Error() +} + +// Cause satisfies the Causer interface +func (err WrappedErrorCode) Cause() error { + if wrapped := WrappedError(err.Top); wrapped != nil { + return wrapped + } + return err.ErrCode +} + +// GetClientData satisfies the HasClientData interface +func (err WrappedErrorCode) GetClientData() interface{} { + return ClientData(err.ErrCode) +} + +var _ ErrorCode = (*WrappedErrorCode)(nil) +var _ HasClientData = (*WrappedErrorCode)(nil) +var _ Causer = (*WrappedErrorCode)(nil) diff --git a/server/api/util.go b/server/api/util.go index 7e10134c44a2..644cba2279ce 100644 --- a/server/api/util.go +++ b/server/api/util.go @@ -40,7 +40,7 @@ func errorResp(rd *render.Render, w http.ResponseWriter, err error) { rd.JSON(w, http.StatusInternalServerError, "nil error") return } - if errCode, ok := errors.Cause(err).(errcode.ErrorCode); ok { + if errCode := errcode.GetErrorCode(err); errCode != nil { w.Header().Set("TiDB-Error-Code", errCode.Code().CodeStr().String()) rd.JSON(w, errCode.Code().HTTPCode(), errcode.NewJSONFormat(errCode)) } else {