unset env-vars does not play nicely with custom decode functions

[twmb]

envconfig.go has this specific block of logic:

go-envconfig/envconfig.go

Lines 570 to 581 in 39c9bc7

	// Handle existing decoders.
	if ok, err := processAsDecoder(v, ef); ok {
	return err
	}
	// We don't check if the value is empty earlier, because the user might want
	// to define a custom decoder and treat the empty variable as a special case.
	// However, if we got this far, none of the remaining parsers will succeed, so
	// bail out now.
	if v == "" {
	return nil
	}

I agree with the sentiment of the block logic: custom decoders may want to process empty env vars in a specific way, rather than return early.

However, if an env for is not set at all, it is currently loaded as an empty string (here:

go-envconfig/envconfig.go

Lines 468 to 491 in 39c9bc7

	val, ok := l.Lookup(key)
	if !ok {
	if opts.Required {
	if pl, ok := l.(*prefixLookuper); ok {
	key = pl.prefix + key
	}
	return "", fmt.Errorf("%w: %s", ErrMissingRequired, key)
	}
	if opts.Default != "" {
	// Expand the default value. This allows for a default value that maps to
	// a different variable.
	val = os.Expand(opts.Default, func(i string) string {
	s, ok := l.Lookup(i)
	if ok {
	return s
	}
	return ""
	})
	}
	}
	return val, nil

)

It seems as if env var overrides should not be applied if the env var does not exist. In my code, this is conflict with a type that uses a field like so,

type SomeType struct {
    // ..other fields

    LogLevel zapcore.Level `env:"LEVEL,omitempty"`
}

I specifically define in my config file that the level is debug. The zapcore code handles empty input to UnmarshalText as ok and writes the log level to info (see here: https://github.com/uber-go/zap/blob/6f34060764b5ea1367eecda380ba8a9a0de3f0e6/zapcore/level.go#L142-L143), and I have no control to stop this override.

My only current option is to wrap this type in another type that specifically handles the empty string as a no-op. Unfortunately, this breaks the potentially desired behavior of when an empty string is actually specified changing the level to info.

ISTM that the better default would be to not process env vars if the env var does not exist.

[sethvargo]

The zapcore code handles empty input to UnmarshalText as ok and writes the log level to info (see here: https://github.com/uber-go/zap/blob/6f34060764b5ea1367eecda380ba8a9a0de3f0e6/zapcore/level.go#L142-L143), and I have no control to stop this override.

I don't understand. Why can't you just set a default envconfig value to another value?

type SomeType struct {
    // ..other fields

    LogLevel zapcore.Level `env:"LEVEL,omitempty,default=debug"`
}

I'm also not sure what you mean by "env var overrides".

[twmb]

https://go.dev/play/p/amK4b3kwJ97

Envconfig is overriding a field that was loaded from a file, even though the environment variable is not set.

Using default doesn't work because then envconfig overrides with the default value, and the config file has a different value than the default.

For an example using zapcore, https://go.dev/play/p/Ug1VlORuxjS.

In either of these examples, removing default=error will change the final result to "info".