The Day the Linter Broke My Code

Another Story…

Imagine you were given a task to design errors for your service.

• DataEOFError should contain the name of the data file and be considered equal to io.ErrUnexpectedEOF.
• ProcessingFailedError should contain a processing ID as a string and wrap a failure Reason error.
• Two different ProcessingFailedError instances should not be equal.

Ok, fine: easy.

You’ll write

type DataEOFError string

func (d DataEOFError) Error() string {
	return "unexpected EOF in file " + strconv.Quote(string(d))
}

func (DataEOFError) Is(err error) bool {
	return err == io.ErrUnexpectedEOF
}

var _ error = DataEOFError("")

The Is function is more or less the same as the example¹. The other error is a piece of cake:

type ProcessingFailedError struct {
	ID     string
	Reason error
}

func (p *ProcessingFailedError) Error() string {
	return fmt.Sprintf("processing %s failed: %s", p.ID, p.Reason)
}

func (p *ProcessingFailedError) Unwrap() error {
	return p.Reason
}

var _ error = &ProcessingFailedError{}

Just some trivial tests remain:

func TestDataEOFError(t *testing.T) {
	err := DataEOFError("data.bin")

	if !errors.Is(err, io.ErrUnexpectedEOF) {
		t.Errorf("Expected %q to be %q, but is not", err, io.ErrUnexpectedEOF)
	}
}

func TestProcessingFailedError(t *testing.T) {
	err1 := &ProcessingFailedError{ID: "1", Reason: DataEOFError("data1.bin")}

	if !errors.Is(err1, io.ErrUnexpectedEOF) {
		t.Errorf("Expected %q to be %q, but is not", err1, io.ErrUnexpectedEOF)
	}

	err2 := &ProcessingFailedError{ID: "2", Reason: DataEOFError("data2.bin")}

	if !errors.Is(err2, io.ErrUnexpectedEOF) {
		t.Errorf("Expected %q to be %q, but is not", err2, io.ErrUnexpectedEOF)
	}

	if err1 == err2 {
		t.Errorf("Expected %q != %q", err1, err2)
	}

	if errors.Is(err1, err2) {
		t.Errorf("Expected %q not to be %q, but is", err1, err2)
	}
}

Tests pass and everything looks great, so you send it to code review (Go Playground).

But Wait…

Your CI uses golangci-lint (a popular Go meta-linter) with the err113 linter enabled. The err113 linter discourages direct error comparisons. You’ll get:

golangci-lint run .
...:18:9: do not compare errors directly "err == io.ErrUnexpectedEOF", use "errors.Is(err, io.ErrUnexpectedEOF)" instead (err113)
    return err == io.ErrUnexpectedEOF
           ^
1 issues:
* err113: 1

This seems reasonable — after all, errors.Is() is generally the preferred way to check error equality in modern Go. You don’t even have to fix this manually; you’ll just run golangci-lint run -E err113 --fix ..

Now your tests are failing (Go Playground):

--- FAIL: TestProcessingFailedError (0.00s)
    ..._test.go:64: Expected "processing 1 failed: unexpected EOF in file \"data1.bin\"" not to be "processing 2 failed: unexpected EOF in file \"data2.bin\"", but is
FAIL

One of those days. The linter’s automatic fix broke your code. It started so simply, but what is happening?

When Linters Go Wrong

Sometimes the tools meant to help us write better code can lead us astray. A well-intentioned linter suggestion turned a perfectly correct implementation into a subtle bug that broke Go’s error handling semantics.

The issue here reveals a fundamental misunderstanding between what the linter expects and what Go’s error handling actually requires. The documentation¹ states: “An Is method should only shallowly compare err and the target and not call Unwrap on either.”

A “shallow compare” in this context means a direct, non-recursive check, typically err == target. The problem is that errors.Is() is not a “shallow compare”; it traverses the error chain by calling Unwrap when available, so it shouldn’t be used within an Is method. The errortype linter catches this:

...:18:9: An Is method should only shallowly compare err (et:unw+)

Understanding the Problem

To understand why this happens, we need to know that errors.Is() traverses an error’s “tree” — the chain of wrapped errors created by functions like fmt.Errorf("context: %w", err).

errors.Is() is not a symmetrical function (despite what its name might suggest). It’s designed this way to traverse err’s chain to find a specific target, not to compare two potentially wrapped error chains.

