Be careful when reusing error variables

Everything we will describe here is fictional and has nothing related to real persons or real facts.

Wile Ethelbert is a software engineer who works for the ACME corporation and, while moving on to a new project, comes across a piece of code that has been running smoothly for about 10 months:

func main() {
  err := NoErrorFunc1()
  if err != nil {
    fmt.Println("Something went wrong in NoErrFunc1")
    return
  }

  err1 := NoErrorFunc2()
  if err1 != nil {
    fmt.Println("Something went wrong in NoErrFunc2")
    return
  }
  fmt.Println("Everything works great!")
}

Looking at the code, however, he notices that there is no real reason to use two different variables to handle the errors, so he decides to modify it:

func main() {
  err := NoErrorFunc1()
  if err != nil {
    fmt.Println("Something went wrong in NoErrFunc1")
    return
  }

  err = NoErrorFunc2()
  if err != nil {
    fmt.Println("Something went wrong in NoErrFunc2")
    return
  }
  fmt.Println("Everything works great!")
}

After he does, something strange happens. The code no longer works and always ends with: "Something went wrong in NoErrFunc2".

What's going on?

Investigation

Wile E. does not give up and decides to get to the bottom of the matter.

Wile E.: "Hmmmm... Nothing changed into NoErrFunc2. Why then did it start returning such errors all of a sudden? Let's add some log and see what's going on!"

package main

import "fmt"

type MyCustomError struct {
    msg1 string
}

func (e MyCustomError) Error() string {
    return e.msg1
}

func NoErrorFunc1() error {
    return nil
}

func NoErrorFunc2() *MyCustomError {
    return nil
}

func main() {
    err := NoErrorFunc1()
    if err != nil {
        fmt.Println("Something went wrong in NoErrFunc1")
        return
    }

    err = NoErrorFunc2()

    fmt.Println("Value of err:", err)
    if err != nil {
        fmt.Println("Something went wrong in NoErrFunc2")
        return
    }
    fmt.Println("Everything works great!")
}

=== OUTPUT ===
Value of err: <nil>
Something went wrong in NoErrFunc2

Wile E. puzzledly looks at the logs...

Wile E: "Hmm... value is nil but the if clause says it is not nil. What's going on?"

Suddenly a bell starts ringing in his head: a few days ago he read a blog about GO interface{} and nil values

Wile E: "It could be something similar! Let me add some more logs!"

package main

import (
    "fmt"
    "reflect"
)

type MyCustomError struct {
    msg1 string
}

func (e MyCustomError) Error() string {
    return e.msg1
}

func NoErrorFunc1() error {
    return nil
}

func NoErrorFunc2() *MyCustomError {
    return nil
}

func main() {
    err := NoErrorFunc1()
    if err != nil {
        fmt.Println("Something went wrong in NoErrFunc1")
        return
    }

    err = NoErrorFunc2()

    fmt.Println("Value of err:", err)
    fmt.Println("Is err nil? ", err == nil)
    _, ok := err.(interface{})
    fmt.Println("Is err an interface{}? ", ok)
    fmt.Println("Reflection - Is err nil? ", reflect.ValueOf(err).IsNil())
    if err != nil {
        fmt.Println("Something went wrong in NoErrFunc2")
        return
    }
    fmt.Println("Everything works great!")
}

=== OUTPUT ===
Value of err: <nil>
Is err nil?  false
Is err an interface{}?  true
Reflection - Is err nil?  true
Something went wrong in NoErrFunc2

Wile E: "Bingo! err is an interface{}! So what was described in the GO interface{} and nil values is exactly what is happening now!"
"But... Why was it working before? I need to add some more logs!"

After some time. Wile E. produces the following code:

package main

import (
    "fmt"
    "reflect"
)

type MyCustomError struct {
    msg1 string
}

func (e MyCustomError) Error() string {
    return e.msg1
}

func NoErrorFunc1() error {
    return nil
}

func NoErrorFunc2() *MyCustomError {
    return nil
}

