Golang Troubleshooting

// 1. Uninitialized struct field
type Server struct {
    logger *log.Logger  // nil if not set in constructor
}

// 2. Unchecked error return — if err != nil, val may be nil/zero
val, err := doSomething()
val.Method()  // panic if doSomething returned nil val with an error

// 3. Map lookup returns zero value
m := map[string]*Config{}
cfg := m["missing"]  // cfg is nil
cfg.Timeout  // panic

// 4. Type assertion without comma-ok
var i interface{} = "hello"
n := i.(int)        // panic
n, ok := i.(int)    // ok == false, no panic

type MyError struct{ msg string }
func (e *MyError) Error() string { return e.msg }

func doWork() error {
    var err *MyError  // typed nil pointer
    return err        // returns non-nil interface containing nil pointer!
}

func main() {
    if err := doWork(); err != nil {
        // This EXECUTES — the interface is non-nil
        fmt.Println(err)  // panic: nil pointer in Error()
    }
}

// FIX: return nil explicitly, not a typed nil variable
func doWork() error {
    return nil
}

// BAD
func doWork() error {
    var err error
    if condition {
        result, err := someFunc() // BUG: new err variable, doesn't set outer one
        if err != nil {
            return err
        }
        process(result)
    }
    return err // always nil — inner err was a different variable
}

// GOOD
func doWork() error {
    var err error
    if condition {
        var result ResultType
        result, err = someFunc() // assigns to outer err
        if err != nil {
            return err
        }
        process(result)
    }
    return err
}

// 1. Nil map write panics
var m map[string]int
m["key"] = 1  // panic: assignment to entry in nil map
// FIX: m := make(map[string]int)
// Note: nil map reads are fine — they return zero value

// 2. Append may share underlying array
a := []int{1, 2, 3}
b := a[:2]
b = append(b, 99)  // overwrites a[2]!
// FIX: full slice expression — b := a[:2:2] to limit capacity

// 3. Range variable capture in goroutine (Go < 1.22)
for _, v := range items {
    go func() {
        process(v)  // v is shared, will likely be last element
    }()
}
// FIX: pass as argument
for _, v := range items {
    go func(v Item) { process(v) }(v)
}
// In Go 1.22+, loop variables are per-iteration (no fix needed)

// 1. Arguments evaluated immediately
x := 1
defer fmt.Println(x)  // prints 1, not 2
x = 2

// 2. Defer in loop — doesn't run until function returns
for _, f := range files {
    file, _ := os.Open(f)
    defer file.Close()  // all Close() calls pile up until return
}
// FIX: wrap in closure
for _, f := range files {
    func() {
        file, _ := os.Open(f)
        defer file.Close()
        // use file
    }()
}

// 3. Named return + defer interaction
func readFile() (err error) {
    f, err := os.Open("file.txt")
    if err != nil { return }
    defer func() {
        if closeErr := f.Close(); err == nil {
            err = closeErr  // modifies named return
        }
    }()
    // ...
    return nil
}

// BAD — silent failure
result, _ := doSomething()
json.Unmarshal(data, &config)
http.ListenAndServe(":8080", nil)

// GOOD — handle or propagate
result, err := doSomething()
if err != nil {
    return fmt.Errorf("doSomething: %w", err)
}

go vet ./...

# More thorough
go get -tool github.com/kisielk/errcheck@latest
go tool errcheck ./...

return fmt.Errorf("reading config from %s: %v", path, err)  // BAD — loses error chain
return fmt.Errorf("reading config from %s: %w", path, err)  // GOOD — preserves Is/As

// Check for specific errors — use errors.Is, not ==
if err == sql.ErrNoRows { ... }            // BAD — breaks if wrapped
if errors.Is(err, sql.ErrNoRows) { ... }   // GOOD — traverses chain

// Extract typed errors
var pathErr *os.PathError
if errors.As(err, &pathErr) { ... }

// 1. Forgetting to cancel — leaks goroutines
ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
// Missing: defer cancel()

