diff --git a/http_logging_test.go b/http_logging_test.go index 09558ab..e7785be 100644 --- a/http_logging_test.go +++ b/http_logging_test.go @@ -30,8 +30,8 @@ func TestLogRequest(t *testing.T) { w.Flush() actual := b.String() //nolint: lll - expected := `{"level":"info","msg":"test1","method":"","url":"","proto":"","remote_addr":"","host":"foo-host:123","header.x-forwarded-proto":"","header.x-forwarded-for":"","user-agent":"","tls":true,"tls.peer_cn":"x\nyz","header.foo":"bar1,bar2"} -{"level":"info","msg":"test2","method":"","url":"","proto":"","remote_addr":"","host":"foo-host:123","header.x-forwarded-proto":"","header.x-forwarded-for":"","user-agent":"","extra1":"v1","extra2":"v2"} + expected := `{"level":"info","msg":"test1","method":"","url":null,"proto":"","remote_addr":"","host":"foo-host:123","header.x-forwarded-proto":"","header.x-forwarded-for":"","user-agent":"","tls":true,"tls.peer_cn":"x\nyz","header.foo":"bar1,bar2"} +{"level":"info","msg":"test2","method":"","url":null,"proto":"","remote_addr":"","host":"foo-host:123","header.x-forwarded-proto":"","header.x-forwarded-for":"","user-agent":"","extra1":"v1","extra2":"v2"} ` if actual != expected { t.Errorf("unexpected:\n%s\nvs:\n%s\n", actual, expected) diff --git a/logger.go b/logger.go index 5499121..c80c13e 100644 --- a/logger.go +++ b/logger.go @@ -25,6 +25,7 @@ package log // import "fortio.org/log" import ( "bytes" + "encoding/json" "flag" "fmt" "io" @@ -32,7 +33,6 @@ import ( "math" "os" "runtime" - "sort" "strconv" "strings" "sync" @@ -589,39 +589,18 @@ type ValueType[T ValueTypes] struct { Val T } -func arrayToString(s []interface{}) string { - var buf strings.Builder - buf.WriteString("[") - for i, e := range s { - if i != 0 { - buf.WriteString(",") - } - vv := ValueType[interface{}]{Val: e} - buf.WriteString(vv.String()) - } - buf.WriteString("]") - return buf.String() -} - -func mapToString(s map[string]interface{}) string { - var buf strings.Builder - buf.WriteString("{") - keys := make([]string, 0, len(s)) - for k := range s { - keys = append(keys, k) +func toJSON(v any) string { + bytes, err := json.Marshal(v) + if err != nil { + return strconv.Quote(fmt.Sprintf("ERR marshaling %v: %v", v, err)) } - sort.Strings(keys) - for i, k := range keys { - if i != 0 { - buf.WriteString(",") - } - buf.WriteString(fmt.Sprintf("%q", k)) - buf.WriteString(":") - vv := ValueType[interface{}]{Val: s[k]} - buf.WriteString(vv.String()) + str := string(bytes) + // This is kinda hacky way to handle both structured and custom serialization errors, and + // struct with no public fields for which we need to call Error() to get a useful string. + if e, isError := v.(error); isError && str == "{}" { + return fmt.Sprintf("%q", e.Error()) } - buf.WriteString("}") - return buf.String() + return str } func (v ValueType[T]) String() string { @@ -629,13 +608,12 @@ func (v ValueType[T]) String() string { switch s := any(v.Val).(type) { case bool, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64: - return fmt.Sprint(v.Val) - case []interface{}: - return arrayToString(s) - case map[string]interface{}: - return mapToString(s) + return fmt.Sprint(s) + case string: + return fmt.Sprintf("%q", s) + /* It's all handled by json fallback now even though slightly more expensive at runtime, it's a lot simpler */ default: - return fmt.Sprintf("%q", fmt.Sprint(v.Val)) + return toJSON(v.Val) // was fmt.Sprintf("%q", fmt.Sprint(v.Val)) } } diff --git a/logger_test.go b/logger_test.go index ed5b5db..cf24fe1 100644 --- a/logger_test.go +++ b/logger_test.go @@ -752,6 +752,52 @@ func TestNoLevel(t *testing.T) { } } +type customError struct { + Msg string + Code int +} + +func (e customError) Error() string { + return fmt.Sprintf("custom error %s (code %d)", e.Msg, e.Code) +} + +func TestSerializationOfError(t *testing.T) { + var err error + kv := Any("err", err) + kvStr := kv.StringValue() + expected := `null` + if kvStr != expected { + t.Errorf("unexpected:\n%s\nvs:\n%s\n", kvStr, expected) + } + err = fmt.Errorf("test error") + Errf("Error on purpose: %v", err) + S(Error, "Error on purpose", Any("err", err)) + kv = Any("err", err) + kvStr = kv.StringValue() + expected = `"test error"` + if kvStr != expected { + t.Errorf("unexpected:\n%s\nvs:\n%s\n", kvStr, expected) + } + err = customError{Msg: "custom error", Code: 42} + kv = Any("err", err) + kvStr = kv.StringValue() + expected = `{"Msg":"custom error","Code":42}` + if kvStr != expected { + t.Errorf("unexpected:\n%s\nvs:\n%s\n", kvStr, expected) + } +} + +func TestToJSON_MarshalError(t *testing.T) { + badValue := make(chan int) + + expected := fmt.Sprintf("\"ERR marshaling %v: %v\"", badValue, "json: unsupported type: chan int") + actual := toJSON(badValue) + + if actual != expected { + t.Errorf("Expected %q, got %q", expected, actual) + } +} + // io.Discard but specially known to by logger optimizations for instance. type discard struct{}