fix: NPE due to checking if error is nil when it can be a value

alecthomas · alecthomas · commit f388f6cd39ae · 2024-11-06T09:36:22.000+11:00
Fixes #468
diff --git a/callbacks.go b/callbacks.go
@@ -81,7 +81,7 @@ func callFunction(f reflect.Value, bindings bindings) error {
 		return err
 	}
 	ferr := out[0]
-	if ferrv := reflect.ValueOf(ferr); !ferrv.IsValid() || ferrv.IsNil() {
+	if ferrv := reflect.ValueOf(ferr); !ferrv.IsValid() || ((ferrv.Kind() == reflect.Interface || ferrv.Kind() == reflect.Pointer) && ferrv.IsNil()) {
 		return nil
 	}
 	return ferr.(error) //nolint:forcetypeassert
diff --git a/options_test.go b/options_test.go
@@ -127,3 +127,22 @@ func TestFlagNamer(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, "SOMEFLAG", app.Model.Flags[1].Name)
 }
+
+type npError string
+
+func (e npError) Error() string {
+	return "ERROR: " + string(e)
+}
+
+func TestCallbackNonPointerError(t *testing.T) {
+	method := func() error {
+		return npError("failed")
+	}
+
+	var cli struct{}
+
+	p, err := New(&cli)
+	assert.NoError(t, err)
+	err = callFunction(reflect.ValueOf(method), p.bindings)
+	assert.EqualError(t, err, "ERROR: failed")
+}

Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ func callFunction(f reflect.Value, bindings bindings) error {`
`81`	`81`	`return err`
`82`	`82`	`}`
`83`	`83`	`ferr := out[0]`
`84`		`- if ferrv := reflect.ValueOf(ferr); !ferrv.IsValid() \|\| ferrv.IsNil() {`
	`84`	`+ if ferrv := reflect.ValueOf(ferr); !ferrv.IsValid() \|\| ((ferrv.Kind() == reflect.Interface \|\| ferrv.Kind() == reflect.Pointer) && ferrv.IsNil()) {`
`85`	`85`	`return nil`
`86`	`86`	`}`
`87`	`87`	`return ferr.(error) //nolint:forcetypeassert`