Skip to content

Commit

Permalink
funcr: Handle nil Stringer, Marshaler, error
Browse files Browse the repository at this point in the history
Produce a useful error rather thn panic.
  • Loading branch information
thockin committed Jan 19, 2022
1 parent eb02c45 commit a58a296
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 11 deletions.
33 changes: 28 additions & 5 deletions funcr/funcr.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,17 @@ const (
flagRawStruct = 0x1 // do not print braces on structs
)

func isNil(i interface{}) bool {
if i == nil {
return true
}
switch reflect.TypeOf(i).Kind() {
case reflect.Ptr, reflect.Map, reflect.Array, reflect.Chan, reflect.Slice:
return reflect.ValueOf(i).IsNil()
}
return false
}

// TODO: This is not fast. Most of the overhead goes here.
func (f Formatter) prettyWithFlags(value interface{}, flags uint32, depth int) string {
if depth > f.opts.MaxLogDepth {
Expand All @@ -349,17 +360,29 @@ func (f Formatter) prettyWithFlags(value interface{}, flags uint32, depth int) s

// Handle types that take full control of logging.
if v, ok := value.(logr.Marshaler); ok {
// Replace the value with what the type wants to get logged.
// That then gets handled below via reflection.
value = v.MarshalLog()
if isNil(v) {
value = "<nil-logr-marshaler>"
} else {
// Replace the value with what the type wants to get logged.
// That then gets handled below via reflection.
value = v.MarshalLog()
}
}

// Handle types that want to format themselves.
switch v := value.(type) {
case fmt.Stringer:
value = v.String()
if isNil(v) {
value = "<nil-fmt-stringer>"
} else {
value = v.String()
}
case error:
value = v.Error()
if isNil(v) {
value = "<nil-error>"
} else {
value = v.Error()
}
}

// Handling the most common types without reflect is a small perf win.
Expand Down
43 changes: 37 additions & 6 deletions funcr/funcr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (p pointErr) MarshalText() ([]byte, error) {
}

// Logging this should result in the MarshalLog() value.
type Tmarshaler string
type Tmarshaler struct{ val string }

func (t Tmarshaler) MarshalLog() interface{} {
return struct{ Inner string }{"I am a logr.Marshaler"}
Expand All @@ -67,7 +67,7 @@ func (t Tmarshaler) Error() string {
}

// Logging this should result in the String() value.
type Tstringer string
type Tstringer struct{ val string }

func (t Tstringer) String() string {
return "I am a fmt.Stringer"
Expand All @@ -77,6 +77,13 @@ func (t Tstringer) Error() string {
return "Error(): you should not see this"
}

// Logging this should result in the Error() value.
type Terror struct{ val string }

func (t Terror) Error() string {
return "I am an error"
}

type TjsontagsString struct {
String1 string `json:"string1"` // renamed
String2 string `json:"-"` // ignored
Expand Down Expand Up @@ -351,16 +358,40 @@ func TestPretty(t *testing.T) {
},
},
{
val: Tmarshaler("foobar"),
val: Tmarshaler{"foobar"},
exp: `{"Inner":"I am a logr.Marshaler"}`,
},
{
val: Tstringer("foobar"),
val: &Tmarshaler{"foobar"},
exp: `{"Inner":"I am a logr.Marshaler"}`,
},
{
val: (*Tmarshaler)(nil),
exp: `"<nil-logr-marshaler>"`,
},
{
val: Tstringer{"foobar"},
exp: `"I am a fmt.Stringer"`,
},
{
val: fmt.Errorf("error"),
exp: `"error"`,
val: &Tstringer{"foobar"},
exp: `"I am a fmt.Stringer"`,
},
{
val: (*Tstringer)(nil),
exp: `"<nil-fmt-stringer>"`,
},
{
val: Terror{"foobar"},
exp: `"I am an error"`,
},
{
val: &Terror{"foobar"},
exp: `"I am an error"`,
},
{
val: (*Terror)(nil),
exp: `"<nil-error>"`,
},
{
val: TjsontagsString{
Expand Down

0 comments on commit a58a296

Please sign in to comment.