func main() {
    err := NoErrorFunc1()
    if err != nil {
        fmt.Println("Something went wrong in NoErrFunc1")
        return
    }

    err1 := NoErrorFunc2()

    fmt.Println("Value of err1:", err1)
    fmt.Println("Is err1 nil? ", err1 == nil)
    _, ok := err1.(interface{})
    fmt.Println("Is err1 an interface{}? ", ok)
    fmt.Println("Reflection - Is err1 nil? ", reflect.ValueOf(err1).IsNil())
    if err1 != nil {
        fmt.Println("Something went wrong in NoErrFunc2")
        return
    }
    fmt.Println("Everything works great!")
}

=== OUTPUT ===
./prog.go:35:11: invalid operation: (err1) (variable of type *MyCustomError) is not an interface

Wile E.: "What? I mean... WHAT?"
"Please Wile, be quiet and think...You just added a check to verify if the variable was an interface{} and... Oh YEAH! How can it be a didn't think about this before? Type assertions can be done only on interfaces and *MyCustomError is not an interface!!"

Wile E. finally understands where the problem was and opens an issue about the NoErrorFunc2()

Before moving on, try to answer a couple of questions:

  1. What was the issue with NoErrorFunc2()?
  2. What is the content of the ISSUE raised by Wile E.?

Solution

When you use the := notation to create a new variable, the variable type is inferred from the right side of the assignment. If the right side of := is an interface, the type will be interface{}, otherwise it will be the type of value on the right side. Look at the following code:

package main

import (
    "fmt"
    "reflect"
)

type Error2 interface {
    error
}

type MyCustomError struct {
    msg1 string
}

func (e MyCustomError) Error() string {
    return e.msg1
}

// Return type is an interface
func NoErrorFunc1() error {
    return nil
}

// Return type is an interface
func NoErrorFunc2() Error2 {
    return nil
}

// Return type is a pointer to a struct (*MyCustomError)
func NoErrorFunc3() *MyCustomError {
    return nil
}

func main() {
    e := NoErrorFunc1()
    fmt.Println("NoErrorFunc1: ", reflect.TypeOf(e), e == nil)
    e = NoErrorFunc2()
    fmt.Println("NoErrorFunc2: ", reflect.TypeOf(e), e == nil)
    e = NoErrorFunc3()
    fmt.Println("NoErrorFunc3: ", reflect.TypeOf(e), e == nil)
}

=== OUTPUT ===
NoErrorFunc1:  <nil> true
NoErrorFunc2:  <nil> true
NoErrorFunc3:  *main.MyCustomError false

As you can see, if the returned type of a function is an interface and its value is nil, both type and value of the returned interface{} will be nil, so it will be considered nil.

On the other hand, If the returned type of a function is not an interface and its value is nil, the type inside the returned interface{} will be the type of the returned value, while the value will be nil; thus the interface{} won't be nil (an interface is considered nil if and only if both its embedded type and value are nil).

How to avoid it

Functions should always return error instead of returning the implementing object. Just changing the code to this makes it work:

package main

import (
    "fmt"
)

type MyCustomError struct {
    msg1 string
}

func (e MyCustomError) Error() string {
    return e.msg1
}

func NoErrorFunc1() error {
    return nil
}

func NoErrorFunc2() error { // We changed this to return `error` instead of `MyCustomError *`
    return nil
}

func main() {
    err := NoErrorFunc1()
    if err != nil {
        fmt.Println("Something went wrong in NoErrFunc1")
        return
    }

    err = NoErrorFunc2()
    if err != nil {
        fmt.Println("Something went wrong in NoErrFunc2")
        return
    }
    fmt.Println("Everything works great!")
}

If however you are not in control of the functions you are calling, then always double-check the type of the error returned by the functions you call

Answers

And now the answers to the questions!

What was the issue with NoErrorFunc2() ?
NoErrorFunc2 was returning an error as *MyCustomError. It needs to be changed to error.

What is the content of the ISSUE raised by Wile E. ?
Please, change the type of the error returned by NoErrorFunc2() to error

Issue History:

  • Wile E. created issue - 2022/07/04 1:48 PM
  • Road Runner made changes: status: new => status: in progress - 2022/07/04 1:48 PM
  • Road Runner made changes: status: in progress => status: resolved - 2022/07/04 1:48 PM
  • Road Runner added a comment: "Beep Beep!" - 2022/07/04 1:48 PM