// 2. Using background context when you should propagate
go doWork(context.Background())  // BAD — can't cancel from parent
go doWork(ctx)                   // GOOD — respects parent cancellation

// 3. Not checking context error
err := doWork(ctx)
if err != nil {
    // Distinguish timeout from other errors
    if ctx.Err() == context.DeadlineExceeded {
        log.Printf("operation timed out")
    } else if ctx.Err() == context.Canceled {
        log.Printf("operation cancelled")
    } else {
        log.Printf("operation failed: %v", err)
    }
}

// 4. Background work outliving request context
func handler(w http.ResponseWriter, r *http.Request) {
    // BAD — background work uses request context that cancels when client disconnects
    go processAsync(r.Context(), data)

    // GOOD — derive a new context for background work (Go 1.21+)
    bgCtx := context.WithoutCancel(r.Context())
    go processAsync(bgCtx, data)
}

// BAD — fatal: concurrent map read and map write
m := make(map[string]int)
go func() { m["key"] = 1 }()  // concurrent write
go func() { _ = m["key"] }()  // concurrent read — fatal!

// GOOD — protect with mutex
var mu sync.RWMutex
m := make(map[string]int)
go func() { mu.Lock(); m["key"] = 1; mu.Unlock() }()
go func() { mu.RLock(); _ = m["key"]; mu.RUnlock() }()

// Or use sync.Map for read-heavy workloads with stable key sets

// BAD — value receiver copies the Mutex
type Counter struct {
    mu    sync.Mutex
    count int
}

func (c Counter) Increment() { // BUG: copies mutex on every call
    c.mu.Lock()
    c.count++
    c.mu.Unlock()
}

// GOOD — pointer receiver
func (c *Counter) Increment() {
    c.mu.Lock()
    defer c.mu.Unlock()
    c.count++
}

// BAD
var wg sync.WaitGroup
for i := 0; i < n; i++ {
    go func() {
        wg.Add(1) // BUG: may run after wg.Wait() returns
        defer wg.Done()
        doWork()
    }()
}
wg.Wait()

// GOOD
var wg sync.WaitGroup
for i := 0; i < n; i++ {
    wg.Add(1) // called BEFORE launching the goroutine
    go func() {
        defer wg.Done()
        doWork()
    }()
}
wg.Wait()

// BAD
func handler(w http.ResponseWriter, r *http.Request) {
    if !authorized(r) {
        http.Error(w, "Forbidden", http.StatusForbidden)
        // BUG: missing return — handler keeps executing
    }
    doSensitiveAction(r)
}

// GOOD
func handler(w http.ResponseWriter, r *http.Request) {
    if !authorized(r) {
        http.Error(w, "Forbidden", http.StatusForbidden)
        return
    }
    doSensitiveAction(r)
}

// BAD
var result map[string]interface{}
json.Unmarshal([]byte(`{"id": 1234567890123456789}`), &result)
id := result["id"].(int) // PANIC: it's float64, not int

// GOOD — use typed struct (preferred)
type Response struct {
    ID int64 `json:"id"`
}

// GOOD — use json.Number when you must use interface{}
dec := json.NewDecoder(bytes.NewReader(data))
dec.UseNumber()
var result map[string]interface{}
dec.Decode(&result)
id, _ := result["id"].(json.Number).Int64()

// BAD
type User struct {
    name  string `json:"name"`  // unexported — silently ignored!
    email string `json:"email"` // unexported — silently ignored!
}
u := User{name: "Alice", email: "[email protected]"}
data, _ := json.Marshal(u) // data is "{}" — no error

// GOOD
type User struct {
    Name  string `json:"name"`
    Email string `json:"email"`
}

// BAD
s := strings.Trim("application/json", "application/")
// Result: "js" — stripped all chars in set {a,p,l,i,c,t,o,n,/} from both ends!

// GOOD
s := strings.TrimPrefix("application/json", "application/")
// Result: "json"

s := "Hello, 世界"
fmt.Println(len(s))    // 13 (bytes), not 9 (characters)
fmt.Println(s[:8])     // "Hello, \xe4" — corrupted! cuts a multi-byte rune

