Don’t return err in Go

Instead, add missing details relevant for debugging.

Those who don’t understand Go errors, they like to complain that the language requires writing tons of “if err != nil { return err }” blocks. Problem is, this is a completely wrong way of handling errors in Go: return err is an antipattern.

Let me show what I mean on some sample code: a helper library for configuring an mTLS connection. (“Mutual TLS” is a way to prove to a server that a client is who they claim to be.)

package mtls

import (
	"crypto/tls"
	"crypto/x509"
	"fmt"
	"os"
)

type ClientConfig struct {
	CAPath   string
	KeyPath  string
	CertPath string
}

func (c *ClientConfig) BuildTLSConfig() (*tls.Config, error) {
	if *c == (ClientConfig{}) {
		return nil, fmt.Errorf("mtls: cannot build tls.Config from empty ClientConfig")
	}

	ret := &tls.Config{}
	if c.CAPath != "" {
		ca, err := os.ReadFile(c.CAPath)
		if err != nil {
			return nil, err // FIXME: BAD, antipattern
		}
		pool := x509.NewCertPool()
		pool.AppendCertsFromPEM(ca)
		ret.RootCAs = pool
	}
	if c.KeyPath != "" || c.CertPath != "" {
		cert, err := tls.LoadX509KeyPair(c.CertPath, c.KeyPath)
		if err != nil {
			return nil, err // FIXME: BAD, antipattern
		}
		ret.Certificates = []tls.Certificate{cert}
	}
	return ret, nil
}

With this poor example of error handling, what will be printed if we pass an invalid path “bad-cert.pem” in ClientConfig.CAPath?

ERROR: open bad-cert.pem: no such file or directory

Coming from a big codebase, this will be somewhat informative, but not much so. A lot of debugging would be needed to find out where exactly this error happened. Still, notice one thing: the standard library’s os.ReadFile() function tried to help us a bit here: it added the name of the bad-cert.pem file in the error message. This is a detail that would definitely be helpful for us in debugging. Can we be inspired by this behavior? Can we add some more details that would be helpful in debugging?

@@ -22,7 +22,7 @@ func (c *ClientConfig) BuildTLSConfig() (*tls.Config, error) {
        if c.CAPath != "" {
                ca, err := os.ReadFile(c.CAPath)
                if err != nil {
-                       return nil, err // FIXME: BAD, antipattern
+                       return nil, fmt.Errorf("mtls: building tls.Config from ClientConfig.CAPath: %w", err)
                }
                pool := x509.NewCertPool()
                pool.AppendCertsFromPEM(ca)
@@ -31,7 +31,7 @@ func (c *ClientConfig) BuildTLSConfig() (*tls.Config, error) {
        if c.KeyPath != "" || c.CertPath != "" {
                cert, err := tls.LoadX509KeyPair(c.CertPath, c.KeyPath)
                if err != nil {
-                       return nil, err // FIXME: BAD, antipattern
+                       return nil, fmt.Errorf("mtls: building tls.Config from ClientConfig.KeyPath & .CertPath: %w", err)
                }
                ret.Certificates = []tls.Certificate{cert}
        }

With this improved error handling code, what will be printed if we pass an invalid path “bad-cert.pem” in ClientConfig.CAPath?

ERROR: mtls: building tls.Config from ClientConfig.CAPath: open bad-cert.pem: no such file or directory

Proponents of exceptions may say, “this is so much manual writing labor, exception stack trace would automate that!” This is somewhat true. However, if looking at the manual labor as an investment, there is a couple advantages to Go’s approach over exceptions:

• by writing custom messages, we can provide more details useful for debugging (for example, the os.ReadFile helped us with a filename);
• by not relying on code line numbers, the messages are more long-lived, and can actually be understood and reasoned about in isolation, without access to the source code.

If we need to programatically detect a “file not found” error here, we can do it nicely with errors.Is, thanks to usage of %w in fmt.Errorf above:

if errors.Is(err, fs.ErrNotExist) {
	fmt.Println("err is File Not Found!")
}

For detecting more complex errors programatically, errors.As is the correct approach. If we want to generate such detectable errors, we will need to start defining our own error types instead of just using fmt.Errorf.