Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Commit

Permalink
Add basic error stack (#642)
Browse files Browse the repository at this point in the history
* Add stack to all error reporting methods

... that could be found using this:

find . -name "*.go" -exec sed -i -e 's/return\(.*\) err$/return\1 errors.WithStack(err)/g' {} \;

See #641

* Added github.com/pkg/errors

See #641

* Fix missing import of github.com/pkg/errors

* Use github.com/pkg/errors.New() instead of standard errors.New()

* Use errors.Errorf() instead of fmt.Errorf()

* Use errors.Cause() in tests that check for a particular error type

* Error type switch based on cause of error

* Print bubbled up errors with stack
  • Loading branch information
kwk authored and aslakknutsen committed Jan 19, 2017
1 parent 7a5b4fd commit 3035502
Show file tree
Hide file tree
Showing 50 changed files with 269 additions and 204 deletions.
17 changes: 9 additions & 8 deletions account/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/almighty/almighty-core/gormsupport"
"github.com/goadesign/goa"
"github.com/jinzhu/gorm"
"github.com/pkg/errors"
uuid "github.com/satori/go.uuid"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -81,10 +82,10 @@ func (m *GormIdentityRepository) Load(ctx context.Context, id uuid.UUID) (*Ident
var native Identity
err := m.db.Table(m.TableName()).Where("id = ?", id).Find(&native).Error
if err == gorm.ErrRecordNotFound {
return nil, err
return nil, errors.WithStack(err)
}

return &native, err
return &native, errors.WithStack(err)
}

// Create creates a new record.
Expand All @@ -96,7 +97,7 @@ func (m *GormIdentityRepository) Create(ctx context.Context, model *Identity) er
err := m.db.Create(model).Error
if err != nil {
goa.LogError(ctx, "error adding Identity", "error", err.Error())
return err
return errors.WithStack(err)
}

return nil
Expand All @@ -109,11 +110,11 @@ func (m *GormIdentityRepository) Save(ctx context.Context, model *Identity) erro
obj, err := m.Load(ctx, model.ID)
if err != nil {
goa.LogError(ctx, "error updating Identity", "error", err.Error())
return err
return errors.WithStack(err)
}
err = m.db.Model(obj).Updates(model).Error

return err
return errors.WithStack(err)
}

// Delete removes a single record.
Expand All @@ -126,7 +127,7 @@ func (m *GormIdentityRepository) Delete(ctx context.Context, id uuid.UUID) error

if err != nil {
goa.LogError(ctx, "error deleting Identity", "error", err.Error())
return err
return errors.WithStack(err)
}

return nil
Expand All @@ -139,7 +140,7 @@ func (m *GormIdentityRepository) Query(funcs ...func(*gorm.DB) *gorm.DB) ([]*Ide

err := m.db.Scopes(funcs...).Table(m.TableName()).Find(&objs).Error
if err != nil && err != gorm.ErrRecordNotFound {
return nil, err
return nil, errors.WithStack(err)
}
return objs, nil
}
Expand All @@ -151,7 +152,7 @@ func (m *GormIdentityRepository) List(ctx context.Context) (*app.IdentityArray,

err := m.db.Model(&Identity{}).Order("full_name").Find(&rows).Error
if err != nil && err != gorm.ErrRecordNotFound {
return nil, err
return nil, errors.WithStack(err)
}
res := app.IdentityArray{}
res.Data = make([]*app.IdentityData, len(rows))
Expand Down
13 changes: 7 additions & 6 deletions account/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/almighty/almighty-core/gormsupport"
"github.com/goadesign/goa"
"github.com/jinzhu/gorm"
"github.com/pkg/errors"
uuid "github.com/satori/go.uuid"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -64,7 +65,7 @@ func (m *GormUserRepository) Load(ctx context.Context, id uuid.UUID) (*User, err
return nil, nil
}

return &native, err
return &native, errors.WithStack(err)
}

