Report the source of a value when we cannot parse it

If you allow a flag to be set from environment variables or files and
a parse error occurs from one of them, it is very useful for the error
message to mention where the value came from.

Without this, it can be difficult to notice an error caused by an
unexpected environment variable being set.

Implements #1167.
main
Mostyn Bramley-Moore 4 years ago
parent ec731febcc
commit 500d6b04e6

@ -372,17 +372,17 @@ func hasFlag(flags []Flag, fl Flag) bool {
return false
}
func flagFromEnvOrFile(envVars []string, filePath string) (val string, ok bool) {
func flagFromEnvOrFile(envVars []string, filePath string) (val string, ok bool, source string) {
for _, envVar := range envVars {
envVar = strings.TrimSpace(envVar)
if val, ok := syscall.Getenv(envVar); ok {
return val, true
return val, true, fmt.Sprintf("from environment variable %q", envVar)
}
}
for _, fileVar := range strings.Split(filePath, ",") {
if data, err := ioutil.ReadFile(fileVar); err == nil {
return string(data), true
return string(data), true, fmt.Sprintf("from file %q", filePath)
}
}
return "", false
return "", false, ""
}

@ -60,12 +60,12 @@ func (f *BoolFlag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *BoolFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
valBool, err := strconv.ParseBool(val)
if err != nil {
return fmt.Errorf("could not parse %q as bool value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as bool value %s for flag %s: %s", val, source, f.Name, err)
}
f.Value = valBool

@ -60,12 +60,12 @@ func (f *DurationFlag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *DurationFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
valDuration, err := time.ParseDuration(val)
if err != nil {
return fmt.Errorf("could not parse %q as duration value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as duration value %s for flag %s: %s", val, source, f.Name, err)
}
f.Value = valDuration

@ -60,12 +60,12 @@ func (f *Float64Flag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *Float64Flag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
valFloat, err := strconv.ParseFloat(val, 10)
if err != nil {
return fmt.Errorf("could not parse %q as float64 value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as float64 value %s for flag %s: %s", val, source, f.Name, err)
}
f.Value = valFloat

@ -119,13 +119,13 @@ func (f *Float64SliceFlag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *Float64SliceFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
f.Value = &Float64Slice{}
for _, s := range strings.Split(val, ",") {
if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as float64 slice value for flag %s: %s", f.Value, f.Name, err)
return fmt.Errorf("could not parse %q as float64 slice value %s for flag %s: %s", f.Value, source, f.Name, err)
}
}

@ -69,10 +69,10 @@ func (f *GenericFlag) GetValue() string {
// Apply takes the flagset and calls Set on the generic flag with the value
// provided by the user for parsing by the flag
func (f GenericFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
if err := f.Value.Set(val); err != nil {
return fmt.Errorf("could not parse %q as value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q %s as value for flag %s: %s", val, source, f.Name, err)
}
f.HasBeenSet = true

@ -60,12 +60,12 @@ func (f *IntFlag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *IntFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
valInt, err := strconv.ParseInt(val, 0, 64)
if err != nil {
return fmt.Errorf("could not parse %q as int value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as int value %s for flag %s: %s", val, source, f.Name, err)
}
f.Value = int(valInt)

@ -60,12 +60,12 @@ func (f *Int64Flag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *Int64Flag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
valInt, err := strconv.ParseInt(val, 0, 64)
if err != nil {
return fmt.Errorf("could not parse %q as int value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as int value %s for flag %s: %s", val, source, f.Name, err)
}
f.Value = valInt

@ -120,12 +120,12 @@ func (f *Int64SliceFlag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *Int64SliceFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
f.Value = &Int64Slice{}
for _, s := range strings.Split(val, ",") {
if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as int64 slice value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as int64 slice value %s for flag %s: %s", val, source, f.Name, err)
}
}

@ -131,12 +131,12 @@ func (f *IntSliceFlag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *IntSliceFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
f.Value = &IntSlice{}
for _, s := range strings.Split(val, ",") {
if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as int slice value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as int slice value %s for flag %s: %s", val, source, f.Name, err)
}
}

@ -56,7 +56,8 @@ func (f *PathFlag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *PathFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
// TODO: how to report the source of parse errors?
if val, ok, _ := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
f.Value = val
f.HasBeenSet = true
}

@ -57,7 +57,8 @@ func (f *StringFlag) GetValue() string {
// Apply populates the flag given the flag set and environment
func (f *StringFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
// TODO: how to report source?
if val, ok, _ := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
f.Value = val
f.HasBeenSet = true
}

@ -123,7 +123,7 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error {
}
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if f.Value == nil {
f.Value = &StringSlice{}
}
@ -134,7 +134,7 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error {
for _, s := range strings.Split(val, ",") {
if err := destination.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as string value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as string value %s for flag %s: %s", val, source, f.Name, err)
}
}

@ -79,30 +79,30 @@ func TestFlagsFromEnv(t *testing.T) {
{"", false, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""},
{"1", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""},
{"false", false, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""},
{"foobar", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, `could not parse "foobar" as bool value for flag debug: .*`},
{"foobar", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, `could not parse "foobar" as bool value from environment variable "DEBUG" for flag debug: .*`},
{"1s", 1 * time.Second, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, ""},
{"foobar", false, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, `could not parse "foobar" as duration value for flag time: .*`},
{"foobar", false, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, `could not parse "foobar" as duration value from environment variable "TIME" for flag time: .*`},
{"1.2", 1.2, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"1", 1.0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"foobar", 0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as float64 value for flag seconds: .*`},
{"foobar", 0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as float64 value from environment variable "SECONDS" for flag seconds: .*`},
{"1", int64(1), &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"1.2", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value for flag seconds: .*`},
{"foobar", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value for flag seconds: .*`},
{"1.2", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value from environment variable "SECONDS" for flag seconds: .*`},
{"1", 1, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"1.2", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value for flag seconds: .*`},
{"foobar", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value for flag seconds: .*`},
{"1.2", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value from environment variable "SECONDS" for flag seconds: .*`},
{"1,2", newSetIntSlice(1, 2), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"1.2,2", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int slice value for flag seconds: .*`},
{"foobar", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int slice value for flag seconds: .*`},
{"1.2,2", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int slice value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int slice value from environment variable "SECONDS" for flag seconds: .*`},
{"1,2", newSetInt64Slice(1, 2), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"1.2,2", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int64 slice value for flag seconds: .*`},
{"foobar", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int64 slice value for flag seconds: .*`},
{"1.2,2", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int64 slice value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int64 slice value from environment variable "SECONDS" for flag seconds: .*`},
{"foo", "foo", &StringFlag{Name: "name", EnvVars: []string{"NAME"}}, ""},
{"path", "path", &PathFlag{Name: "path", EnvVars: []string{"PATH"}}, ""},
@ -110,12 +110,12 @@ func TestFlagsFromEnv(t *testing.T) {
{"foo,bar", newSetStringSlice("foo", "bar"), &StringSliceFlag{Name: "names", EnvVars: []string{"NAMES"}}, ""},
{"1", uint(1), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"1.2", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint value for flag seconds: .*`},
{"foobar", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint value for flag seconds: .*`},
{"1.2", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint value from environment variable "SECONDS" for flag seconds: .*`},
{"1", uint64(1), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"1.2", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint64 value for flag seconds: .*`},
{"foobar", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint64 value for flag seconds: .*`},
{"1.2", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint64 value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint64 value from environment variable "SECONDS" for flag seconds: .*`},
{"foo,bar", &Parser{"foo", "bar"}, &GenericFlag{Name: "names", Value: &Parser{}, EnvVars: []string{"NAMES"}}, ""},
}
@ -1758,7 +1758,7 @@ func TestFlagFromFile(t *testing.T) {
}
for _, filePathTest := range filePathTests {
got, _ := flagFromEnvOrFile(filePathTest.name, filePathTest.path)
got, _, _ := flagFromEnvOrFile(filePathTest.name, filePathTest.path)
if want := filePathTest.expected; got != want {
t.Errorf("Did not expect %v - Want %v", got, want)
}

@ -123,9 +123,9 @@ func (f *TimestampFlag) Apply(set *flag.FlagSet) error {
}
f.Value.SetLayout(f.Layout)
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if err := f.Value.Set(val); err != nil {
return fmt.Errorf("could not parse %q as timestamp value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as timestamp value %s for flag %s: %s", val, source, f.Name, err)
}
f.HasBeenSet = true
}

@ -54,11 +54,11 @@ func (f *UintFlag) GetUsage() string {
// Apply populates the flag given the flag set and environment
func (f *UintFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
valInt, err := strconv.ParseUint(val, 0, 64)
if err != nil {
return fmt.Errorf("could not parse %q as uint value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as uint value %s for flag %s: %s", val, source, f.Name, err)
}
f.Value = uint(valInt)

@ -54,11 +54,11 @@ func (f *Uint64Flag) GetUsage() string {
// Apply populates the flag given the flag set and environment
func (f *Uint64Flag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
if val != "" {
valInt, err := strconv.ParseUint(val, 0, 64)
if err != nil {
return fmt.Errorf("could not parse %q as uint64 value for flag %s: %s", val, f.Name, err)
return fmt.Errorf("could not parse %q as uint64 value %s for flag %s: %s", val, source, f.Name, err)
}
f.Value = valInt

Loading…
Cancel
Save