Alt: swallow all panics

go-logr · Jan 21, 2022 · 79ce89b · 79ce89b
1 parent deee1ed
commit 79ce89b
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 56 deletions.
diff --git a/funcr/funcr.go b/funcr/funcr.go
@@ -598,54 +598,31 @@ func isEmpty(v reflect.Value) bool {
 
 func invokeMarshaler(m logr.Marshaler) (ret interface{}) {
 	defer func() {
-		if err := recover(); err != nil {
-			if isNil(m) {
-				ret = "<nil-logr-marshaler>"
-			} else {
-				panic(err)
-			}
+		if r := recover(); r != nil {
+			ret = fmt.Sprintf("<panic: %s>", r)
 		}
 	}()
 	return m.MarshalLog()
 }
 
 func invokeStringer(s fmt.Stringer) (ret string) {
 	defer func() {
-		if err := recover(); err != nil {
-			if isNil(s) {
-				ret = "<nil-fmt-stringer>"
-			} else {
-				panic(err)
-			}
+		if r := recover(); r != nil {
+			ret = fmt.Sprintf("<panic: %s>", r)
 		}
 	}()
 	return s.String()
 }
 
 func invokeError(e error) (ret string) {
 	defer func() {
-		if isNil(e) {
-			if err := recover(); err != nil {
-				ret = "<nil-error>"
-			} else {
-				panic(err)
-			}
+		if r := recover(); r != nil {
+			ret = fmt.Sprintf("<panic: %s>", r)
 		}
 	}()
 	return e.Error()
 }
 
-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
-}
-
 // Caller represents the original call site for a log line, after considering
 // logr.Logger.WithCallDepth and logr.Logger.WithCallStackHelper.  The File and
 // Line fields will always be provided, while the Func field is optional.

diff --git a/funcr/funcr_test.go b/funcr/funcr_test.go
@@ -237,9 +237,8 @@ func TestPretty(t *testing.T) {
 	}
 
 	cases := []struct {
-		val  interface{}
-		exp  string // used in cases where JSON can't handle it
-		pmsg string // used to test panic cases
+		val interface{}
+		exp string // used in cases where JSON can't handle it
 	}{
 		{val: "strval"},
 		{val: "strval\nwith\t\"escapes\""},
@@ -389,11 +388,11 @@ func TestPretty(t *testing.T) {
 		},
 		{
 			val: (*Tmarshaler)(nil),
-			exp: `"<nil-logr-marshaler>"`,
+			exp: `"<panic: value method github.com/go-logr/logr/funcr.Tmarshaler.MarshalLog called using nil *Tmarshaler pointer>"`,
 		},
 		{
-			val:  Tmarshalerpanic{"foobar"},
-			pmsg: "Tmarshalerpanic",
+			val: Tmarshalerpanic{"foobar"},
+			exp: `"<panic: Tmarshalerpanic>"`,
 		},
 		{
 			val: Tstringer{"foobar"},
@@ -405,11 +404,11 @@ func TestPretty(t *testing.T) {
 		},
 		{
 			val: (*Tstringer)(nil),
-			exp: `"<nil-fmt-stringer>"`,
+			exp: `"<panic: value method github.com/go-logr/logr/funcr.Tstringer.String called using nil *Tstringer pointer>"`,
 		},
 		{
-			val:  Tstringerpanic{"foobar"},
-			pmsg: "Tstringerpanic",
+			val: Tstringerpanic{"foobar"},
+			exp: `"<panic: Tstringerpanic>"`,
 		},
 		{
 			val: Terror{"foobar"},
@@ -421,11 +420,11 @@ func TestPretty(t *testing.T) {
 		},
 		{
 			val: (*Terror)(nil),
-			exp: `"<nil-error>"`,
+			exp: `"<panic: value method github.com/go-logr/logr/funcr.Terror.Error called using nil *Terror pointer>"`,
 		},
 		{
-			val:  Terrorpanic{"foobar"},
-			pmsg: "Terrorpanic",
+			val: Terrorpanic{"foobar"},
+			exp: `"<panic: Terrorpanic>"`,
 		},
 		{
 			val: TjsontagsString{
@@ -579,21 +578,6 @@ func TestPretty(t *testing.T) {
 
 	f := NewFormatter(Options{})
 	for i, tc := range cases {
-		// Handle cases that are supposed to panic.
-		if tc.pmsg != "" {
-			func() {
-				defer func() {
-					r := recover()
-					if r == nil {
-						t.Errorf("[%d]: expected panic(%q)", i, tc.pmsg)
-					} else if fmt.Sprintf("%v", r) != tc.pmsg {
-						t.Errorf("[%d]: expected panic(%q), got %q", i, tc.pmsg, r)
-					}
-				}()
-				_ = f.pretty(tc.val)
-			}()
-			continue
-		}
 		ours := f.pretty(tc.val)
 		want := ""
 		if tc.exp != "" {