// Create creates a new record.
Expand All @@ -76,7 +77,7 @@ func (m *GormUserRepository) Create(ctx context.Context, u *User) error {
err := m.db.Create(u).Error
if err != nil {
goa.LogError(ctx, "error adding User", "error", err.Error())
return err
return errors.WithStack(err)
}

return nil
Expand All @@ -89,11 +90,11 @@ func (m *GormUserRepository) Save(ctx context.Context, model *User) error {
obj, err := m.Load(ctx, model.ID)
if err != nil {
goa.LogError(ctx, "error updating User", "error", err.Error())
return err
return errors.WithStack(err)
}
err = m.db.Model(obj).Updates(model).Error
if err != nil {
return err
return errors.WithStack(err)
}
return nil
}
Expand All @@ -108,7 +109,7 @@ func (m *GormUserRepository) Delete(ctx context.Context, id uuid.UUID) error {

if err != nil {
goa.LogError(ctx, "error deleting User", "error", err.Error())
return err
return errors.WithStack(err)
}

return nil
Expand All @@ -121,7 +122,7 @@ func (m *GormUserRepository) Query(funcs ...func(*gorm.DB) *gorm.DB) ([]*User, e

err := m.db.Scopes(funcs...).Table(m.TableName()).Find(&objs).Error
if err != nil && err != gorm.ErrRecordNotFound {
return nil, err
return nil, errors.WithStack(err)
}
return objs, nil
}
Expand Down
6 changes: 4 additions & 2 deletions application/transaction.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
package application

import "github.com/pkg/errors"

// Transactional executes the given function in a transaction. If todo returns an error, the transaction is rolled back
func Transactional(db DB, todo func(f Application) error) error {
var tx Transaction
var err error
if tx, err = db.BeginTransaction(); err != nil {
return err
return errors.WithStack(err)
}
if err := todo(tx); err != nil {
tx.Rollback()
return err
return errors.WithStack(err)
}
return tx.Commit()
}
5 changes: 3 additions & 2 deletions comment/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/almighty/almighty-core/gormsupport"
"github.com/goadesign/goa"
"github.com/jinzhu/gorm"
errs "github.com/pkg/errors"
uuid "github.com/satori/go.uuid"
)

Expand Down Expand Up @@ -55,7 +56,7 @@ func (m *GormCommentRepository) Create(ctx context.Context, u *Comment) error {
err := m.db.Create(u).Error
if err != nil {
goa.LogError(ctx, "error adding Comment", "error", err.Error())
return err
return errs.WithStack(err)
}

return nil
Expand Down Expand Up @@ -87,7 +88,7 @@ func (m *GormCommentRepository) List(ctx context.Context, parent string) ([]*Com

err := m.db.Table(m.TableName()).Where("parent_id = ?", parent).Order("created_at").Find(&objs).Error
if err != nil && err != gorm.ErrRecordNotFound {
return nil, err
return nil, errs.WithStack(err)
}
return objs, nil
}
Expand Down
5 changes: 5 additions & 0 deletions errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ const (
stNotFoundErrorMsg = "%s with id '%s' not found"
)

// Constants that can be used to identify internal server errors
const (
ErrInternalDatabase = "database_error"
)

type simpleError struct {
message string
}
Expand Down
2 changes: 2 additions & 0 deletions glide.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ import:
- package: github.com/mitchellh/mapstructure
- package: github.com/pelletier/go-toml
version: ^0.3.5
- package: github.com/pkg/errors
version: ^0.8.0
- package: github.com/spf13/afero
- package: github.com/spf13/cast
- package: github.com/spf13/jwalterweatherman
Expand Down
5 changes: 3 additions & 2 deletions gormapplication/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/almighty/almighty-core/workitem"
"github.com/almighty/almighty-core/workitem/link"
"github.com/jinzhu/gorm"
"github.com/pkg/errors"
)

// A TXIsoLevel specifies the characteristics of the transaction
Expand Down Expand Up @@ -160,12 +161,12 @@ func (g *GormDB) BeginTransaction() (application.Transaction, error) {
func (g *GormTransaction) Commit() error {
err := g.db.Commit().Error
g.db = nil
return err
return errors.WithStack(err)
}

// Rollback implements TransactionSupport
func (g *GormTransaction) Rollback() error {
err := g.db.Rollback().Error
g.db = nil
return err
return errors.WithStack(err)
}
5 changes: 3 additions & 2 deletions iteration/iteration.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/almighty/almighty-core/gormsupport"
"github.com/goadesign/goa"
"github.com/jinzhu/gorm"
errs "github.com/pkg/errors"
uuid "github.com/satori/go.uuid"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -61,7 +62,7 @@ func (m *GormIterationRepository) Create(ctx context.Context, u *Iteration) erro
err := m.db.Create(u).Error
if err != nil {
goa.LogError(ctx, "error adding Iteration", "error", err.Error())
return err
return errs.WithStack(err)
}

return nil
Expand All @@ -74,7 +75,7 @@ func (m *GormIterationRepository) List(ctx context.Context, spaceID uuid.UUID) (

err := m.db.Where("space_id = ?", spaceID).Find(&objs).Error
if err != nil && err != gorm.ErrRecordNotFound {
return nil, err
return nil, errs.WithStack(err)
}
return objs, nil
}
Expand Down
4 changes: 2 additions & 2 deletions jsonapi/error_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

"github.com/almighty/almighty-core/errors"
"github.com/goadesign/goa"
goaerrors "github.com/pkg/errors"
errs "github.com/pkg/errors"
"golang.org/x/net/context"
)

Expand Down Expand Up @@ -37,7 +37,7 @@ func ErrorHandler(service *goa.Service, verbose bool) goa.Middleware {
if e == nil {
return nil
}
cause := goaerrors.Cause(e)
cause := errs.Cause(e)
status := http.StatusInternalServerError
var respBody interface{}
respBody, status = ErrorToJSONAPIErrors(e)
Expand Down
17 changes: 9 additions & 8 deletions jsonapi/jsonapi_utility.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/almighty/almighty-core/app"
"github.com/almighty/almighty-core/errors"
"github.com/goadesign/goa"
pkgerrors "github.com/pkg/errors"
errs "github.com/pkg/errors"
)

const (
Expand All @@ -26,11 +26,12 @@ const (
// This function knows about the models package and the errors from there
// as well as goa error classes.
func ErrorToJSONAPIError(err error) (app.JSONAPIError, int) {
detail := err.Error()
cause := errs.Cause(err)
detail := cause.Error()
var title, code string
var statusCode int
var id *string
switch err.(type) {
switch cause.(type) {
case errors.NotFoundError:
code = ErrorCodeNotFound
title = "Not found error"
Expand All @@ -56,7 +57,7 @@ func ErrorToJSONAPIError(err error) (app.JSONAPIError, int) {
title = "Unknown error"
statusCode = http.StatusInternalServerError

cause := pkgerrors.Cause(err)
cause := errs.Cause(err)
if err, ok := cause.(goa.ServiceError); ok {
statusCode = err.ResponseStatus()
idStr := err.Token()
Expand Down Expand Up @@ -116,18 +117,18 @@ func JSONErrorResponse(x InternalServerError, err error) error {
switch status {
case http.StatusBadRequest:
if ctx, ok := x.(BadRequest); ok {
return ctx.BadRequest(jsonErr)
return errs.WithStack(ctx.BadRequest(jsonErr))
}
case http.StatusNotFound:
if ctx, ok := x.(NotFound); ok {
return ctx.NotFound(jsonErr)
return errs.WithStack(ctx.NotFound(jsonErr))
}
case http.StatusUnauthorized:
if ctx, ok := x.(Unauthorized); ok {
return ctx.Unauthorized(jsonErr)
return errs.WithStack(ctx.Unauthorized(jsonErr))
}
default:
return x.InternalServerError(jsonErr)
return errs.WithStack(x.InternalServerError(jsonErr))
}
return nil
}
13 changes: 7 additions & 6 deletions login/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package login

import (
"encoding/json"
"errors"
"fmt"
"log"
"net/http"
"sync"

errs "github.com/pkg/errors"

"github.com/almighty/almighty-core/account"
"github.com/almighty/almighty-core/app"
"github.com/almighty/almighty-core/configuration"
Expand Down Expand Up @@ -144,11 +146,10 @@ func (keycloak keycloakOAuthProvider) getUser(ctx context.Context, token *oauth2
client := keycloak.config.Client(ctx, token)
resp, err := client.Get(configuration.GetKeycloakEndpointUserinfo())
if err != nil {
return nil, err
return nil, errs.WithStack(err)
}

var user openIDConnectUser

json.NewDecoder(resp.Body).Decode(&user)

return &user, nil
Expand Down Expand Up @@ -179,13 +180,13 @@ type openIDConnectUser struct {
func ContextIdentity(ctx context.Context) (string, error) {
tm := ReadTokenManagerFromContext(ctx)
if tm == nil {
return "", errors.New("Missing token manager")
return "", errs.New("Missing token manager")
}
uuid, err := tm.Locate(ctx)
if err != nil {
// TODO : need a way to define user as Guest
log.Println("Geust User")
return "", err
fmt.Println("Guest User")
return "", errs.WithStack(err)
}
return uuid.String(), nil
}
Expand Down
Loading

0 comments on commit 3035502

Please sign in to comment.