This means it’s possible that errors.Is(err, target) != errors.Is(target, err). So you would expect:

	wrapped := fmt.Errorf("wrapped: %w", err)

	fmt.Printf("%t, %t\n",
		errors.Is(wrapped, err), // true
		errors.Is(err, wrapped), // false
	)

to print “true, false” (Go Playground), since wrapped contains the underlying² err, but err does not contain wrapped in its chain.

When you call errors.Is() within an Is method, you violate this assumption. So in our broken example, this will print “true, true” (Go Playground).

In our test, errors.Is(err1, err2) traverses err1’s tree, calling the Is method on every error in it. This is fine until it reaches the wrapped DataEOFError. Its Is method then erroneously calls errors.Is(err2, io.ErrUnexpectedEOF), resulting in true. Correct would have been comparing err2 == io.ErrUnexpectedEOF, which is false.

So wrapping the broken DataEOFError has an effect on the whole error chain.

Real-World Example

This isn’t just a theoretical problem — it exists in production code. Let’s look at the fluxcd Controller Runtime:

// ErrKeyNotFound is returned when a required key is not found in a secret.
var ErrKeyNotFound = errors.New("key not found in secret")

// KeyNotFoundError is returned when a specific key is not found in a secret.
type KeyNotFoundError struct {
	Key    string
	Secret *corev1.Secret
}

func (e *KeyNotFoundError) Error() string {
	return fmt.Sprintf("secret '%s': key '%s' not found", client.ObjectKeyFromObject(e.Secret), e.Key)
}

func (e *KeyNotFoundError) Is(target error) bool {
	return errors.Is(target, ErrKeyNotFound)
}

(Source github.com/fluxcd/pkg/runtime/secrets/error.go)

package main

import (
	"errors"
	"fmt"

	"github.com/fluxcd/pkg/runtime/secrets"
	corev1 "k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func main() {
	stagingErr := getDBKey("staging", "dbkey-s")
	prodErr := getDBKey("production", "dbkey-p")

	if errors.Is(stagingErr, secrets.ErrKeyNotFound) {
		fmt.Println("key not found in staging")
	}

	if errors.Is(prodErr, secrets.ErrKeyNotFound) {
		fmt.Println("key not found in production")
	}

	if stagingErr == prodErr {
		fmt.Println("Unexpected error identity for different key/environment")
	}

	if errors.Is(stagingErr, prodErr) {
		fmt.Println("Unexpected matching errors for different key/environment")
	}
}

func getDBKey(namespace, key string) error {
	secret := &corev1.Secret{
		ObjectMeta: metav1.ObjectMeta{
			Name:      "db-credentials",
			Namespace: namespace,
		},
		Type: corev1.SecretTypeOpaque,
	}

	return &secrets.KeyNotFoundError{Key: key, Secret: secret}
}

As a result, errors.Is(stagingErr, prodErr) unexpectedly returns true.

This breaks error handling when you want to determine whether follow-up errors are caused by a specific root error. You can’t filter or group them reliably anymore because unrelated errors might be considered equivalent.

Conclusion

Our journey started with a seemingly helpful linter suggestion that broke our code by violating a fundamental principle of Go’s error handling: a custom Is method must perform a shallow, direct comparison. By calling errors.Is recursively, we broke its contract, leading to unexpected equivalences between distinct error instances. This is a subtle bug that can undermine the reliability of error checking, especially when trying to identify the root cause of a failure in a complex system.

To write robust and correct error-handling code, keep these key principles in mind:

• Implement Is correctly: Inside a custom Is method, use direct comparison (==) with the target. Never call errors.Is recursively.

• Linter rules aren’t always context-aware: The err113 rule is generally good advice, but it doesn’t understand the specific context of an Is method implementation, which requires direct comparison. A specialized linter like errortype can help catch these specific antipatterns.

• Test your errors: Write thorough tests for your error types, including negative cases (e.g., ensuring different error instances are not considered equal by errors.Is).

While linters are valuable tools, they can sometimes give advice that’s incorrect in specific contexts. The original design was correct — using direct comparison in an Is method follows Go’s error handling principles. The linter’s automatic fix introduced a bug by applying a general rule without understanding the specific requirements of implementing the Is interface.

By understanding the “why” behind the rules, you can use tools effectively without letting them lead you astray.

• ←Previous
• Next→