Add trace information into internal error

Fixes #10839
Add a StackTrace func in to Error, and log it when Harbor gets a internal

Signed-off-by: wang yan <wangyan@vmware.com>

src/lib/errors/const.go

@@ -0,0 +1,66 @@
+package errors
+
+const (
+	// NotFoundCode is code for the error of no object found
+	NotFoundCode = "NOT_FOUND"
+	// ConflictCode ...
+	ConflictCode = "CONFLICT"
+	// UnAuthorizedCode ...
+	UnAuthorizedCode = "UNAUTHORIZED"
+	// BadRequestCode ...
+	BadRequestCode = "BAD_REQUEST"
+	// ForbiddenCode ...
+	ForbiddenCode = "FORBIDDEN"
+	// PreconditionCode ...
+	PreconditionCode = "PRECONDITION"
+	// GeneralCode ...
+	GeneralCode = "UNKNOWN"
+	// DENIED it's used by middleware(readonly, vul and content trust) and returned to docker client to index the request is denied.
+	DENIED = "DENIED"
+	// PROJECTPOLICYVIOLATION ...
+	PROJECTPOLICYVIOLATION = "PROJECTPOLICYVIOLATION"
+	// ViolateForeignKeyConstraintCode is the error code for violating foreign key constraint error
+	ViolateForeignKeyConstraintCode = "VIOLATE_FOREIGN_KEY_CONSTRAINT"
+	// DIGESTINVALID ...
+	DIGESTINVALID = "DIGEST_INVALID"
+)
+
+// NotFoundError is error for the case of object not found
+func NotFoundError(err error) *Error {
+	return New(err).WithCode(NotFoundCode).WithMessage("resource not found")
+}
+
+// ConflictError is error for the case of object conflict
+func ConflictError(err error) *Error {
+	return New(err).WithCode(ConflictCode).WithMessage("resource conflict")
+}
+
+// DeniedError is error for the case of denied
+func DeniedError(err error) *Error {
+	return New(err).WithCode(DENIED).WithMessage("denied")
+}
+
+// UnauthorizedError is error for the case of unauthorized accessing
+func UnauthorizedError(err error) *Error {
+	return New(err).WithCode(UnAuthorizedCode).WithMessage("unauthorized")
+}
+
+// BadRequestError is error for the case of bad request
+func BadRequestError(err error) *Error {
+	return New(err).WithCode(BadRequestCode).WithMessage("bad request")
+}
+
+// ForbiddenError is error for the case of forbidden
+func ForbiddenError(err error) *Error {
+	return New(err).WithCode(ForbiddenCode).WithMessage("forbidden")
+}
+
+// PreconditionFailedError is error for the case of precondition failed
+func PreconditionFailedError(err error) *Error {
+	return New(err).WithCode(PreconditionCode).WithMessage("precondition failed")
+}
+
+// UnknownError ...
+func UnknownError(err error) *Error {
+	return New(err).WithCode(GeneralCode).WithMessage("unknown")
+}

src/lib/errors/errors.go

@@ -35,9 +35,10 @@ type Error struct {
 	Cause   error  `json:"-"`
 	Code    string `json:"code"`
 	Message string `json:"message"`
+	Stack   *stack `json:"-"`
 }
 
