Return "unknown command" error for unknown subcommands.

[MAnyKey]

Right now there is a difference in handling between unknown command and unknown subcommand.
Let's consider the example:

package main

import "github.com/spf13/cobra"

func main() {

	subCmd := &cobra.Command{
		Use: "foo",
	}
	subCmd.AddCommand(&cobra.Command{
		Use: "bar",
		Run: func(cmd *cobra.Command, args []string) {
			cmd.Println("bar")
		},
	})
	subCmd.AddCommand(&cobra.Command{
		Use: "baz",
		Run: func(cmd *cobra.Command, args []string) {
			cmd.Println("baz")
		},
	})

	rootCmd := &cobra.Command{
		Use: "test",
	}
	rootCmd.AddCommand(subCmd)
	rootCmd.Execute()
}

When I run it as ./test unknown I get 'unknown command' error:

Error: unknown command "unknown" for "test"
Run 'test --help' for usage.

If I run it as ./test foo unknown I receive help message:

$ go run main.go foo unknown
Usage:
  test foo [command]

Available Commands:
  bar         
  baz         

Flags:
  -h, --help   help for foo

Use "test foo [command] --help" for more information about a command.

It seems very much like inconsistency.

I suppose that we don't need to return flag.ErrHelp on not runnable command and treat it like "unknown subcommand".

[eparis]

It's a really dumb inconsistency, but one which has been here since day 1 and we haven't 'fixed' for fear of breaking expectations for someone. I believe it's possible to get the behavior you want by setting the Command.PositionalArgs to a function which does what you want. Maybe something like MaximumNArgs(0) ?

[MAnyKey]

Thanks for the response.
Actually, setting Run field is needed too (even if it is just an empty function).

[pclalv]

instead of setting, PositionalArgs: MaximumNArgs(0), you can set Args: cobra.NoArgs, the result of which is that the subcommand's 'unknown command' behavior will be consistent with that of the top-level command. Run needs to be set to some dummy function. e.g.:

var someCmd = &cobra.Command{
	Use:   "some",
	Args: cobra.NoArgs,
	Run:  func(*cobra.Command, []string) {},
}

[dionysius]

While what the previous commenter said is true, now we have an unfortunate double usage output, which we don't want and is not valid

Error: unknown command "fla" for "prog subcommand"
Usage:
  prog subcommand [flags]
  prog subcommand [command]
...

and since I want it to error when no subcommand is called, I define RunE():

        ...,
	RunE: func(*cobra.Command, []string) error {
		return fmt.Errorf("missing command")
	},

How about a cobra.NoFunc variable which cobra can check against to, which it can detect to not output the additional usage line?

[MAnyKey]

@dionysius I guess you need to set SilenceUsage to true in that case.

[dionysius]

