Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

integrate error codes with pkg/errors to provide error tracing #1200

Merged
merged 26 commits into from
Sep 30, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
bc61230
add pkg/errors to Gopkg
gregwebs Aug 16, 2018
7a5bbab
integrate with pkg/errors
gregwebs Aug 15, 2018
4b604c5
Merge remote-tracking branch 'main/master' into gregwebs/error-code-t…
gregwebs Aug 24, 2018
8a0ced9
fix comment spellings
gregwebs Aug 27, 2018
c3c9a11
search for the deepest available stack trace
gregwebs Aug 27, 2018
c4a46a3
re-use the existing StackTrace function
gregwebs Aug 28, 2018
3c88460
only internal errors should show a stack trace
gregwebs Aug 28, 2018
48e18df
Merge remote-tracking branch 'main/master' into gregwebs/error-code-t…
gregwebs Aug 28, 2018
acb0499
update documentation
gregwebs Aug 28, 2018
4314b76
Merge branch 'master' into gregwebs/error-code-tracing
gregwebs Sep 6, 2018
35dc1b0
Upgrade pkg/errors to latest pingcap/errors
gregwebs Sep 8, 2018
a4d546f
use stack code in pingcap/errors
gregwebs Sep 8, 2018
70edcec
Merge branch 'master' into gregwebs/error-code-tracing
gregwebs Sep 8, 2018
ee73eb5
use errors.Unwrap
gregwebs Sep 8, 2018
b79940d
error grouping
gregwebs Aug 15, 2018
b026410
upgrade pingcap/errors
gregwebs Sep 11, 2018
ff5c090
use the latest from pingcap/errors
gregwebs Sep 11, 2018
4d88fdf
Merge remote-tracking branch 'main/master' into gregwebs/error-code-t…
gregwebs Sep 11, 2018
99af76d
bugfix
gregwebs Sep 11, 2018
dee08a8
update to latest pingcap/errors (docs update)
gregwebs Sep 21, 2018
84a95c4
Merge remote-tracking branch 'main/master' into gregwebs/error-code-t…
gregwebs Sep 21, 2018
71fd836
Merge remote-tracking branch 'main/master' into gregwebs/error-code-t…
gregwebs Sep 25, 2018
64bcff4
vendor: errcode is now its own package
gregwebs Sep 25, 2018
8fcb0b4
switch to separate package for errcode
gregwebs Sep 25, 2018
55f3967
fix gofmt
gregwebs Sep 26, 2018
6ef458d
Merge branch 'master' into gregwebs/error-code-tracing
gregwebs Sep 29, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 13 additions & 3 deletions pkg/error_code/codes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 }
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
44 changes: 33 additions & 11 deletions pkg/error_code/error_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 Previous field in JSONFormat.
// Stack traces are automatically added by NewInternalErr and show up as the Stack field in JSONFormat.
package errcode

import (
"fmt"
"net/http"
"strings"

"github.com/pkg/errors"
)

// CodeStr is a representation of the type of a particular error.
Expand Down Expand Up @@ -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"`
Expand All @@ -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
// }
Expand Down Expand Up @@ -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"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you show me an example about after marshal? And so many filed, do we need such detailed information?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per the documentation Previous is normally empty. Actually, now that we switched error packages, Stack will be present. I will add a change to remove it for non-internal errors. After that, there is no change for non-internal errors in PD after this PR. Previous is designed to be most useful in the future for the case of seeing the original error returned from another service (e.g. TiKV or TiDB SQL contacting PD, etc).

}

// OperationClientData gives the results of both the ClientData and Operation functions.
Expand All @@ -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,
}
}

Expand Down
54 changes: 48 additions & 6 deletions pkg/error_code/error_code_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -202,13 +203,33 @@ 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)
ErrorEquals(t, err, "internal child error")
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"))
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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])
}
}
8 changes: 7 additions & 1 deletion pkg/error_code/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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 {
Expand Down
Loading