-// Error returns a human readable error.
+// Error returns a human readable error, error.Error() will not contains the track information. Needs it? just call error.StackTrace()
 func (e *Error) Error() string {
 	var parts []string
 
@@ -58,6 +59,11 @@ func (e *Error) Error() string {
 	return strings.Join(parts, ", ")
 }
 
+// StackTrace ...
+func (e *Error) StackTrace() string {
+	return e.Stack.frames().format()
+}
+
 // WithMessage ...
 func (e *Error) WithMessage(format string, v ...interface{}) *Error {
 	e.Message = fmt.Sprintf(format, v...)
@@ -120,31 +126,6 @@ func NewErrs(err error) Errors {
 	return Errors{err}
 }
 
-const (
-	// NotFoundCode is code for the error of no object found
-	NotFoundCode = "NOT_FOUND"
-	// ConflictCode ...
-	ConflictCode = "CONFLICT"
-	// UnAuthorizedCode ...
-	UnAuthorizedCode = "UNAUTHORIZED"
-	// BadRequestCode ...
-	BadRequestCode = "BAD_REQUEST"
-	// ForbiddenCode ...
-	ForbiddenCode = "FORBIDDEN"
-	// PreconditionCode ...
-	PreconditionCode = "PRECONDITION"
-	// GeneralCode ...
-	GeneralCode = "UNKNOWN"
-	// DENIED it's used by middleware(readonly, vul and content trust) and returned to docker client to index the request is denied.
-	DENIED = "DENIED"
-	// PROJECTPOLICYVIOLATION ...
-	PROJECTPOLICYVIOLATION = "PROJECTPOLICYVIOLATION"
-	// ViolateForeignKeyConstraintCode is the error code for violating foreign key constraint error
-	ViolateForeignKeyConstraintCode = "VIOLATE_FOREIGN_KEY_CONSTRAINT"
-	// DIGESTINVALID ...
-	DIGESTINVALID = "DIGEST_INVALID"
-)
-
 // New ...
 func New(in interface{}) *Error {
 	var err error
@@ -159,6 +140,7 @@ func New(in interface{}) *Error {
 	return &Error{
 		Cause:   err,
 		Message: err.Error(),
+		Stack:   newStack(),
 	}
 }
 
@@ -170,6 +152,7 @@ func Wrap(err error, message string) *Error {
 	e := &Error{
 		Cause:   err,
 		Message: message,
+		Stack:   newStack(),
 	}
 	return e
 }
@@ -181,6 +164,7 @@ func Wrapf(err error, format string, args ...interface{}) *Error {
 	}
 	e := &Error{
 		Cause: err,
+		Stack: newStack(),
 	}
 	return e.WithMessage(format, args...)
 }
@@ -192,46 +176,6 @@ func Errorf(format string, args ...interface{}) *Error {
 	}
 }
 
-// NotFoundError is error for the case of object not found
-func NotFoundError(err error) *Error {
-	return New(err).WithCode(NotFoundCode).WithMessage("resource not found")
-}
-
-// ConflictError is error for the case of object conflict
-func ConflictError(err error) *Error {
-	return New(err).WithCode(ConflictCode).WithMessage("resource conflict")
-}
-
-// DeniedError is error for the case of denied
-func DeniedError(err error) *Error {
-	return New(err).WithCode(DENIED).WithMessage("denied")
-}
-
-// UnauthorizedError is error for the case of unauthorized accessing
-func UnauthorizedError(err error) *Error {
-	return New(err).WithCode(UnAuthorizedCode).WithMessage("unauthorized")
-}
-
-// BadRequestError is error for the case of bad request
-func BadRequestError(err error) *Error {
-	return New(err).WithCode(BadRequestCode).WithMessage("bad request")
-}
-
-// ForbiddenError is error for the case of forbidden
-func ForbiddenError(err error) *Error {
-	return New(err).WithCode(ForbiddenCode).WithMessage("forbidden")
-}
-
-// PreconditionFailedError is error for the case of precondition failed
-func PreconditionFailedError(err error) *Error {
-	return New(err).WithCode(PreconditionCode).WithMessage("precondition failed")
-}
-
-// UnknownError ...
-func UnknownError(err error) *Error {
-	return New(err).WithCode(GeneralCode).WithMessage("unknown")
-}
-
 // Cause gets the root error
 func Cause(err error) error {
 	for err != nil {

src/lib/errors/errors_test.go

@@ -44,6 +44,31 @@ func TestErrCode(t *testing.T) {
 	}
 }
 
+const (
+	caller1Stack = "errors.caller1"
+	caller2Stack = "errors.caller2"
+	caller3Stack = "errors.caller3"
+)
+
+func caller1() error {
+	err := caller2()
+	return err
+}
+
+func caller2() error {
+	err := caller3()
+	return err
+}
+
+func caller3() error {
+	err := caller4()
+	return New(err).WithMessage("it's caller 3.")
+}
+
+func caller4() error {
+	return errors.New("it's caller 4")
+}
+
 type ErrorTestSuite struct {
 	suite.Suite
 }
@@ -55,6 +80,15 @@ func (suite *ErrorTestSuite) TestNewCompatibleWithStdlib() {
 	suite.Equal(err2.Error(), err1.Error())
 }
 
+func (suite *ErrorTestSuite) TestStackTrace() {
+	err := caller1()
+	suite.Contains(err.(*Error).StackTrace(), caller1Stack)
+	suite.Contains(err.(*Error).StackTrace(), caller2Stack)
+	suite.Contains(err.(*Error).StackTrace(), caller3Stack)
+	suite.Contains(err.Error(), "it's caller 3.")
+	suite.Contains(err.Error(), "it's caller 4")
+}
+
 func TestErrorTestSuite(t *testing.T) {
 	suite.Run(t, &ErrorTestSuite{})
 }

src/lib/errors/stack.go

@@ -0,0 +1,48 @@
+package errors
+
+import (
+	"fmt"
+	"runtime"
+	"strings"
+)
+
+const maxDeepth = 50
+
+type stack []uintptr
+
+func (s *stack) frames() StackFrames {
+	var stackFrames StackFrames
+	frames := runtime.CallersFrames(*s)
+	for {
+		frame, next := frames.Next()
+		// filter out runtime
+		if !strings.Contains(frame.File, "runtime/") {
+			stackFrames = append(stackFrames, frame)
+		}
+		if !next {
+			break
+		}
+	}
+	return stackFrames
+}
+
+// newStack ...
+func newStack() *stack {
+	var pcs [maxDeepth]uintptr
+	n := runtime.Callers(3, pcs[:])
+	var st stack = pcs[0:n]
+	return &st
+}
+
+// StackFrames ...
+// ToDo we can define an Harbor frame to customize trace message, but it depends on requirement
+type StackFrames []runtime.Frame
+
+// Output: <File>:<Line>, <Method>
+func (frames StackFrames) format() string {
+	var msg string
+	for _, frame := range frames {
+		msg = msg + fmt.Sprintf("\n%v:%v, %v", frame.File, frame.Line, frame.Function)
+	}
+	return msg
+}

src/lib/errors/stack_test.go

@@ -0,0 +1,30 @@
+package errors
+
+import (
+	"fmt"
+	"github.com/stretchr/testify/suite"
+	"testing"
+)
+
+type stackTestSuite struct {
+	suite.Suite
+}
+
+func (c *stackTestSuite) SetupTest() {}
+
+func (c *stackTestSuite) TestFrame() {
+	stack := newStack()
+	frames := stack.frames()
+	c.Equal(len(frames), 4)
+	fmt.Println(frames.format())
+}
+
+func (c *stackTestSuite) TestFormat() {
+	stack := newStack()
+	frames := stack.frames()
+	c.Contains(frames[len(frames)-1].Function, "testing.tRunner")
+}
+
+func TestStackTestSuite(t *testing.T) {
+	suite.Run(t, &stackTestSuite{})
+}

src/lib/log/logger.go

@@ -330,6 +330,7 @@ func line(callDepth int) string {
 	_, file, line, ok := runtime.Caller(callDepth)
 	if !ok {
 		file = "???"
+
 		line = 0
 	}
 	l := strings.SplitN(file, srcSeparator, 2)

src/server/error/error.go

@@ -47,10 +47,11 @@ var (
 // And the error is logged as well
 func SendError(w http.ResponseWriter, err error) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
-	statusCode, errPayload := apiError(err)
+	statusCode, errPayload, stackTrace := apiError(err)
 	// the error detail is logged only, and will not be sent to the client to avoid leaking server information
 	if statusCode >= http.StatusInternalServerError {
 		log.Error(errPayload)
+		log.Debug(stackTrace)
 		err = errors.New(nil).WithCode(errors.GeneralCode).WithMessage("internal server error")
 		errPayload = errors.NewErrs(err).Error()
 	} else {
@@ -63,7 +64,7 @@ func SendError(w http.ResponseWriter, err error) {
 
 // generates the HTTP status code based on the specified error,
 // envelops the error into an error array as the payload and return them
-func apiError(err error) (statusCode int, errPayload string) {
+func apiError(err error) (statusCode int, errPayload, stackTrace string) {
 	code := 0
 	var openAPIErr openapi.Error
 	if errors.As(err, &openAPIErr) {
@@ -85,7 +86,11 @@ func apiError(err error) (statusCode int, errPayload string) {
 	if code == 0 {
 		code = http.StatusInternalServerError
 	}
-	return code, errors.NewErrs(err).Error()
+	fullStack := ""
+	if _, ok := err.(*errors.Error); ok {
+		fullStack = err.(*errors.Error).StackTrace()
+	}
+	return code, errors.NewErrs(err).Error(), fullStack
 }
 
 var _ middleware.Responder = &ErrResponder{}

src/server/error/error_test.go

@@ -18,6 +18,7 @@ import (
 	openapi "github.com/go-openapi/errors"
 	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/lib/errors"
+	pkg_errors "github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"net/http"
 	"net/http/httptest"
@@ -51,32 +52,33 @@ func TestAPIError(t *testing.T) {
 	var err error
 	// open API error: github.com/go-openapi/errors.Error
 	err = openapi.New(400, "bad request")
-	statusCode, payload := apiError(err)
+	statusCode, payload, stacktrace := apiError(err)
 	assert.Equal(t, http.StatusBadRequest, statusCode)
 	assert.Equal(t, `{"errors":[{"code":"BAD_REQUEST","message":"bad request"}]}`, payload)
+	assert.Contains(t, stacktrace, `error.apiError`)
 
 	// legacy error
 	err = &commonhttp.Error{
 		Code:    http.StatusNotFound,
 		Message: "not found",
 	}
-	statusCode, payload = apiError(err)
+	statusCode, payload, stacktrace = apiError(err)
 	assert.Equal(t, http.StatusNotFound, statusCode)
 	assert.Equal(t, `{"errors":[{"code":"NOT_FOUND","message":"not found"}]}`, payload)
+	assert.Contains(t, stacktrace, `error.apiError`)
 
 	// errors.Error
-	err = &errors.Error{
-		Cause:   nil,
-		Code:    errors.NotFoundCode,
-		Message: "resource not found",
-	}
-	statusCode, payload = apiError(err)
+	err = errors.New(nil).WithCode(errors.NotFoundCode).WithMessage("resource not found")
+	statusCode, payload, stacktrace = apiError(err)
 	assert.Equal(t, http.StatusNotFound, statusCode)
 	assert.Equal(t, `{"errors":[{"code":"NOT_FOUND","message":"resource not found"}]}`, payload)
+	assert.Contains(t, stacktrace, `error.TestAPIError`)
 
-	// common error
-	e := errors.New("customized error")
-	statusCode, payload = apiError(e)
+	// common error, common error has no stacktrace
+	e := pkg_errors.New("customized error")
+	statusCode, payload, stacktrace = apiError(e)
 	assert.Equal(t, http.StatusInternalServerError, statusCode)
 	assert.Equal(t, `{"errors":[{"code":"UNKNOWN","message":"customized error"}]}`, payload)
+	assert.Contains(t, stacktrace, ``)
+
 }