Unfortunately not, referring to your example
Original returning an error (by cobra), the usage but exit 0
With Run we can return an error (by ourselves), optionally the usage and Exit 1, but as you can see, the Usage line is double (what I'm referring to in my last comment) - Which is technically the truth, since we've provided a Run() function! (It can be run by itself with persistentFlags OR using a subcommand which will have its own flags)
With Run + SilenceUsage everywhere no change in the effective Usage() output

That's why - since we don't want to change break the current behaviour, I'm suggesting that we have a way for defining, that example "Original" exits with 1 - without all these hacky workarounds in the following examples, which lead to technically correct behaviour!

That means, my suggestion with cobra.NoFunc is actually not a good idea, better would be rootCmd = cobra.Command{..., ExitErrorOnMissingSubcommand: true, ...} or something like that

Or actually changing the behaviour and exit 1 on usage error always and note that as breaking change for a new major release branch

While writing this up, checking some of the current tooling for their behaviour, I realized

• git has only 1 command level (e.g. git log)
• ip has 2 levels, but if only provided the first level (e.g. ip netns), it automatically means list of the 2nd level
• kubectl errors 1 if there are missing subcommands (e.g. kubectl get), but they have overridden anyway all possible aspects of cobra
• lxc errors 0 (e.g. lxc profile), but they also use cobra so "suffering" from the exactly this topic here
• openstack python client errors 2 (e.g. openstack server) - the first real-world example I've found without go or cobra

[github-actions]

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

[stgraber]

We just had this reported as a bug against LXD today and it's certainly frustrating that there is no way to solve it without causing an invalid usage...

[johnSchnake]

Since this issue had quite a bit of discussion and reaction I'm removing the stale label and marking as needing more triage. I'm a new maintainer so I am unsure if this had been discussed more out of band from this issue. The fact that the first comment mentioned it was a known/accepted state of things makes me unsure if there is intent to fix it.

Since numerous users are asking for it, it seems very reasonable though.

[Johnlon]

This is just broken - please fix

[Johnlon]

While writing this up, checking some of the current tooling for their behaviour, I realized

I'd like to think that these other broken users would welcome a fix, but anyway... please choose one of @dionysius suggestions, ie ExitErrorOnMissingSubcommand or declare a breaking change (and perhaps a new flag LegacyExitOkOnMissingSubcommand for those who want to retain the current broken behaviour).

@dionysius - maybe make a PR?

[sami-alajrami]

We worked around this by intercepting the errors from the root command and checking them to see if they came from an unknown command. To do this, you need to set SilenceErrors: true on the root command, then process the returned error.

follow the link below to see the snippet of code that processes the error:
https://github.com/kosli-dev/cli/blob/0c04ff508a07776b2f2fa1d714cc2b60c23a9f67/cmd/kosli/main.go#L23-L53

[dionysius]

Hey @johnSchnake I missed your comment, thank you for picking up the project!

In my point of view this issue is still open and I would love to see an intent to address it. As you've seen users more and more are stumbling over it, who mind this behaviour as wrongly, and thus wish for a fix. The problem is also that there is no "easy" workaround - you have to dig deep to get it round.

For this issue going forward we'd need a clear decision from the cobra maintainer(s) and maybe some boundaries. The decision is rougly whether we can do a breaking change, or not, or if this is actually as intended. And this indecisiveness probably led to no one (me included) taking time to offer a PR since we didn't want to waste time for nothing.

In my opinion this issue is broken due to a) the reported inconsistency (see OP's post) and b) usage error (if someone uses a command syntax wrongly). In general a non-error should be interpreted as "the command has been accepted" and the output can now be handled by the caller. At cobra level we only define the syntax of the command usage. If accepted the implementer can still return an error due to logical/runtime constraints. I don't know if there's a standard or guideline somewhere on how programs should react in such cases to undermine this opinion. But apart from that I believe cobra does also error if an unknown argument is used (cannot check at the moment). We also need to consider the existing implementations (some big projects use cobra) - if we'd do a breaking change, whether it is that impactful that it must be an additional option.

So I see two arguments in favour of a breaking change: consistency and expected outcome. I think many (me included) could also live with one more config option to opt out to the "for us" correct behaviour. So It's up for the maintainer(s) decision ultimately :)

[Johnlon]

Please please

[shiouen]

Is there an update on this issue? Still experiencing that dreaded 0 exit code when trying to run a subcommand that does not exist.

[marckhouzam]

I haven’t had time to refresh my memory about everything has been discussed in this issue. However, have you tried adding a value to the Args field for your command? I think that should fix the exit 0 problem.

[pawelprazak]

It would be nice if this issue could be resolved.

From what I understand the problem is that func (c *Command) execute(a []string) (err error) (github.com/spf13/cobra@v1.6.1/command.go:823) returns early if !c.Runnable() resulting in printing help message without any additional context (e.g. missing command).

For anyone interested in a workaround: I was successful using SetHelpFunc and manually checking the given command against the defined commands and their aliases. I've also accessed the args through command.Flags().Args() for convenience.

[caozhuozi]

cobra needs more maintainers....

[avamsi]

If it helps anyone, avamsi/climate@1e88533#diff-4d7d1923693fc5ce892add2ea2907a744e77ea0b50c1939ccc5067cb48a466a3 is how I worked around this for a CLI framework I maintain (https://github.com/avamsi/climate, which is built on top of Cobra).