// FIX: use utf8.RuneCountInString for character count
fmt.Println(utf8.RuneCountInString(s)) // 9

// FIX: convert to []rune for character-based slicing
runes := []rune(s)
fmt.Println(string(runes[:8])) // "Hello, 世"

// FIX: use for-range to iterate over characters (runes), not bytes
for _, r := range s { ... } // iterates runes

// BAD
for {
    select {
    case msg := <-ch:
        if msg == "quit" {
            break // BUG: only breaks the select, loop continues forever
        }
        process(msg)
    }
}

// GOOD — use labeled break
loop:
for {
    select {
    case msg := <-ch:
        if msg == "quit" {
            break loop // breaks the for loop
        }
        process(msg)
    }
}

// BAD
type Status int
const (
    Active   Status = iota // 0 — same as zero value!
    Inactive               // 1
)
type User struct {
    Status Status // zero value is Active — but was it intentional?
}

// GOOD — reserve 0 for "unknown"
type Status int
const (
    StatusUnknown  Status = iota // 0 — explicit unset sentinel
    StatusActive                 // 1
    StatusInactive               // 2
)

// BAD — recover() in main cannot catch panic in child goroutine
func main() {
    defer func() {
        if r := recover(); r != nil {
            fmt.Println("recovered:", r) // NEVER REACHED
        }
    }()
    go func() {
        panic("crash!") // crashes the whole program
    }()
    time.Sleep(time.Second)
}

// GOOD — each goroutine must recover its own panics
func main() {
    go func() {
        defer func() {
            if r := recover(); r != nil {
                log.Printf("goroutine recovered: %v", r)
            }
        }()
        panic("crash!") // recovered within this goroutine
    }()
    time.Sleep(time.Second)
}

// BAD — deferred cleanup never runs
func main() {
    f, _ := os.Create("data.tmp")
    defer f.Close()       // NEVER RUNS
    defer os.Remove(f.Name()) // NEVER RUNS

    if err := process(); err != nil {
        log.Fatal(err) // calls os.Exit(1) — skips all defers!
    }
}

// GOOD — return from main instead, or restructure so defers run
func main() {
    if err := run(); err != nil {
        fmt.Fprintf(os.Stderr, "error: %v\n", err)
        os.Exit(1) // defers in run() already ran when it returned
    }
}

func run() error {
    f, _ := os.Create("data.tmp")
    defer f.Close()
    return process()
}

// BAD — may fail even for the same instant
t1 := time.Now()
data, _ := t1.MarshalJSON()
var t2 time.Time
t2.UnmarshalJSON(data)
fmt.Println(t1 == t2) // false! t1 has monotonic, t2 doesn't

// GOOD — .Equal() ignores monotonic clock
fmt.Println(t1.Equal(t2)) // true

// Also: strip monotonic explicitly when storing/comparing
t1 = t1.Round(0) // strips monotonic reading

// BAD — connection leak if rows aren't closed
rows, err := db.Query("SELECT id FROM users")
if err != nil { return err }
for rows.Next() {
    // ...
}
// rows never closed — connection leak!

// GOOD — always defer Close
rows, err := db.Query("SELECT id FROM users")
if err != nil { return err }
defer rows.Close()
for rows.Next() {
    // ...
}
if err := rows.Err(); err != nil { // don't forget to check rows.Err()
    return err
}

// BAD — panic: send on closed channel
ch := make(chan int, 1)
close(ch)
ch <- 1 // panic!

// GOOD — only the sender should close, never the receiver
// Use a done channel or context to signal completion
func producer(ch chan<- int, done <-chan struct{}) {
    defer close(ch)
    for i := 0; ; i++ {
        select {
        case ch <- i:
        case <-done:
            return
        }
    }
}

// BAD — after ch is closed, this loops at 100% CPU
for {
    select {
    case v := <-ch: // fires continuously after ch closes
        process(v)   // processes zero values forever
    case <-done:
        return
    }
}

// GOOD — nil the channel after it closes
for {
    select {
    case v, ok := <-ch:
        if !ok {
            ch = nil // nil channel blocks forever in select — disables this case
            continue
        }
        process(v)
    case <-done:
        return
    }
}

