Repository: alecthomas/kong
Pull request #384 (2023-10-07 07:02:19)

Drop automatic message prefix from command errors
Resolves https://github.com/alecthomas/kong/issues/383.

With this PR, kong will pass through error messages from commands’ `Run()` methods as is, without wrapping them up or reformatting them.

The `callMethod` is obsolete now, its only purpose was the error wrapping. The callers can use `callFunction` directly.

alecthomas: Ah I remember what the _intent_ of this was now - it was to put the command name in the error message. So eg. if you had `foo cmd` the error would be prefixed with `cmd: ...`. 

Patch:
diff --git a/callbacks.go b/callbacks.go
index 3a8a45f..8771a3e 100644
--- a/callbacks.go
+++ b/callbacks.go
@@ -127,11 +127,3 @@ func callAnyFunction(f reflect.Value, bindings bindings) (out []any, err error)
 	}
 	return out, nil
 }
-
-func callMethod(name string, v, f reflect.Value, bindings bindings) error {
-	err := callFunction(f, bindings)
-	if err != nil {
-		return fmt.Errorf("%s.%s(): %w", v.Type(), name, err)
-	}
-	return nil
-}
diff --git a/context.go b/context.go
index dddf796..0b41079 100644
--- a/context.go
+++ b/context.go
@@ -762,7 +762,7 @@ func (c *Context) RunNode(node *Node, binds ...interface{}) (err error) {
 	}
 
 	for _, method := range methods {
-		if err = callMethod("Run", method.node.Target, method.method, method.binds); err != nil {
+		if err = callFunction(method.method, method.binds); err != nil {
 			return err
 		}
 	}
diff --git a/kong.go b/kong.go
index 255633f..c4eda71 100644
--- a/kong.go
+++ b/kong.go
@@ -336,7 +336,7 @@ func (k *Kong) applyHook(ctx *Context, name string) error {
 		binds.add(ctx, trace)
 		binds.add(trace.Node().Vars().CloneWith(k.vars))
 		binds.merge(ctx.bindings)
-		if err := callMethod(name, value, method, binds); err != nil {
+		if err := callFunction(method, binds); err != nil {
 			return err
 		}
 	}
@@ -364,7 +364,7 @@ func (k *Kong) applyHookToDefaultFlags(ctx *Context, node *Node, name string) er
 				continue
 			}
 			path := &Path{Flag: flag}
-			if err := callMethod(name, flag.Target, method, binds.clone().add(path)); err != nil {
+			if err := callFunction(method, binds.clone().add(path)); err != nil {
 				return next(err)
 			}
 		}


Test patch:
diff --git a/kong_test.go b/kong_test.go
index 1ef424e..e7a5ba0 100644
--- a/kong_test.go
+++ b/kong_test.go
@@ -641,6 +641,23 @@ func TestRun(t *testing.T) {
 	assert.Equal(t, "argping", cli.Three.SubCommand.Arg)
 }
 
+type failCmd struct{}
+
+func (f failCmd) Run() error {
+	return errors.New("this command failed")
+}
+
+func TestPassesThroughOriginalCommandError(t *testing.T) {
+	var cli struct {
+		Fail failCmd `kong:"cmd"`
+	}
+	p := mustNew(t, &cli)
+	ctx, _ := p.Parse([]string{"fail"})
+	err := ctx.Run()
+	assert.Error(t, err)
+	assert.Equal(t, err.Error(), "this command failed")
+}
+
 func TestInterpolationIntoModel(t *testing.T) {
 	var cli struct {
 		Flag    string `default:"${default_value}" help:"Help, I need ${somebody}" enum:"${enum}"`
diff --git a/options_test.go b/options_test.go
index 2dbd0ba..bb9cdaa 100644
--- a/options_test.go
+++ b/options_test.go
@@ -38,7 +38,7 @@ func TestBindTo(t *testing.T) {
 
 	p, err := New(&cli, BindTo(impl("foo"), (*iface)(nil)))
 	assert.NoError(t, err)
-	err = callMethod("method", reflect.ValueOf(impl("??")), reflect.ValueOf(method), p.bindings)
+	err = callFunction(reflect.ValueOf(method), p.bindings)
 	assert.NoError(t, err)
 	assert.Equal(t, "foo", saw)
 }
@@ -58,8 +58,8 @@ func TestInvalidCallback(t *testing.T) {
 
 	p, err := New(&cli, BindTo(impl("foo"), (*iface)(nil)))
 	assert.NoError(t, err)
-	err = callMethod("method", reflect.ValueOf(impl("??")), reflect.ValueOf(method), p.bindings)
-	assert.EqualError(t, err, `kong.impl.method(): return value of func(kong.iface) string must implement "error"`)
+	err = callFunction(reflect.ValueOf(method), p.bindings)
+	assert.EqualError(t, err, `return value of func(kong.iface) string must implement "error"`)
 }
 
 type zrror struct{}
@@ -83,7 +83,7 @@ func TestCallbackCustomError(t *testing.T) {
 
 	p, err := New(&cli, BindTo(impl("foo"), (*iface)(nil)))
 	assert.NoError(t, err)
-	err = callMethod("method", reflect.ValueOf(impl("??")), reflect.ValueOf(method), p.bindings)
+	err = callFunction(reflect.ValueOf(method), p.bindings)
 	assert.NoError(t, err)
 	assert.Equal(t, "foo", saw)
 }