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

Better error messages for parsing errors #269

Open
ck-schmidi opened this issue Jul 13, 2020 · 3 comments
Open

Better error messages for parsing errors #269

ck-schmidi opened this issue Jul 13, 2020 · 3 comments

Comments

@ck-schmidi
Copy link

We still have an old open issue for our CLI tool which is kind of a cosmetic thing, but it's still annoying and we would really like to have it fixed somehow.

If the parsing of a flag fails this error message gets created:

invalid argument "test" for "-w, --warning" flag: strconv.ParseInt: parsing "test": invalid syntax

For us, exposing internal function names to our users is not really an option.

There was also a discussion on the same topic for the golang stdlibrary. And they fixed the error messages

They went from:
invalid boolean value "3" for -debug: strconv.ParseBool: parsing "3": invalid syntax

to
invalid boolean value "3" for -debug: parse error

which doesn't expose any technical details to the users.

It would be nice if we could a similar way for the pflag package.

According to my example one proposal would be:
invalid argument "test" for "-w, --warning" flag: parse error

or even better (type in the front part of the message):
invalid integer value "test" for "-w, --warning" flag: parse error

Another approach could be providing a format function, so users of the pflag library can inject a format function for a customized formatting of parsing errors.

I checked all old issues and didn't find any similar problems. It's just a proposal and I would spend time providing a patch for it, but maybe some of you have thoughts on it.

@Emyrk
Copy link

Emyrk commented Sep 29, 2023

@ck-schmidi I also would like this, but instead of imposing a formatted string, I would prefer a sentinel error I can check and format myself.

type FlagParseError struct {
  // The flag being parsed and all it's context.
  Flag *Flag
  // The error thrown by the parse/set function
  Err error
  // The input value
  Value string
}

Then the caller can check if errors.As() and do w/e they want.

@reuvenharrison
Copy link

I just bumped into this through https://github.com/Tufin/oasdiff which uses https://github.com/spf13/cobra.

Running:

oasdiff changelog data/common-params/params_in_path.yaml data/common-params/params_in_op.yaml --flatten-params=x

Returns:

Error: invalid argument "x" for "--flatten-params" flag: strconv.ParseBool: parsing "x": invalid syntax

For more details see Tufin/oasdiff#482.

@tomqwpl
Copy link

tomqwpl commented Mar 13, 2024

I second this. spf13 shouldn't be returning internal programmer targeted errors ("strconv.ParseBool..."). It should return a simpler end user targeted error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants