diff --git a/api/log/log.go b/api/log/log.go index dc030c39..cdfd2653 100644 --- a/api/log/log.go +++ b/api/log/log.go @@ -39,7 +39,7 @@ func Error(rw http.ResponseWriter, err error) { } var st StackTracedError - if !errors.As(err, &st) { + if errors.As(err, &st) { rl.WithFields(map[string]interface{}{ "stack-trace": fmt.Sprintf("%+v", st.StackTrace()), }) diff --git a/api/log/log_test.go b/api/log/log_test.go index fcd3ea2b..c9ec89f2 100644 --- a/api/log/log_test.go +++ b/api/log/log_test.go @@ -6,11 +6,16 @@ import ( "net/http/httptest" "reflect" "testing" + "unsafe" + + pkgerrors "github.com/pkg/errors" "github.com/smallstep/certificates/logging" ) func TestError(t *testing.T) { + + t.Setenv("STEPDEBUG", "1") // force to end of `Error` function instead of early return theError := errors.New("the error") type args struct { @@ -21,9 +26,10 @@ func TestError(t *testing.T) { name string args args withFields bool + want error }{ - {"normalLogger", args{httptest.NewRecorder(), theError}, false}, - {"responseLogger", args{logging.NewResponseLogger(httptest.NewRecorder()), theError}, true}, + {"normalLogger", args{httptest.NewRecorder(), theError}, false, theError}, + {"responseLogger", args{logging.NewResponseLogger(httptest.NewRecorder()), theError}, true, theError}, } for _, tt := range tests { @@ -32,8 +38,61 @@ func TestError(t *testing.T) { if tt.withFields { if rl, ok := tt.args.rw.(logging.ResponseLogger); ok { fields := rl.Fields() - if !reflect.DeepEqual(fields["error"], theError) { - t.Errorf("ResponseLogger[\"error\"] = %s, wants %s", fields["error"], theError) + if !reflect.DeepEqual(fields["error"], tt.want) { + t.Errorf("ResponseLogger[\"error\"] = %s, wants %s", fields["error"], tt.want) + } + } else { + t.Error("ResponseWriter does not implement logging.ResponseLogger") + } + } + }) + } +} + +type mockStackTracedError struct{} + +func (t *mockStackTracedError) Error() string { + return "a stacktraced error" +} + +func (t *mockStackTracedError) StackTrace() pkgerrors.StackTrace { + f := struct{}{} + return pkgerrors.StackTrace{ // fake stacktrace + pkgerrors.Frame(unsafe.Pointer(&f)), + pkgerrors.Frame(unsafe.Pointer(&f)), + } +} + +func TestError_StackTracedError(t *testing.T) { + + t.Setenv("STEPDEBUG", "1") + aStackTracedError := mockStackTracedError{} + + type args struct { + rw http.ResponseWriter + err error + } + tests := []struct { + name string + args args + withFields bool + want error + }{ + {"responseLoggerWithStackTracedError", args{logging.NewResponseLogger(httptest.NewRecorder()), &aStackTracedError}, true, &aStackTracedError}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + Error(tt.args.rw, tt.args.err) + if tt.withFields { + if rl, ok := tt.args.rw.(logging.ResponseLogger); ok { + fields := rl.Fields() + if !reflect.DeepEqual(fields["error"], tt.want) { + t.Errorf("ResponseLogger[\"error\"] = %s, wants %s", fields["error"], tt.want) + } + // `stack-trace` expected to be set; not interested in actual output + if _, ok := fields["stack-trace"]; !ok { + t.Errorf("ResponseLogger[\"stack-trace\"] not set") } } else { t.Error("ResponseWriter does not implement logging.ResponseLogger")