Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return non-zero exit code upon non-runnable subcommand #1156

Open
donggangcj opened this issue Jul 11, 2020 · 15 comments · May be fixed by #1157 or #2167
Open

return non-zero exit code upon non-runnable subcommand #1156

donggangcj opened this issue Jul 11, 2020 · 15 comments · May be fixed by #1157 or #2167
Assignees
Labels
area/cobra-command Core `cobra.Command` implementations kind/bug A bug in cobra; unintended behavior triage/needs-info Needs more investigation from maintainers or more info from the issue provider

Comments

@donggangcj
Copy link

donggangcj commented Jul 11, 2020

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
Copy link
Author

/assign

donggangcj pushed a commit to donggangcj/cobra that referenced this issue Jul 11, 2020
@donggangcj donggangcj linked a pull request Jul 11, 2020 that will close this issue
@umarcor
Copy link
Contributor

umarcor commented Jul 11, 2020

Ref #842.

@github-actions
Copy link

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

@jharshman
Copy link
Collaborator

@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
Copy link
Collaborator

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

@jharshman
Copy link
Collaborator

jharshman commented Sep 24, 2020

@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
Copy link
Contributor

umarcor commented Sep 25, 2020

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
Copy link

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

@colemickens
Copy link

colemickens commented Dec 1, 2020

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

@github-actions
Copy link

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

@johnSchnake johnSchnake added the triage/needs-info Needs more investigation from maintainers or more info from the issue provider label Mar 25, 2022
@johnSchnake
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link

glawler commented Jul 23, 2023

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
Copy link
Collaborator

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

@ldufresnegs
Copy link

ldufresnegs commented Sep 28, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cobra-command Core `cobra.Command` implementations kind/bug A bug in cobra; unintended behavior triage/needs-info Needs more investigation from maintainers or more info from the issue provider
Projects
None yet
8 participants