MiddleDebuggingOccasionalNot answered yet
Why does this load always return a nil error even when parse fails?
This load should return parse's error when parsing fails, but it always returns a nil error to the caller.
func load() (*Config, error) {
cfg := &Config{}
if data, err := read(); err == nil {
cfg, err = parse(data)
_ = err
}
return cfg, nil
}Identify the bug and explain the cause.
The function hard-codes return cfg, nil, so parse's error is swallowed. A related classic is cfg, err := parse(data) with := in an inner scope, which shadows the outer cfg so the parsed value is lost. Fix: return the error on failure and avoid accidental := shadowing; go vet catches it.
- ✗Hard-coding
return cfg, nilinstead of returning the real error - ✗Using
:=in an inner scope, silently shadowing the outer variable - ✗Assuming the success-only
ifbranch meansparsecannot fail
- →How does
:=decide whether to create a new variable or reuse an outer one? - →What does
go vet's shadow analysis report on this function?
Contents
Find the bug
func load() (*Config, error) {
cfg := &Config{}
if data, err := read(); err == nil {
cfg, err = parse(data) // := vs = subtlety below
_ = err
}
return cfg, nil // BUG: always returns a nil error
}Why the error is lost
Two related problems:
- Swallowed error. The function unconditionally returns
return cfg, nil. Even ifparsereturned an error, it is assigned to the localerr, marked_ = err, and forgotten —nilalways flows out.
- Shadowing via
:=. The classic related bug is writingcfg, err := parse(data)(with:=) in an inner scope. Because there is a new variable on the left (errfrom theif),:=creates a newcfgthat shadows the outer one. The parsed config lands in the local shadow and is lost when theifblock ends.
✅ The fix is to propagate the error and not shadow:
func load() (*Config, error) {
data, err := read()
if err != nil {
return nil, err
}
cfg, err := parse(data)
if err != nil {
return nil, err
}
return cfg, nil
}go vet and golangci-lint's shadow check catch the shadowing.
Contents
