Leaking Undefined Behavior

Don’t use = when assigning with an error. Typically, this is paired with predefining an err variable, though this may be implicit in an earlier := and thus be invisible.

This often presents in a few ways ways:

// Assigning outside the current scope
var somethingOutsideTheScope T
{
  var err error
  somethingOutsideTheScope, err = somethingThatReturnsError()
}

// Assigning to a struct field
var err error
something.field, err = somethingThatReturnsError()

// Reusing the same variables
foo, err := somethingThatReturnsError()
...
foo, err = somethingElseThatReturnsError()

While this pattern is often correct when initially written, it is very easy to accidentally break in the future. Its correctness relies on:

The function that is being called returns something meaningful along with a non-nil error
The calling code checks the error and correctly avoids using the other return values

Both of these are easy to accidentally change as the code evolves over time. The function might start returning an uninitialized or partially initialized value along with the error in order to “simplify” the code, and this is typically safe because the canonical assumption is that the caller will ignore all returned values in the case of error unless documented otherwise. The calling code might also switch to logging the error instead of failing, might start using a multi-error pattern, or might allow tests to bypass the failure. It’s also possible for either of these things to happen as a result of a bug.

In all of these cases, it is very likely that the ramifications of the change (i.e. leaking a “bad” value beyond the scope of the call) is not visible when the change is made, and the issues that surface will often be very far away from the code that introduced the bug, because the value “escaped” from the call + error check that was supposed to keep it contained.

It is good to get into the habit of using a (new) local variable and only passing along the value after checking the error, as this will tend to avoid the leaks above:

// Assigning outside the current scope
var somethingOutsideTheScope T
{
  sots, err := somethingThatReturnsError()
  if err != nil { ... }
  somethingOutsideTheScope = sots
}

// Assigning to a struct field
sf, err := somethingThatReturnsError()
if err != nil { ... }
something.field = sf

// Reusing the same variables
foo1, err := somethingThatReturnsError()
...
foo2, err := somethingElseThatReturnsError()

In the cases above, the “leak” (i.e. where the value escapes the call + error check) becomes explicit and happens after the conditional, so there is no opportunity for the pre-check value to leak out and there is increased visibility to help call attention if the error handling is going to change.

kylelemons/leaking_undefined_behavior.md

Leaking Undefined Behavior