AlgoMaster Logo

Common Go Mistakes

High Priority26 min readUpdated June 6, 2026

Go's surface is small, but a handful of patterns trip up nearly every developer who writes more than a few hundred lines of it. The mistakes in this chapter aren't exotic. They're the bugs that ship to production, cause weird panics during a Friday afternoon deploy, and show up in postmortems. Each one is presented as broken code, a fix, and a short explanation of why the language behaves that way.

Writing to a Nil Map

A map declared with var is nil. Reading from a nil map is fine (you get the zero value), but writing to one panics. The bug usually appears when a struct field is a map and the constructor forgets to allocate it.

Broken:

The zero value of map[string]int is nil, not an empty map. Writing to it has nowhere to go. The same bug happens with var m map[string]int; m["x"] = 1 outside any struct.

Fixed:

Allocate maps explicitly with make or a composite literal (map[string]int{}). For struct fields that need to be writable, do the allocation in the constructor.

Loop Variable Capture in Goroutines and Closures

Before Go 1.22, the loop variable in for i := range slice was reused across iterations. Goroutines or closures that captured i (or v) all saw the same variable, which usually held the last value by the time they ran.

Broken (pre-1.22 semantics):

On Go 1.21 and earlier, this often printed index=2 id=A-102 three times. Every goroutine read the same i and id, and by the time they ran, both held the final loop values.

Fixed (pre-1.22):

Go 1.22 changed the spec: loop variables now get a fresh scope per iteration. If your go.mod declares go 1.22 or higher, the original "broken" code works correctly. The shadowing trick still compiles, just does nothing useful. For older modules or for code you want to keep portable, keep the explicit shadow.

The 1.22 change makes existing code safer, but it also means a loop that captures a pointer to the loop variable now gets a different pointer each iteration. Code that relied on the old reuse for memory locality breaks subtly.

Goroutine Leaks

A goroutine that blocks forever on a channel receive or send never returns. The Go runtime can't garbage-collect a goroutine that's still running, so the goroutine, its stack, and everything it references stays in memory. Over hours this can fill a process with millions of dead goroutines.

Broken:

The five timeouts return quickly, but the five goroutines keep running, eventually try to send into channels nobody's reading from, and block forever. The channel is unbuffered, so the send blocks until a receiver shows up. None will.

The diagram shows the leak: the caller times out and forgets the worker exists, but the worker still tries to deliver its result on an unbuffered channel.

Fixed:

The buffer size of 1 lets the worker send its result whether or not anyone reads it. The goroutine completes and gets collected. For longer-running work, prefer a context.Context so the worker can detect cancellation and exit early instead of doing wasted work.

Every leaked goroutine costs at least ~2 KB of stack plus whatever it holds via closure variables. A leak at 100 RPS will exhaust memory in hours.

Variable Shadowing in if err Chains

Go's short variable declaration in an if statement creates a new variable scoped to the if block. When you reuse the name err in nested blocks, the inner err shadows the outer one. Code that looks like it propagates the error often drops it.

Broken:

The function reports success even though the charge failed. The if err := charge(id); err != nil line declares a new err that lives only inside the inner block. The outer err was set by validate and is still nil.

Fixed:

The pattern is to return early on every error rather than nesting and reassigning. go vet -shadow (available via golang.org/x/tools/cmd/shadow) flags suspicious shadows, but the discipline of "return early, don't reassign" is the real fix.

Slice Aliasing Surprises

A slice is a header (pointer, length, capacity) pointing to a backing array. Slicing it produces another header that shares the same backing array. Two slices can look independent but write to the same memory, with confusing results.

Broken:

preview had length 2 and capacity 4 (the full cart's capacity). The append had room in the existing backing array, so it wrote "stapler" at index 2, which overwrote "eraser" in the cart.

Fixed:

cart[:2:2] is the full slice expression: low, high, max. Capping the capacity at 2 forces append to allocate a fresh backing array, so writes to preview no longer touch cart.

append allocates a new array when the slice has no spare capacity, which copies every element. Repeatedly slicing and appending in hot paths can become O(n^2) if you don't pre-size or cap capacity.

Ignored Errors

Go's explicit error returns are a feature, not noise. Discarding them with _ or skipping the check looks tidy but hides real failures.

Broken:

If os.Open fails, in is nil. The defer in.Close() then panics, or worse, runs without issue and the code continues with corrupted state. The io.Copy call also swallows partial-write failures, which is how production "successful" backups end up with truncated files.

Fixed:

The exceptions to "always check errors" are narrow: fmt.Println to stdout (where a failed write is unrecoverable anyway) and bytes.Buffer.Write (which is documented to always return a nil error). For everything else, handle the error, even if "handle" means "wrap and return".

Defer in a Loop

defer queues the call to run when the surrounding function returns, not when the surrounding block ends. Deferring inside a loop means the deferred calls pile up and only fire after the entire function exits.

Broken:

If filenames has ten thousand entries, all ten thousand file descriptors stay open until processOrders returns. On most systems that hits the per-process file descriptor limit and later os.Open calls start failing with "too many open files".

Fixed:

Each call to processOne is its own function scope, so the defer f.Close() fires when that call returns, not when the outer loop finishes. The general rule: if you want defer to run at the end of one iteration, make that iteration its own function.

Mutating a Map While Ranging Over It

The Go spec is explicit: adding entries to a map during a range produces unspecified behavior. The iteration may visit the new entry, skip it, or even revisit older entries multiple times. Deleting the current key is allowed, but adding keys is not.

Broken:

Sometimes the output includes pen-restock, sometimes it doesn't, sometimes it shows up and then gets iterated over and triggers a third entry. The behavior depends on the map's internal layout and the iteration order, both of which are randomized.

Fixed:

Two passes: one to find candidates, one to apply changes. The same rule applies to slices if you're using indices that overlap with new appends. The cleanest pattern is to read in one loop and write in another.

A second pass is O(n) on top of the iteration, but it's predictable. The cost of an unspecified map mutation is hours of debugging when it goes wrong in production.

The Nil Interface Trap

An interface value holds two things: a concrete type and a value. The interface is nil only when both halves are nil. A typed nil pointer assigned to an interface produces a non-nil interface, which makes if err != nil checks lie.

Broken:

placeOrder returns a typed nil *OrderError. When that value is assigned to the error return type, the interface gets type *OrderError and value nil. The interface itself is not nil (the type half is set), so the err != nil check passes. The Error() method is then called on a nil receiver, which works in this case (it prints <nil>) but often panics.

Fixed:

The rule: never return a typed nil pointer as an interface. Declare the return type as the interface (error) and return the literal nil on success. If you absolutely have to return a typed value, check for nil explicitly inside the function before returning.

time.Parse and Timezones

time.Parse and time.Parse family use UTC unless the input includes a timezone offset. time.ParseInLocation lets you specify the zone. Most "wrong time" bugs come from forgetting which one to use.

Broken:

The user meant 9 AM Chicago time. time.Parse saw no timezone in the input, so it assumed UTC. Converting to Chicago time turned it into 4 AM, five hours off.

Fixed:

If the input has a zone offset like -05:00, time.Parse reads it. If not, time.Parse defaults to UTC and time.ParseInLocation defaults to the location you pass. For user-supplied timestamps without explicit zones, always go through ParseInLocation with the right zone, never assume UTC.

Taking the Address of a Loop Variable

Closely related to the loop variable capture issue, but a different bug. Even with Go 1.22's per-iteration scoping, code that takes a pointer to a loop variable, or passes it as a pointer to another function, can produce confusing results when the pointer outlives the iteration.

Broken:

On Go 1.21 and earlier, this prints the same order three times (the last one), because o was reused. On Go 1.22+, each iteration gets its own o, and the bug above disappears for this exact pattern. But the deeper issue remains: &o is a pointer to a copy of each slice element, not to the slice itself. Mutating *r won't change the original.

Fixed:

&orders[i] points directly into the slice's backing array. The references stay valid as long as the slice isn't reallocated. If you need pointers to elements, index into the slice rather than taking the address of the range variable.

Copying a sync.Mutex

A sync.Mutex is a value type whose zero value is an unlocked mutex. Copying a mutex value gives you two mutexes that don't know about each other, which defeats the point. Most copies happen by accident: passing a struct by value, returning a struct that contains a mutex, or ranging over a slice of structs with mutexes.

Broken:

The value receiver (c Counter) copies the entire struct, including the mutex, on every call. Each goroutine locks its own private copy of the mutex, increments its own private count, and the original counter is never touched. go vet catches this with the diagnostic "Increment passes lock by value: Counter contains sync.Mutex".

Fixed:

Any struct that contains a sync.Mutex, sync.RWMutex, sync.WaitGroup, or sync.Once should be used through a pointer. Methods on it should use pointer receivers, and APIs that hand it back to callers should return pointers, not values. Run go vet regularly; it catches the common cases automatically.

Unwrapping Errors: %w vs %v

Wrapping an error with fmt.Errorf and the %w verb preserves the original through errors.Is and errors.As. Using %v builds a new error whose message contains the original's text but discards the relationship. Code downstream that tries to test for errors.Is(err, sql.ErrNoRows) won't find what it's looking for.

Broken:

The error message reads correctly, but errors.Is can't follow it back to sql.ErrNoRows, so the "no such order" branch never runs. Callers can't tell missing rows apart from a network error.

Fixed:

Use %w when the inner error is a meaningful sentinel or type that callers might inspect. Use %v when you just want to format the error's string and don't care about identity. As a rule of thumb, %w is the right default; use %v only when you're deliberately hiding the inner error.

The diagram captures the two paths. With %w the wrapped error still answers to errors.Is. With %v the inner identity is gone, even if its message is preserved in the outer string.

Forgetting to Close db.Query Rows

db.Query returns a *sql.Rows that holds a connection from the pool. The connection isn't returned until you either iterate rows.Next() past the last row or call rows.Close() explicitly. Forgetting to close them leaks connections, eventually exhausting the pool.

Broken:

The early return after collecting 5 IDs leaves the rows iterator open. The connection sits with the result set forever until the garbage collector eventually runs the finalizer (which is best-effort and slow). Under load this pattern bleeds connections.

Fixed:

Two additions: defer rows.Close() runs on every exit path, and rows.Err() catches errors that happened during iteration (a row read failure won't make rows.Next() return an error; it returns false, and the actual error lives on rows). The same applies to *sql.Stmt (close prepared statements you no longer need) and *sql.Tx (always commit or roll back).

Premature Interface Declaration

Go's interfaces are satisfied implicitly. You don't need to declare one before writing the implementation. A common mistake, especially for developers coming from languages with explicit interface declarations, is to write an interface alongside its only implementation, "for testability" or "for flexibility".

Broken:

There's only one implementation, used in one place. The interface adds two indirection layers, one for every method dispatch, and a second file to keep in sync. It also encourages callers to depend on the abstract type when the concrete type would be more honest. The interface lives in the producing package, which is backwards: in Go, interfaces are best defined where they're used, not where they're implemented.

Fixed:

The interface (orderFinder) is small, local to the consumer, and contains only the methods the consumer actually uses. The producer just exposes a concrete *PostgresOrderRepository. When a test needs a fake, it implements the small interface, not the big one. The Go proverb is "the bigger the interface, the weaker the abstraction"; defining interfaces alongside implementations almost guarantees they're bigger than they need to be.

Quiz

Common Mistakes Quiz

10 quizzes