Skip to content

Commit

Permalink
Swallow panic when calling String or Error (#221)
Browse files Browse the repository at this point in the history
If a panic occurs while calling String or Error,
the reporter recovers from it and ignores it, proceeding with
its usual functionality for formatting a value.
  • Loading branch information
dsnet committed Jun 22, 2020
1 parent 77ae86f commit d669b04
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
13 changes: 13 additions & 0 deletions cmp/compare_test.go
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"crypto/md5"
"encoding/json"
"errors"
"flag"
"fmt"
"io"
Expand Down Expand Up @@ -874,6 +875,18 @@ func reporterTests() []test {
)

return []test{{
label: label + "/PanicStringer",
x: struct{ X fmt.Stringer }{struct{ fmt.Stringer }{nil}},
y: struct{ X fmt.Stringer }{bytes.NewBuffer(nil)},
wantEqual: false,
reason: "panic from fmt.Stringer should not crash the reporter",
}, {
label: label + "/PanicError",
x: struct{ X error }{struct{ error }{nil}},
y: struct{ X error }{errors.New("")},
wantEqual: false,
reason: "panic from error should not crash the reporter",
}, {
label: label + "/AmbiguousType",
x: foo1.Bar{},
y: foo2.Bar{},
Expand Down
18 changes: 12 additions & 6 deletions cmp/report_reflect.go
Expand Up @@ -125,12 +125,18 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind,
// implementations crash when doing so.
if (t.Kind() != reflect.Ptr && t.Kind() != reflect.Interface) || !v.IsNil() {
var prefix, strVal string
switch v := v.Interface().(type) {
case error:
prefix, strVal = "e", v.Error()
case fmt.Stringer:
prefix, strVal = "s", v.String()
}
func() {
// Swallow and ignore any panics from String or Error.
defer func() { recover() }()
switch v := v.Interface().(type) {
case error:
strVal = v.Error()
prefix = "e"
case fmt.Stringer:
strVal = v.String()
prefix = "s"
}
}()
if prefix != "" {
maxLen := len(strVal)
if opts.LimitVerbosity {
Expand Down
12 changes: 12 additions & 0 deletions cmp/testdata/diffs
Expand Up @@ -252,6 +252,18 @@
})),
}
>>> TestDiff/Transformer/AcyclicString
<<< TestDiff/Reporter/PanicStringer
struct{ X fmt.Stringer }{
- X: struct{ fmt.Stringer }{},
+ X: s"",
}
>>> TestDiff/Reporter/PanicStringer
<<< TestDiff/Reporter/PanicError
struct{ X error }{
- X: struct{ error }{},
+ X: e"",
}
>>> TestDiff/Reporter/PanicError
<<< TestDiff/Reporter/AmbiguousType
interface{}(
- "github.com/google/go-cmp/cmp/internal/teststructs/foo1".Bar{},
Expand Down

0 comments on commit d669b04

Please sign in to comment.