// BAD — spins at 100% CPU waiting for a message
for {
    select {
    case msg := <-ch:
        process(msg)
    default:
        // runs immediately when ch has nothing — tight loop!
    }
}

// GOOD — remove default to block until a message arrives
for {
    select {
    case msg := <-ch:
        process(msg)
    case <-ctx.Done():
        return
    }
}

// GOOD — if you need non-blocking check, add a small sleep or ticker
for {
    select {
    case msg := <-ch:
        process(msg)
    default:
        time.Sleep(10 * time.Millisecond) // yield CPU
    }
}

// BAD — silent truncation
var big int64 = 256
small := int8(big)
fmt.Println(small) // 0 — silently overflowed!

var n int64 = math.MaxInt64
n32 := int32(n)
fmt.Println(n32) // -1 — silently wrapped!

// GOOD — check bounds before converting
func safeIntToInt32(n int64) (int32, error) {
    if n < math.MinInt32 || n > math.MaxInt32 {
        return 0, fmt.Errorf("value %d overflows int32", n)
    }
    return int32(n), nil
}

// BAD — user can escape the base directory
base := "/srv/files"
userInput := "../../etc/passwd"
path := filepath.Join(base, userInput)
// path = "/etc/passwd" — escaped!

// GOOD (Go 1.24+) — confine access to the base directory
root, err := os.OpenRoot("/srv/files")
if err != nil {
    return err
}
defer root.Close()
file, err := root.Open(userInput)
if err != nil {
    return err
}
defer file.Close()

func safePath(base, userInput string) (string, error) {
    if userInput == "" || filepath.IsAbs(userInput) || !filepath.IsLocal(userInput) {
        return "", fmt.Errorf("invalid relative path: %q", userInput)
    }

    path := filepath.Join(base, userInput)
    rel, err := filepath.Rel(base, path)
    if err != nil {
        return "", fmt.Errorf("checking path: %w", err)
    }
    if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
        return "", fmt.Errorf("path traversal attempt: %s", userInput)
    }

    return path, nil
}

type Sizer interface {
    Size() int
}

type File struct{ size int }
func (f *File) Size() int { return f.size } // pointer receiver

var s Sizer
s = File{}   // COMPILE ERROR: File does not implement Sizer (*File does)
s = &File{}  // OK — *File has the Size method

// This is because the compiler can't always take the address of a value
// (e.g., map values, return values). Pointer receiver = pointer required.

// BAD — recompiles regex on every call
func isEmail(s string) bool {
    re := regexp.MustCompile(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`)
    return re.MatchString(s)
}

// GOOD — compile once at package level
var emailRe = regexp.MustCompile(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`)

func isEmail(s string) bool {
    return emailRe.MatchString(s)
}

// BAD — init() depends on another init() having run first
var db *sql.DB

func init() {
    // Assumes config init() already ran — fragile!
    db, _ = sql.Open("postgres", config.DatabaseURL)
}

// GOOD — use explicit initialization
func main() {
    cfg := loadConfig()
    db := setupDatabase(cfg)
    startServer(db)
}

// BAD — output order is random every run
m := map[string]int{"a": 1, "b": 2, "c": 3}
for k, v := range m {
    fmt.Printf("%s=%d ", k, v) // different order each time!
}

// GOOD — sort keys when order matters
keys := make([]string, 0, len(m))
for k := range m {
    keys = append(keys, k)
}
sort.Strings(keys)
for _, k := range keys {
    fmt.Printf("%s=%d ", k, m[k])
}

// Surprising: fallthrough doesn't check the next condition
switch x := 5; {
case x > 10:
    fmt.Println(">10")
    fallthrough
case x > 0:
    fmt.Println(">0")
    fallthrough
case x < 0:
    fmt.Println("<0") // EXECUTES even though 5 is not < 0!
}
// Output: >0, <0

// fallthrough is rarely needed. Prefer listing multiple values:
switch status {
case "active", "enabled":
    enable()
}