Skip to content

Commit 9df5e0a

Browse files
fix!: handle sql/driver.Valuer types properly in slogjson (#219)
Currently, if a field like `sql.NullInt32` has `Valid: False`, `sloghuman` will export it's value as `<nil>`, regardless of it's `String`. This is because it checks `(driver.Valuer).Value()`. However, `slogjson` currently sets the value to the json string of the raw struct: ```json { "fields": { "Code": "{Int32:0 Valid:false}", "ValidCode": "{Int32:12 Valid:true}" } } ``` This PR handles this case by first checking if the type implements `sql/driver.Valuer`. If `Valid` is `false` then a JSON `null` value is produced: ```json { "fields": { "Code": null, "ValidCode": 12 } } ``` This matches the behaviour of `sloghuman`. This is technically a breaking change, as these types are now `T | null` instead of `String`, where `T` is the corresponding JSON type of `sql.Null<V>`
1 parent 0ec81e6 commit 9df5e0a

File tree

3 files changed

+40
-4
lines changed

3 files changed

+40
-4
lines changed

map.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package slog
22

33
import (
44
"bytes"
5+
"database/sql/driver"
56
"encoding/json"
67
"fmt"
78
"reflect"
@@ -70,6 +71,14 @@ func marshalList(rv reflect.Value) []byte {
7071
}
7172

7273
func encode(v interface{}) []byte {
74+
if vr, ok := v.(driver.Valuer); ok {
75+
var err error
76+
v, err = vr.Value()
77+
if err != nil {
78+
return encodeJSON(fmt.Sprintf("error calling Value: %v", err))
79+
}
80+
}
81+
7382
switch v := v.(type) {
7483
case json.Marshaler:
7584
return encodeJSON(v)

map_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,12 @@ func TestMap(t *testing.T) {
6262
{
6363
"msg": "wrap1",
6464
"fun": "cdr.dev/slog_test.TestMap.func2",
65-
"loc": "`+mapTestFile+`:41"
65+
"loc": "`+mapTestFile+`:41"
6666
},
6767
{
6868
"msg": "wrap2",
6969
"fun": "cdr.dev/slog_test.TestMap.func2",
70-
"loc": "`+mapTestFile+`:42"
70+
"loc": "`+mapTestFile+`:42"
7171
},
7272
"EOF"
7373
],
@@ -93,7 +93,7 @@ func TestMap(t *testing.T) {
9393
{
9494
"msg": "failed to marshal to JSON",
9595
"fun": "cdr.dev/slog.encodeJSON",
96-
"loc": "`+mapTestFile+`:131"
96+
"loc": "`+mapTestFile+`:140"
9797
},
9898
"json: error calling MarshalJSON for type slog_test.complexJSON: json: unsupported type: complex128"
9999
],

sloggers/slogjson/slogjson_test.go

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package slogjson_test
33
import (
44
"bytes"
55
"context"
6+
"database/sql"
67
"fmt"
78
"runtime"
89
"testing"
@@ -33,7 +34,33 @@ func TestMake(t *testing.T) {
3334
l.Error(ctx, "line1\n\nline2", slog.F("wowow", "me\nyou"))
3435

3536
j := entryjson.Filter(b.String(), "ts")
36-
exp := fmt.Sprintf(`{"level":"ERROR","msg":"line1\n\nline2","caller":"%v:33","func":"cdr.dev/slog/sloggers/slogjson_test.TestMake","logger_names":["named"],"trace":"%v","span":"%v","fields":{"wowow":"me\nyou"}}
37+
exp := fmt.Sprintf(`{"level":"ERROR","msg":"line1\n\nline2","caller":"%v:34","func":"cdr.dev/slog/sloggers/slogjson_test.TestMake","logger_names":["named"],"trace":"%v","span":"%v","fields":{"wowow":"me\nyou"}}
3738
`, slogjsonTestFile, span.SpanContext().TraceID().String(), span.SpanContext().SpanID().String())
3839
assert.Equal(t, "entry", exp, j)
3940
}
41+
42+
func TestNoDriverValue(t *testing.T) {
43+
t.Parallel()
44+
45+
b := &bytes.Buffer{}
46+
l := slog.Make(slogjson.Sink(b))
47+
l = l.Named("named")
48+
validField := sql.NullString{
49+
String: "cat",
50+
Valid: true,
51+
}
52+
invalidField := sql.NullString{
53+
String: "dog",
54+
Valid: false,
55+
}
56+
validInt := sql.NullInt64{
57+
Int64: 42,
58+
Valid: true,
59+
}
60+
l.Error(bg, "error!", slog.F("inval", invalidField), slog.F("val", validField), slog.F("int", validInt))
61+
62+
j := entryjson.Filter(b.String(), "ts")
63+
exp := fmt.Sprintf(`{"level":"ERROR","msg":"error!","caller":"%v:60","func":"cdr.dev/slog/sloggers/slogjson_test.TestNoDriverValue","logger_names":["named"],"fields":{"inval":null,"val":"cat","int":42}}
64+
`, slogjsonTestFile)
65+
assert.Equal(t, "entry", exp, j)
66+
}

0 commit comments

Comments
 (0)