skip to content

A refactor swapped %w for %v in one layer, errors.Is(err, ErrNotFound) stopped matching and 404s became 500s. How do you find and prevent that?

level: seniorimportance: should knowfreq 48%

answer

  1. the message text is a red herring
  2. print the chain, do not read the log
  3. where the walk stops is the culprit
  4. a test at the boundary, not a comment

basics

~20 s

Walk the returned error with errors.Unwrap and print each level's type: the walk stops at the layer that used %v, and that layer is the culprit. Restore %w, then pin the promise with a test asserting errors.Is at the package boundary.

solid answer

~50 s

Confirm the diagnosis instead of guessing from the message, which is unchanged: in a test or a throwaway helper, loop `errors.Unwrap` over the error the exported function returned and print `%T` at each level. The walk stops at the layer that formatted its cause with `%v`, and that is the layer to fix back to `%w`. Then make the regression impossible to repeat. Add a test in the package that owns the sentinel asserting `errors.Is(err, ErrNotFound)` holds for the error its exported function actually returns, plus a test at the transport layer that a not-found error maps to 404. Write the matchable errors into the package doc comment so `go doc` states the contract next to the code. In review, treat `%v` applied to an error as a deliberate decision to flatten that needs a stated reason, not as a formatting preference.

code

go · 5 lines
go
func printChain(err error) {
	for e := err; e != nil; e = errors.Unwrap(e) {
		fmt.Printf("%T: %v\n", e, e)
	}
}

go deeper

for a junior

Recall that errors.Is only finds a sentinel while every layer in between wrapped with %w, and that the printed message tells you nothing about whether that link survived.

for a middle

Be ready to walk the chain by hand with errors.Unwrap, print the type at each level, and explain precisely why identical text can hide a break in identity.

for a senior

Show the whole loop: confirm the break, fix the verb, add the boundary test that pins the contract, and state the review rule you apply to %v on an error value.

for a principal

Own that this was a silent breaking change to a published error contract, and decide how such contracts get documented, tested and versioned so a formatting edit cannot alter downstream behaviour again.

## Why this failure is invisible The symptom is a status-code change with no change in log text. A layer that used to write ``` return fmt.Errorf("load record %s: %w", id, err) ``` now writes `%v`. The message is byte-for-byte identical, so log-based monitoring, message-comparing tests and human eyes all see nothing. What changed is that the returned error no longer has an `Unwrap` method, so the chain ends there. The handler above it calls `errors.Is(err, ErrNotFound)`, gets false, falls into its default branch and returns 500 where it used to return 404. Generalise the shape: a chain is only as strong as its weakest link, and the break is silent by construction because Go's wrapping carries identity, not text. ## Step one: confirm, do not infer The fastest confirmation is to print the chain: ``` for e := err; e != nil; e = errors.Unwrap(e) { fmt.Printf("%T: %v\n", e, e) } ``` Run it against the error the exported function actually returns, not against something you construct in the test. You get one line per level and the list stops early. The last type printed is the layer that flattened the cause; everything under it is text only. If the list contains the sentinel, wrapping is fine and your bug is elsewhere, for example the handler comparing with `==` or matching the wrong sentinel. ## Step two: fix, which is trivial Restore `%w` in the offending call. That is the entire code fix, and it is why the interesting part of this question is everything around it. ## Step three: why the toolchain did not catch it Nothing in the compiler or `go vet` can know your intent. `%v` on an error is valid formatting used deliberately all over real code. `go vet`'s printf check reports a `%w` whose operand does not implement `error`; its `errorsas` check validates `errors.As` targets. Neither has any opinion about a `%w` that became a `%v`. A test is the only mechanism that encodes the intent, which means the absence of one is the real defect the incident exposed. ## Step four: pin the contract with a boundary test Write the test against the exported surface, not against internals: ``` func TestLoadRecordWrapsNotFound(t *testing.T) { _, err := repo.LoadRecord(context.Background(), "missing-id") if !errors.Is(err, repo.ErrNotFound) { t.Fatalf("want ErrNotFound in chain, got %v", err) } } ``` This fails the moment any layer in the return path flattens the chain, whichever layer it was, and it does not care how the message is worded. One such test per matchable sentinel is cheap and is the strongest documentation of the promise that exists, because it runs. Pair it with a test one level up: a not-found error from the service produces a 404 from the handler. Now the behaviour users see is also pinned, and a chain break fails two tests with clear names rather than showing up as a change in your error-rate graph. ## Step five: state the contract where callers read it The package's doc comment should say which errors callers may match on: ``` // LoadRecord returns ErrNotFound, wrapped, when no record has the given id. ``` `go doc` then puts the promise directly next to the code for anyone deciding what to depend on, and it gives a reviewer something concrete to check a change against. An undocumented chain is a contract nobody agreed to and everybody relies on. ## Step six: the review rule Adopt a stated rule: in code that returns errors from an exported function, `%v` on an error value is flattening and needs a reason. It is often the right choice, but it should be a decision, not a leftover. That rule is what makes this class of change visible in review, where it is a one-character conversation, instead of in production, where it is a status-code incident. ## What not to do - Do not have the handler string-match the message for "not found". It is untestable, it breaks on wording changes, and it entrenches the broken chain. - Do not paper over it by re-checking `errors.Is` at every layer. The fix is one verb; extra checks add noise and still fail when a new layer flattens. - Do not treat the swap as harmless once you have restored it. It was a breaking change to a published error contract, and the postmortem finding is that the contract was untested, not that someone typed the wrong letter.

  • Why did neither the compiler nor go vet catch this change?
    Neither can know intent. `%v` on an error is valid, common formatting. `go vet`'s printf check flags a `%w` whose operand does not implement `error`, and its errorsas check validates `errors.As` targets; nothing inspects a `%w` that became `%v`. The code compiles, vets and prints identically, so only a test that asserts identity can catch it.
  • What single test would have caught this before the refactor merged?
    A test at the package boundary rather than inside it: call the exported function for a missing id and assert `errors.Is(err, ErrNotFound)` is true. It fails whichever layer in the return path flattened the chain, it says nothing about message wording, and it documents the promise for the next person editing that code.
  • Is changing %w to %v ever a legitimate refactor?
    Yes, when a layer deliberately stops exposing a cause, for example a package that no longer wants callers matching on the storage error beneath it. But that is a breaking change to the package's error contract: it belongs in the doc comment and the release notes, and it should land with the boundary test deleted on purpose, not quietly inside a formatting cleanup.

saying these in an interview costs you the question

  • Diagnoses from the error message, which is unchanged
  • Blames errors.Is rather than the broken chain
  • Assumes go vet would have flagged the verb change
  • Fixes it by string-matching the message for not found
  • Adds errors.Is checks at every layer instead of restoring %w