return non-zero exit code upon non-runnable subcommand

[donggangcj]

when excuting a root comand , cobra returns success on typos and exit code is 1.

➜  ~ helm notfoundsubcommand
Error: unknown command "notfoundsubcommand" for "helm"
Run 'helm --help' for usage.
➜  ~ echo $?
1

when excuting a subcommand ,cobra returns success on typos but exit code is 0.

➜  ~ helm repo sadd foo https://foo/bar

This command consists of multiple subcommands to interact with chart repositories.

It can be used to add, remove, list, and index chart repositories.

Usage:
  helm repo [command]

Available Commands:
  add         add a chart repository
....

➜  ~ echo $?
0

This issue has come up before and this is related pr.

The related helm pr is here.

[donggangcj]

/assign

[umarcor]

Ref #842.

[github-actions]

This issue is being marked as stale due to a long period of inactivity

[jharshman]

@donggangcj , Interestingly enough I am not able to repro this. I'm curious why Helm would be yielding different results.

serenity Cobra Testing Ground/1156 » cat main.go 
package main

import (
  "github.com/spf13/cobra"
  "fmt"
)

var (
  parent = &cobra.Command{
    Short: "issue 1156",
    Long: "testing for issue 1156",
  }
  childOne = &cobra.Command{
    Use: "childone",
  }
  childTwo = &cobra.Command{
    Use: "childtwo",
    Run: func(cmd *cobra.Command, args []string) {
      fmt.Println("hello 1156")
    },
  }
)

func init() {
  parent.AddCommand(childOne)
  childOne.AddCommand(childTwo)
}

func main() {
  parent.Execute()
}
serenity Cobra Testing Ground/1156 » go run ./main.go typochildone
Error: unknown command "typochildone" for ""
Run ' --help' for usage.
serenity Cobra Testing Ground/1156 » echo $?
0
serenity Cobra Testing Ground/1156 » go run ./main.go childone typochildtwo
Usage:
   childone [command]

Available Commands:
  childtwo    

Flags:
  -h, --help   help for childone

Use " childone [command] --help" for more information about a command.
serenity Cobra Testing Ground/1156 » echo $?
0

Upon further review. This behavior seems to be stemming from logic within Helm itself.
https://github.com/helm/helm/blob/b0fdb5461fbc7e1b52f61613d3e7a22d85c7a794/cmd/helm/helm.go#L99

[marckhouzam]

@jharshman IIRC you need to specify a value for the Args field in your commands to trigger the problem.

[jharshman]

@marckhouzam I see, not sure it has anything to do with Args, but I am able to replicate the issue now.
The root of this problem might just be that there are different error paths here...

error being dropped due to it being of type flag.ErrHelp:

=> 954:			if err == flag.ErrHelp {
   955:				cmd.HelpFunc()(cmd, args)
   956:				return cmd, nil
   957:			}

So basically you can invoke your root command incorrectly and receive an error whereas sub commands after a successful root command invocation run through the above and return no error.

[umarcor]

A relevant modification in #842 is precisely fixing these inconsistencies. For deciding when to show the help and when to add args/subcommands in the help of non-runnable subcommands invoked without args, these situations need to be detected and reworked. Unfortunately, #842 is blocked by #841.

mongodb/mongodb-atlas-cli#202 (comment)

[github-actions]

This issue is being marked as stale due to a long period of inactivity

[colemickens]

this is still important to me (I was trying to trigger the bot,...)

[github-actions]

This issue is being marked as stale due to a long period of inactivity

[johnSchnake]

With the time since the original complaint I want to repro this again and make sure its still relevant. I think it is and there may even be a duplicate of this issue elsewhere but wanted to label this as part of the backlog cleanup (#1600)

[marckhouzam]

IIRC, has something to do with the legacyArgs() function is may be tricky to fix due to backwards-compatibility issue.
But I agree it is poor behaviour and would be great to find a solution.

[glawler]

Hello. I am also running into this. The issue happens for me when a subcommand is passed an arugment it does not understand. Instead of the program exiting with non-zero, it dumps the usage string and exits with 0.

Minimal working example:

package main

import (
    "errors"
    "flag"
    "os"

    "github.com/spf13/cobra"
)

var root = &cobra.Command{
    Use: "myCmd",
}

var subCmd = &cobra.Command{
    Use: "subCmd",
}

func main() {

    root.AddCommand(subCmd)
    
    err := root.Execute()
    if err != nil {
         os.Exit(1)
    }
    os.Exit(0)
}

Then build, go build and run myCmd subCmd notAValidArg. Usage will be printed (which in this case is just an empty string) and the exit value is 0. Compare that with myCmd notAValigArg which will exit with 1.

I believe if you just return the error instead of nil, this main will exit with 1 in both cases.

cobra/command.go

Line 1074 in fd865a4

return cmd, nil

So this:

		if errors.Is(err, flag.ErrHelp) {
			cmd.HelpFunc()(cmd, args)
			return cmd, err
		}

Just propagate the error back up to the caller. I can submit this as a PR, if you'd like.

[marckhouzam]

I’d have to look back at the details but the issue was that previous fixes have caused backwards compatibility issues.

[ldufresnegs]

Since my issue was closed as a duplicate to this, I would like to try to raise this issue again.

I understand that changing the behavior could break backward compatibility, but is there some workaround to detect this case?