alexvanin/certificates
1
0
Fork 0
forked from TrueCloudLab/certificates
Code Pull requests Activity

Fix StackTracedError logging

Browse source 
When running with `STEPDEBUG=1`, a response with a `StackTracedError`
would result in a nil pointer error. This commit fixes the check and
adds a test case.
This commit is contained in:
Herman Slatman 2022-11-17 12:29:54 +01:00
parent 8a2e49a1e3
commit 362be72120
No known key found for this signature in database
GPG key ID: F4D8A44EA0A75A4F
2 changed files with 64 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
api/log/log.go
View file

@ -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()),
})

67
api/log/log_test.go
View file

@ -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")