From 87b3e45c38d060541e2e9be6cd49d012e237ebbb Mon Sep 17 00:00:00 2001 From: Emirlan Rasulov Date: Sat, 4 Mar 2023 15:16:32 +0600 Subject: [PATCH] Refactor error messages for missing variables. Before we would return error right aways. Now when we encounter an error on parsing stage, we try to list all the missing or incorrect envs. This should make debugging for our users easier Also added some tests for the new error type. --- cleanenv.go | 70 +++++++++++++++++++++++++++++++++++++++++++----- cleanenv_test.go | 44 ++++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 9 deletions(-) diff --git a/cleanenv.go b/cleanenv.go index dd4e1d4..6bbdcdf 100644 --- a/cleanenv.go +++ b/cleanenv.go @@ -398,6 +398,50 @@ func readStructMetadata(cfgRoot interface{}) ([]structMeta, error) { return metas, nil } +type readEnvVarsError struct { + missingEnvs []string + parsingErrs []error +} + +func (e readEnvVarsError) IsEmpty() bool { + return len(e.missingEnvs) == 0 && len(e.parsingErrs) == 0 +} + +func (e readEnvVarsError) Error() string { + var tmp string + for i, env := range e.missingEnvs { + tmp += fmt.Sprintf("\t%q", env) + // add new line if this is not the last element + if i != len(e.missingEnvs)-1 { + tmp += "\n" + } + } + missings := fmt.Sprintf("missing required environment variables: \n%s", tmp) + + tmp = "" + for i, err := range e.parsingErrs { + tmp += fmt.Sprintf("\t%v", err) + // add new line if this is not the last element + if i != len(e.parsingErrs)-1 { + tmp += "\n" + } + } + parsingErrs := fmt.Sprintf("parsing errors for environment variables: \n%s", tmp) + + var res string + + switch { + case len(e.missingEnvs) != 0 && len(e.parsingErrs) != 0: + res = fmt.Sprintf("%s\n%s", missings, parsingErrs) + case len(e.missingEnvs) != 0: + res = missings + case len(e.parsingErrs) != 0: + res = parsingErrs + } + + return res +} + // readEnvVars reads environment variables to the provided configuration structure func readEnvVars(cfg interface{}, update bool) error { metaInfo, err := readStructMetadata(cfg) @@ -405,21 +449,29 @@ func readEnvVars(cfg interface{}, update bool) error { return err } + // store initial configuration, so we can return default values if errors occur + initialCfg := reflect.ValueOf(cfg).Elem().Interface() + if updater, ok := cfg.(Updater); ok { if err := updater.Update(); err != nil { return err } } + var errs readEnvVarsError + for _, meta := range metaInfo { // update only updatable fields if update && !meta.updatable { continue } - var rawValue *string + var ( + rawValue *string + env string + ) - for _, env := range meta.envList { + for _, env = range meta.envList { if value, ok := os.LookupEnv(env); ok { rawValue = &value break @@ -427,10 +479,8 @@ func readEnvVars(cfg interface{}, update bool) error { } if rawValue == nil && meta.required && meta.isFieldValueZero() { - return fmt.Errorf( - "field %q is required but the value is not provided", - meta.fieldName, - ) + errs.missingEnvs = append(errs.missingEnvs, env) + continue } if rawValue == nil && meta.isFieldValueZero() { @@ -447,10 +497,16 @@ func readEnvVars(cfg interface{}, update bool) error { } if err := parseValue(meta.fieldValue, *rawValue, meta.separator, meta.layout); err != nil { - return fmt.Errorf("parsing field %v env %v: %v", meta.fieldName, envName, err) + errs.parsingErrs = append(errs.parsingErrs, fmt.Errorf("field %v env %q: %v", meta.fieldName, envName, err)) } } + if !errs.IsEmpty() { + // restore initial configuration + reflect.ValueOf(cfg).Elem().Set(reflect.ValueOf(initialCfg)) + return errs + } + return nil } diff --git a/cleanenv_test.go b/cleanenv_test.go index d51490c..b77a959 100644 --- a/cleanenv_test.go +++ b/cleanenv_test.go @@ -1191,8 +1191,8 @@ func TestTimeLocation(t *testing.T) { func TestSkipUnexportedField(t *testing.T) { conf := struct { Database struct { - Host string `yaml:"host" env:"DB_HOST" env-description:"Database host"` - Port string `yaml:"port" env:"DB_PORT" env-description:"Database port"` + Host string `yaml:"host" env:"DB_HOST" env-description:"Database host"` + Port string `yaml:"port" env:"DB_PORT" env-description:"Database port"` } `yaml:"database"` server struct { Host string `yaml:"host" env:"SRV_HOST,HOST" env-description:"Server host" env-default:"localhost"` @@ -1210,3 +1210,43 @@ func TestSkipUnexportedField(t *testing.T) { t.Fatal("expect value on exported fields") } } + +func TestReturnMissingVariables(t *testing.T) { + conf := struct { + Port string `env:"PORT" env-required:"true"` + JWTsalt string `env:"JWT_SALT" env-required:"true"` + ReadTimout time.Duration `env:"READ_TIMEOUT" env-required:"true"` + WriteTimeout time.Duration `env:"WRITE_TIMEOUT" env-default:"15s"` + }{} + expectedErrMsg := `missing required environment variables: + "PORT" + "JWT_SALT" + "READ_TIMEOUT" +parsing errors for environment variables: + field WriteTimeout env "WRITE_TIMEOUT": time: invalid duration "incorrect"` + + os.Setenv("WRITE_TIMEOUT", "incorrect") + defer os.Clearenv() + + err := ReadEnv(&conf) + + if err == nil { + t.Fatal("expect error") + } + + switch errt := err.(type) { + case readEnvVarsError: + t.Log(errt.Error()) + if len(errt.missingEnvs) != 3 { + t.Fatalf("wrong number of missing envs: got %v want %v", len(errt.missingEnvs), 4) + } + if len(errt.parsingErrs) != 1 { + t.Fatalf("wrong number of parsing errors: got %v want %v", len(errt.parsingErrs), 1) + } + if errt.Error() != expectedErrMsg { + t.Fatalf("wrong error message: got %v want %v", errt.Error(), expectedErrMsg) + } + default: + t.Fatal("expect type of error to be readEnvVarsError") + } +}