urfave / cli

A simple, fast, and fun package for building command line apps in Go
https://cli.urfave.org
MIT License
22.36k stars 1.71k forks source link

exit code of 0 when invalid flag is used #610

Closed nyang5 closed 5 years ago

nyang5 commented 7 years ago

currently if an invalid command is given the exit code will be 3 when an invalid flag is given the exit code is 0?

is there a reason for this and how do I change the exit code when an invalid flag is given?

jszwedko commented 7 years ago

This does feel inconsistent. Thanks for the report @nyang5 !

jszwedko commented 7 years ago

An invalid flag will bubble an error up to App.Run() so you can check and exit at that level with a non-zero code.

lox commented 7 years ago

An invalid flag will bubble an error up to App.Run() so you can check and exit at that level with a non-zero code.

@jszwedko can you explain a bit more how that might look? I've tried replicating the app.OnUsageError function like in https://github.com/vektorlab/slackcat/commit/5add3e434b990f33931ecc5bfccc38ef0ace08a7, but we still see a silent app exit when a BoolFlag has a value like test.

lox commented 7 years ago

Some more details on the above, the latest master of urfave/cli seems to handle parameters with non-parseable values:

go run *.go bootstrap --clean-checkout=llamas                                                                                     
Incorrect Usage: invalid boolean value "llamas" for -clean-checkout: strconv.ParseBool: parsing "llamas": invalid syntax

However, it silently fails with environment variables:

BUILDKITE_CLEAN_CHECKOUT=llamas go run *.go bootstrap
exit 0
jszwedko commented 7 years ago

Hi @lox,

Can you try that with the latest cli? Or provide the commit you are using? I wasn't able to replicate with:

package main

import (
    "log"
    "os"

    "github.com/urfave/cli"
)

func main() {
    app := cli.NewApp()

    app.Flags = []cli.Flag{
        cli.BoolFlag{
            Name:   "clean-checkout",
            EnvVar: "BUILDKITE_CLEAN_CHECKOUT",
            Usage:  "language for the greeting",
        },
    }

    app.Action = func(c *cli.Context) error {
        return nil
    }

    err := app.Run(os.Args)
    if err != nil {
        log.Fatal(err)
    }
}
$ BUILDKITE_CLEAN_CHECKOUT=llamas go run /tmp/tmp.go
2017/11/11 16:09:34 could not parse llamas as bool value for flag clean-checkout: strconv.ParseBool: parsing "llamas": invalid syntax
exit status 1
lox commented 7 years ago

Ah-hah, I wasn't catching the error from app.Run. Works with master now. Thanks!

jszwedko commented 7 years ago

No problem! I'll leave this open to make sure the issue with exit codes is addressed.

jkanywhere commented 6 years ago

Should HandleExitCoder call OsExiter(1) when passed a "normal" error?

For example:

err := errors.New("neither an ExitCoder nor a MultiError")
cli.HandleExitCoder(err) // should this os.Exit(1) ?
coilysiren commented 5 years ago

Given that this is from last year, I think I'm comfortable closing it 🙂 feel free to re-open / open a new issue / comment in support if there's still interest here!