From 8e354c075ab9cdbbff410ac5e734416e366e0002 Mon Sep 17 00:00:00 2001 From: Dwiyan Rahmanianto Date: Fri, 4 Mar 2022 21:21:03 +0700 Subject: [PATCH 1/2] fix safeError & safeString for json format --- json_logger.go | 4 ++-- json_logger_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/json_logger.go b/json_logger.go index 0cedbf8..9c95beb 100644 --- a/json_logger.go +++ b/json_logger.go @@ -68,7 +68,7 @@ func safeString(str fmt.Stringer) (s string) { if v := reflect.ValueOf(str); v.Kind() == reflect.Ptr && v.IsNil() { s = "NULL" } else { - panic(panicVal) + s = fmt.Sprintf("panic stringer value: %v", panicVal) } } }() @@ -82,7 +82,7 @@ func safeError(err error) (s interface{}) { if v := reflect.ValueOf(err); v.Kind() == reflect.Ptr && v.IsNil() { s = nil } else { - panic(panicVal) + s = fmt.Sprintf("panic error value: %v", panicVal) } } }() diff --git a/json_logger_test.go b/json_logger_test.go index cb049dd..1be4a61 100644 --- a/json_logger_test.go +++ b/json_logger_test.go @@ -60,6 +60,19 @@ func TestJSONLoggerNilStringerKey(t *testing.T) { } } +func TestJSONLoggerPanicStringerValue(t *testing.T) { + t.Parallel() + + buf := &bytes.Buffer{} + logger := log.NewJSONLogger(buf) + if err := logger.Log("k", unsafeStringer{}); err != nil { + t.Fatal(err) + } + if want, have := `{"k":"panic stringer value: error"}`+"\n", buf.String(); want != have { + t.Errorf("\nwant %#v\nhave %#v", want, have) + } +} + func TestJSONLoggerNilErrorValue(t *testing.T) { t.Parallel() @@ -73,6 +86,19 @@ func TestJSONLoggerNilErrorValue(t *testing.T) { } } +func TestJSONLoggerPanicErrorValue(t *testing.T) { + t.Parallel() + + buf := &bytes.Buffer{} + logger := log.NewJSONLogger(buf) + if err := logger.Log("err", unsafeError{}); err != nil { + t.Fatal(err) + } + if want, have := `{"err":"panic error value: error"}`+"\n", buf.String(); want != have { + t.Errorf("\nwant %#v\nhave %#v", want, have) + } +} + func TestJSONLoggerNoHTMLEscape(t *testing.T) { t.Parallel() buf := &bytes.Buffer{} @@ -160,6 +186,18 @@ func (s stringError) Error() string { return string(s) } +type unsafeStringer struct{} + +func (s unsafeStringer) String() string { + panic("error") +} + +type unsafeError struct{} + +func (s unsafeError) Error() string { + panic("error") +} + func BenchmarkJSONLoggerSimple(b *testing.B) { benchmarkRunner(b, log.NewJSONLogger(ioutil.Discard), baseMessage) } From 419972698d89b7749b104a16d817d106bf4453c3 Mon Sep 17 00:00:00 2001 From: Dwiyan Rahmanianto Date: Tue, 15 Mar 2022 19:47:56 +0700 Subject: [PATCH 2/2] improve the error message when panic inside safe wrapper method --- json_logger.go | 4 ++-- json_logger_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/json_logger.go b/json_logger.go index 9c95beb..d0faed4 100644 --- a/json_logger.go +++ b/json_logger.go @@ -68,7 +68,7 @@ func safeString(str fmt.Stringer) (s string) { if v := reflect.ValueOf(str); v.Kind() == reflect.Ptr && v.IsNil() { s = "NULL" } else { - s = fmt.Sprintf("panic stringer value: %v", panicVal) + s = fmt.Sprintf("PANIC in String method: %v", panicVal) } } }() @@ -82,7 +82,7 @@ func safeError(err error) (s interface{}) { if v := reflect.ValueOf(err); v.Kind() == reflect.Ptr && v.IsNil() { s = nil } else { - s = fmt.Sprintf("panic error value: %v", panicVal) + s = fmt.Sprintf("PANIC in Error method: %v", panicVal) } } }() diff --git a/json_logger_test.go b/json_logger_test.go index 1be4a61..a7ef4ea 100644 --- a/json_logger_test.go +++ b/json_logger_test.go @@ -68,7 +68,7 @@ func TestJSONLoggerPanicStringerValue(t *testing.T) { if err := logger.Log("k", unsafeStringer{}); err != nil { t.Fatal(err) } - if want, have := `{"k":"panic stringer value: error"}`+"\n", buf.String(); want != have { + if want, have := `{"k":"PANIC in String method: error"}`+"\n", buf.String(); want != have { t.Errorf("\nwant %#v\nhave %#v", want, have) } } @@ -94,7 +94,7 @@ func TestJSONLoggerPanicErrorValue(t *testing.T) { if err := logger.Log("err", unsafeError{}); err != nil { t.Fatal(err) } - if want, have := `{"err":"panic error value: error"}`+"\n", buf.String(); want != have { + if want, have := `{"err":"PANIC in Error method: error"}`+"\n", buf.String(); want != have { t.Errorf("\nwant %#v\nhave %#v", want, have) } }