Update Stringer panic check to look like stdlib (#857)

There's no behaviour changes, but there are a couple of refactorings:
 * Name the named return error `retErr`, and use explicit return
   values. The only purpose of the named return is for the panic
   handling.
 * Make the panic handling look more similar to the standard library
   and add a reference to the stdlib code in fmt that does the
   same checks.

Author: prashantv

zapcore/field.go

@@ -205,18 +205,23 @@ func addFields(enc ObjectEncoder, fields []Field) {
 	}
 }
 
-func encodeStringer(key string, stringer interface{}, enc ObjectEncoder) (err error) {
+func encodeStringer(key string, stringer interface{}, enc ObjectEncoder) (retErr error) {
+	// Try to capture panics (from nil references or otherwise) when calling
+	// the String() method, similar to https://golang.org/src/fmt/print.go#L540
 	defer func() {
-		if v := recover(); v != nil {
-			val := reflect.ValueOf(stringer)
-			if val.Kind() == reflect.Ptr && val.IsNil() {
+		if err := recover(); err != nil {
+			// If it's a nil pointer, just say "<nil>". The likeliest causes are a
+			// Stringer that fails to guard against nil or a nil pointer for a
+			// value receiver, and in either case, "<nil>" is a nice result.
+			if v := reflect.ValueOf(stringer); v.Kind() == reflect.Ptr && v.IsNil() {
 				enc.AddString(key, "<nil>")
-			} else {
-				err = fmt.Errorf("PANIC=%v", v)
+				return
 			}
+
+			retErr = fmt.Errorf("PANIC=%v", err)
 		}
 	}()
 
 	enc.AddString(key, stringer.(fmt.Stringer).String())
-	return
+	return nil
 }