Go Errors

errors.md

Golang Errors

https://blog.golang.org/go1.13-errors

Error Handling

Don’t just check errors, handle them gracefully

Anti-patterns: 🚫

// shadowing the error with some meaningless message
if err != nil {
  err = errors.New("this is fine 🔥")
  return "", err
}

// not adding context to the error
if err != nil {
  return "", err
}

// not wrapping the error with %w
if err != nil {
  return "", fmt.Errorf("trying to do x, err: %v", err)
  return "", fmt.Errorf("trying to do x, err: %s", err.Error())
}

Good practices: ✅

// add context and wrap the original error with %w
if err != nil {
  return "", fmt.Errorf("trying to do x, err: %w", err)
}

// if posible at this level log the error 
if err != nil {
  log.WithError(err).Error("trying to do x")
  return "", fmt.Errorf("trying to do x, err: %w", err)
}

Use this practice with care when returning internal errors to the end client. Returning the detailed error information may reveal implementation details, thus comprising security. Instead, use a good strategy for handling errors, e.g. log the detailed error information at the controller layer and then return an appropriate error message to the client.

When logging errors, be careful when logging at several layers and modules. This may lead to an excess of log messages. Instead, delegate the logging responsibility to a single module. The controller layer is a good place to do it, as all request pass through it.

Check and Return Errors, not Strings

Anti-patterns: 🚫

// fileA
func GetAllVideos(accID string) error {
  vs, err := dynamodb.GetVideos(accID)
  if err != nil {
    // ignorig/shadowing original error
    return fmt.Errorf("error getting videos for %s", accID)
  }
  // if the there is no error but there are no videos either
  if len(vs) > 0 { 
    return fmt.Errorf("not found videos for %s", accID)
  }
  // do something with obj
  return nil
}
// ------------------------------------------------------ //
// fileB
err := GetAllVideos(accID)
if err != nil {
  if strings.Contains(err.Error(), "not found") {
    // do alternative flow 
  } else {
    return err
  }
}

Good practices: ✅

// fileA
var (
  // always public package error variables
  ErrGetVideos      = errors.New("get-videos")
  ErrVideosNotFound = errors.New("videos-not-found")
)
func GetAllVideos(accID string) error {
  vs, err := dynamodb.GetVideos(accID)
  if err != nil { 
    // ⚠️ return specific error type and use original error as cause
    return fmt.Errorf("getting videos for %s, cause: %s, err: %w", accID, err, ErrGetVideos)
  }
  // if the there is no error but there are no videos either
  if len(vs) <= 0 {
    // ⚠️ return specific error type
    return fmt.Errorf("accID: %s, err: %w", accID, ErrVideosNotFound)
  }
  // do something with obj
  // ....
  return nil
}
// ------------------------------------------------------ //
// fileB
err := GetAllVideos(accID)
if err != nil {
  // ⚠️ check specific error
  if errors.Is(err, fileA.NotFoundVideosError) {
    // do alternative flow 
  } else {
    return fmt.Errorf("getting videos for flowB, accID: %s, err: %w", accID, err)
  }
}

UnitTests

Don’t use logic in unit tests

Anti-patterns: 🚫

// check expect error as bool
if tt.expectError {
  svc.On("Do", mock.Any).Return("", tt.ExpectedErrorResponse)
}

// check expect error as error
if tt.expectedError != nil {
  svc.On("Do", mock.Any).Return("", tt.expectedError)
}

// check expect service mock call
if tt.expectedServiceCall {
  if tt.ExpectServiceCallError {
    svc.On("Do", mock.Any).Return("", tt.ExpectedServiceErrorResponse)
  } else {
    svc.On("Do", mock.Any).Return(tt.ExpectedServiceResponse, nil)
  }
}

Good Practices: ✅

// just dont do it 😄 TBD

Additional notes: 📝

• It’s ok to create separate test scenarios for positive and negative behavior.

• Always do single-purpose tests.

Use expected type error instead of a boolean flag

Anti-pattern: 🚫

if tt.expectError {
  svc.On("Do", mock.Any).Return("", tt.ExpectedErrorResponse)
}

Good practice: ✅

func TestFoo_Error(t *testing.T) {
    type test struct {
        ServiceError error
        input        string
        ExpectedError error
    }
    tests := []test{
        {
          ServiceError: somepackage.AnotherError
          input: "data",
          ExpectedError: SomeError, 
        },
    }

    for _, tt := range tests {
        svc := mock.Service{}
        svc.On("Do", mock.Any).Return("", tt.ServiceError)
        
        foo := NewFoo(svc)
        err := foo(tt.input)
        
        assert.NotNil(t, err)
        assert.ErrorIs(t, err, tt.ExpectedError)
    }
}

Before the 1.7.0 version of the github.com/stretchr/testify library the recommended idiom to assert errors was assert.True(t, errors.Is(err, tt.ExpectedError)) But it’s better to > > > upgrade testify to the latest version and use the assert.ErrorIs function.

Use assertExpectations method

Always use assertExpectations to verify that the mocks are properly used whenever you are using

Good practice: ✅

// TestSomething is an example of how to use our test object to
// make assertions about some target code we are testing.
func TestSomething(t *testing.T) {
  testObj := new(MyMockedObject)
  testObj.On("DoSomething", 123).Return(true, nil)
  targetFuncThatDoesSomethingWithObj(testObj)

  // assert that the expectations were met
  testObj.AssertExpectations(t)
}

• Use constants for default values, like: anyAccountID, anyPlayback right.

• TBD: use test data files or arrays.

💡 If the function is hard to test (or mock), consider change the function.

Panic vs Fatal

The log.Panic vs log.Fatal is essentially panic vs os.Exit(1). The latter will not let deferred calls to run. Most of the time, it’s not what you want. It also makes testing much harder. It’s quite simple to stop panic in test by the means of recover. It’s much harder to cope with a code that does os.Exit somewhere while you’re trying to do end2end testing without loosing coverage info.