﻿# Error handling in Go: Common pitfalls

Developers coming to Go from languages that use try/catch constructs, like Java or C\#, may feel a bit turned around\. The inner voice suggests using 'recover' with 'defer' as the nearest equivalent, but that's considered bad practice\. This article covers why, and looks at common error\-handling mistakes in Go\. 

![1371_Error_Handling_Issue/image1.png](https://import.viva64.com/docx/blog/1371_Error_Handling_Issue/image1.png)

## Introduction

The idea for this article came up spontaneously\. Error handling in Go is already well covered, so I wasn't planning to rehash the same points\.

But things changed while I was building diagnostic rules for PVS\-Studio's new Go analyzer\. Testing them on real open\-source projects, I kept running into the same error\-handling mistakes\. 

We've [written before](https://pvs-studio.com/en/blog/posts/go/1342/) about how we test the analyzer on large projects from GitHub, but this time, the findings were interesting enough to deserve their own article\. Hopefully it saves someone a headache\. But first, here's a quick refresher on how error handling works in Go, and the reasoning behind it\.

## Common approach to error handling in Go

Leaving out `try catch` wasn't an oversight—it was a deliberate design decision\. The authors believe that tying error handling to that control structure makes code more convoluted\. It also pushes developers to treat plain errors as exceptions\. You can read more about this [here](https://go.dev/doc/faq#exceptions)\.

Take a program that opens a file and modifies it\. Various scenarios are possible: a file might exist or might not\. Treating a missing file as an exception feels wrong\.

Go follows a simple principle: [an error is a value](https://go.dev/blog/errors-are-values)\. It's an ordinary value that a function returns explicitly, and the caller must explicitly handle it\. Look at the example:

```cpp
file, err := os.Open(filename)
if err != nil {
  // error handling
}
```

At first glance the code looks verbose, and it's obviously [boilerplate](https://en.wikipedia.org/wiki/Boilerplate_code)\. But the upside is that the control flow remains transparent, and the program is easy to read from top to bottom\.

But then why do we need `panic` and `recover`?

In Go, `panic` is a sign of that something went unexpectedly wrong\. In ways that should never happen in correct code: an out\-of\-bounds slice index, dereferencing a nil pointer, and so on\. The `recover` mechanism lets us intercept a `panic` before it brings the whole program down\. But using it as try/catch substitute goes against Go idioms and conventions\. 

That said, catching a `panic` does have its place\. The most common approach is at package or goroutine boundaries, so that a `panic` doesn't crash the entire program:

```cpp
func (e *encodeState) marshal(v any, opts encOpts) (err error) {
  defer func() {
    if r := recover(); r != nil {
      if je, ok := r.(jsonError); ok {
        err = je.error
      } else {
        panic(r)
      }
    }
  }()
  e.reflectValue(reflect.ValueOf(v), opts)
  return nil
}
```

This example comes from the standard `json` package, specifically the [`encode.go`](https://go.dev/src/encoding/json/encode.go) file\. Here, if a `panic` unwinds the stack up to the top\-level function call, the program recovers and returns an expected error value\. 

So `recover` is meant to prevent crashes, not to handle business logic\.

## How to make mistakes when handling errors

So, in Go errors are stored in a variable and checked against `nil`\. It seems simple, but even this straightforward approach leaves room for mistakes\. As an example, let's look at the [vault](https://github.com/hashicorp/vault) project—an open\-source tool for managing sensitive data\.

```cpp
func (c *Core) PersistTOTPKey(....) error {
  ks := &totpKey{
    Key: key,
  }
  val, err := jsonutil.EncodeJSON(ks)
  if err != nil {
    return err
  }
  if c.barrier.Put(ctx, &logical.StorageEntry{
    Key:   fmt.Sprintf(....),
    Value: val,
  }); err != nil {
    return err
  }
  return nil
}
```

The PVS\-Studio warning: [V8014](https://pvs-studio.com/en/docs/warnings/v8014/) Two 'if' statements have identical conditions\. The first 'if' statement contains function return, making the second 'if' redundant, or the code contains a logical error\. [login\_mfa\.go 1099](https://github.com/hashicorp/vault/blob/d73581e2356f73d0138b06da2e66ce5ebfdf1a0a/vault/login_mfa.go#L1099)

In this snippet, the error from `jsonutil.EncodeJSON(ks)` is checked\. Then, the same error is checked again in the next condition:

```cpp
if c.barrier.Put(ctx, &logical.StorageEntry{
        Key:   fmt.Sprintf(....),
        Value: val,
    }); err != nil {
        return err
    }
```

Clearly, there's no point in checking the same error twice\. Besides, if `errl != nil` is true, the method will return\. The developer most likely intended to capture a value from `Put` and check it\. What's more, the `Put` method returns an error value that is worth checking:

```cpp
type Storage interface {
  ....
  Put(context.Context, *StorageEntry) error
  ....
}
```

So, the fix might look like this:

```cpp
val, err := jsonutil.EncodeJSON(ks)
if err != nil {
  return err
}
if err := c.barrier.Put(ctx, &logical.StorageEntry{
  Key:   fmt.Sprintf(....),
  Value: val,
  }); err != nil {
    return err
}
```

The second `if` block checks the error that `Put` returns\.

In the [FerretDB](https://github.com/FerretDB/FerretDB/) project, which is an alternative to MongoDB, we spot a similar one:

```cpp
func (h *Handler) msgKillCursors(....) (*middleware.Response, error) {
  ....
  cursorsV, err := getRequiredParamAny(doc, "cursors")
  if err != nil {
    return nil, err
  }

  curArr, ok := cursorsV.(wirebson.AnyArray)
  if !ok {                                             // <=
    msg := fmt.Sprintf(....)
    return nil, lazyerrors.Error(....)
  }

  cursors, err := curArr.Decode()
  if !ok {                                             // <=
    return nil, lazyerrors.Error(err)
  }
  ....
}
```

The PVS\-Studio warning: [V8014](https://pvs-studio.com/en/docs/warnings/v8014/) Two 'if' statements have identical conditions\. The first 'if' statement contains function return, making the second 'if' redundant, or the code contains a logical error\. [msg\_killcursors\.go 61](https://github.com/FerretDB/FerretDB/blob/799235dab9e350655e72e65a0b24d849f7d68143/internal/handler/msg_killcursors.go#L61)

The value of the `err` variable changes here\. But instead, the code checks twice if the `cursorsV` variable is cast to the `wirenson.AnyArray` type\. Most likely, the second `!ok` was a mistake, and the code should look like this:

```cpp
cursors, err := curArr.Decode()
if err != nil {
  return nil, lazyerrors.Error(err)
}
```

This is another piece of code that accidentally re\-checks the same error, now in [Incus](https://github.com/lxc/incus)—a manager for virtual machines\.

```cpp
func (d *proxy) Start() (*deviceConfig.RunConfig, error) {
  ....
  err = p.Save(pidPath)
  if err != nil {
    // Kill Process if started, but could not save the file
    err2 := p.Stop()
    if err != nil {
      return fmt.Errorf("....: %s: %s", err, err2)
  }

    return fmt.Errorf("....: %w", d.name, err)
  }
  ....
}
```

The PVS\-Studio warning: [V8020](https://pvs-studio.com/en/docs/warnings/v8020/) Recurring check\. The 'err \!\= nil' condition was already verified on line 407 [proxy\.go 407](https://github.com/lxc/incus/blob/321c40f963e9c1e7b68736fb4941431cccac3d60/internal/server/device/proxy.go#L407)

The second condition should obviously check `err2` instead of `err`—`err` has already been checked and hasn't changed since\. Also, `err2` appears in the error handling:

```cpp
return fmt.Errorf("....: %s: %s", err, err2)
```

So, it makes sense to check it too\. PVS\-Studio flagged 65 warning of this type in the open\-source platform [Rancher](https://github.com/rancher/rancher)—more than anywhere else:

```cpp
func (c *MachineClient) ListAll(opts *types.ListOpts) (....) {
  resp := &MachineCollection{}
  resp, err := c.List(opts)
  if err != nil {
    return resp, err
  }
  data := resp.Data
  for next, err := resp.Next(); 
    next != nil && err == nil; next, err = next.Next() {
    data = append(data, next.Data...)
    resp = next
    resp.Data = data
  }
  if err != nil {
    return resp, err
  }
  return resp, err
}
```

The PVS\-Studio warning: [V8014](https://pvs-studio.com/en/docs/warnings/v8014/) Two 'if' statements have identical conditions\. The first 'if' statement contains function return, making the second 'if' redundant, or the code contains a logical error\. [zz\_generated\_cluster\.x\-k8s\.io\.machine\.go 113](https://github.com/rancher/rancher/blob/ec043c0cdf36b6bfe8ebc938c24af878a7a76d20/pkg/client/generated/cluster/v1beta2/zz_generated_cluster.x-k8s.io.machine.go#L113)

This one is tricky precisely because it looks right: the loop iterates as long as `next != nil` and `err == nil`, then check `err != nil` after the loop\. But there's a catch\. 

The analyzer points out that the `err` used inside the loop is not the same `err` that is checked after it\. The `err` checked after the loop is the original error from the earlier declaration:

```cpp
resp, err := c.List(opts)
```

If the loop terminates because `err != nil`, we'd want to output that error\. Instead, the code will return the error from the earlier declaration\. 

The fix is easy: check `err` inside the loop and return early\. This way, the method returns the error that actually caused the iteration to stop\.  

The number of repeated warnings is easy to explain: many types require the same code with minor changes, so the old code gets copied along with all its bugs\.

I got curious and traced the origins of this code, which lead me to this [commit](https://github.com/rancher/rancher/commit/3e5ab6a7baf58a2d63d1f07b9bdeb79b2c7d3875)\. It adds a whopping 307,251 lines of code, and the comment "Add generated code" settles it\. The bug was introduced and then spread across the entire project\.

## Conclusion

What's the takeaway for today? Go is unquestionably clearer and more explicit about error handling\. Still, it's possible to slip up—as the examples from open\-source GitHub projects show that\. Even in large and popular projects \(some have over 30,000 stars\), some very ordinary errors persist\.

This is a good reminder that no codebase is too big or too well\-known to benefit from an extra set of eyes\. That's where a static analyzer comes in handy\.


> We've recently launched the beta test for the new PVS\\\-Studio analyzers coming\\\. You are welcome to join it \[here\]\(https://pvs\-studio\.com/en/pvs\-studio\-eap/\)\\\! The EAP is available for JavaScript, TypeScript, and Go\\\.



Take care of yourself and your code\!