Skip to content

Commit

Permalink
integrate with pkg/errors
Browse files Browse the repository at this point in the history
add the Causer interface
internal errors record a StackTrace
  • Loading branch information
gregwebs committed Aug 16, 2018
1 parent 5fc39c6 commit b70f5f6
Show file tree
Hide file tree
Showing 7 changed files with 331 additions and 22 deletions.
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 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.
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"`
}

// 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
56 changes: 50 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,32 @@ 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) {
// TODO: change the test to actually look at JSON serialization
// otherwise error/stack show up
t.Errorf("%v\nClientData expected: %#v\n ClientData but got: %#v", errPrefix, expected, got)
}
}

Expand All @@ -343,3 +379,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
107 changes: 107 additions & 0 deletions pkg/error_code/stack.go
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit b70f5f6

Please sign in to comment.