Skip to content

Commit

Permalink
feat(api,sdk): Fix some issue with error wrap (#3493)
Browse files Browse the repository at this point in the history
  • Loading branch information
richardlt authored and sguiheux committed Oct 29, 2018
1 parent 8f8749a commit 2e9a792
Show file tree
Hide file tree
Showing 53 changed files with 374 additions and 258 deletions.
76 changes: 76 additions & 0 deletions docs/content/contribute/error_management.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
+++
title = "Error management"
weight = 1

+++

This page explains how to deal with errors in CDS code. Error returned from CDS contains a message, an HTTP status code, a stack trace and a unique id.

Errors can be forwarded to a Graylog instance then retrieved directly from the ctl (see api.graylog and log.graylog sections in cds configuration file to setup).
```bash
cdsctl admin errors get <error_uuid>
```

## Usage in code

All errors from lib should be wrapped like **sdk.WithStack(err)** or **sdk.WrapError(err, format, values...)** directly when created.
```go
if err := json.Unmarshal(...); err != nil {
return sdk.WithStack(err) // or return sdk.WrapError(err, "Cannot unmarshal given data")
}
```

**WrapError** can be used to add more details about an error when returned.
```go
func one() error { return sdk.WithStack(json.Unmarshal(...)) }

func two() error { return sdk.WrapError(one(), "Error calling one") }

func three() error { return sdk.WrapError(two(), "Error calling two") }
```

If the error was already wrapped an not more info is needed you should run it directly.
```go
func four() error {
if err := three(); err != nil {
return err
}
...
}
```

To create an error that will generate a specific HTTP status code you should use the **sdk.NewError** func or returned an existing sdk.Error.
```go
if err := json.Unmarshal(...); err != nil {
return sdk.NewError(sdk.ErrWrongRequest, err) // returns a 400 http code with translated message and cause.
}

if err := json.Unmarshal(...); err != nil {
return sdk.WrapError(sdk.ErrWrongRequest, "Cannot unmarshal given data") // returns a 400 http code with translated message and cause.
}

if err := json.Unmarshal(...); err != nil {
return sdk.WithStack(sdk.ErrWrongRequest) // returns a 400 http code with translated message but no info about the source error.
}
```

To compare if an error match a existing sdk.Err use the **sdk.ErrorIs** func, using equality operator will not work if the error was wrapped.
A not wrapped lib error will match sdk.ErrUnknownError (to check if error is unknown you can use sdk.ErrorIsunknown).
```go
if err := one(); err != nil {
if sdk.ErrorIs(err, sdk.ErrNotFound) {
// do something specific for not found error
}
}

err := json.Unmarshal(...)
sdk.ErrorIs(err, sdk.ErrUnknownError) => true
sdk.ErrorIsUnknown(err) => true
```

To check if an error root cause is equal to a known library error you could use the **sdk.Cause** func.
```go
err := sdk.WrapError(sdk.WrapError(sql.ErrNoRows, "The error is now wrapped"), "Add more info on the error")
err == sql.ErrNoRows => false
sdk.Cause(err) == sql.ErrNoRows => true
```
2 changes: 1 addition & 1 deletion engine/api/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (api *API) deleteActionHandler() service.Handler {

a, errLoad := action.LoadPublicAction(api.mustDB(), name)
if errLoad != nil {
if errLoad != sdk.ErrNoAction {
if !sdk.ErrorIs(errLoad, sdk.ErrNoAction) {
log.Warning("deleteAction> Cannot load action %s: %T %s", name, errLoad, errLoad)
}
return errLoad
Expand Down
29 changes: 16 additions & 13 deletions engine/api/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,23 @@ func (api *API) adminTruncateWarningsHandler() service.Handler {
func (api *API) getAdminServicesHandler() service.Handler {
return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
srvs := []sdk.Service{}

var err error
if r.FormValue("type") != "" {
srvs, err = services.FindByType(api.mustDB(), r.FormValue("type"))
} else {
srvs, err = services.All(api.mustDB())
}

if err != nil {
return sdk.WrapError(err, "getAdminServicesHandler")
return err
}

for i := range srvs {
srv := &srvs[i]
srv.Hash = ""
srv.Token = ""
}

return service.WriteJSON(w, srvs, http.StatusOK)
}
}
Expand All @@ -51,15 +53,15 @@ func (api *API) getAdminServiceHandler() service.Handler {
name := vars["name"]
srv, err := services.FindByName(api.mustDB(), name)
if err != nil {
return sdk.WrapError(err, "getAdminServiceHandler")
return err
}
srv.Hash = ""
srv.Token = ""
if srv.GroupID != nil {
g, err := group.LoadGroupByID(api.mustDB(), *srv.GroupID)
if err != nil {
if err != sdk.ErrGroupNotFound {
return sdk.WrapError(err, "getAdminServiceHandler")
if !sdk.ErrorIs(err, sdk.ErrGroupNotFound) {
return sdk.WithStack(err)
}
} else {
srv.Group = g
Expand Down Expand Up @@ -89,17 +91,19 @@ func selectDeleteAdminServiceCallHandler(api *API, method string) service.Handle
return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
srvs, err := services.FindByType(api.mustDB(), r.FormValue("type"))
if err != nil {
return sdk.WrapError(err, "selectDeleteAdminServiceCallHandler")
return err
}
if len(srvs) == 0 {
return sdk.WrapError(sdk.ErrNotFound, "No hooks service found")
}

query := r.FormValue("query")
btes, code, err := services.DoRequest(ctx, srvs, method, query, nil)
if err != nil {
sdkErr := sdk.Error{
return sdk.NewError(sdk.Error{
Status: code,
Message: err.Error(),
}
return sdkErr
}, err)
}

log.Debug("selectDeleteAdminServiceCallHandler> %s : %s", query, string(btes))
Expand All @@ -113,7 +117,7 @@ func putPostAdminServiceCallHandler(api *API, method string) service.Handler {
return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
srvs, err := services.FindByType(api.mustDB(), r.FormValue("type"))
if err != nil {
return sdk.WrapError(err, "putPostAdminServiceCallHandler")
return err
}

query := r.FormValue("query")
Expand All @@ -125,11 +129,10 @@ func putPostAdminServiceCallHandler(api *API, method string) service.Handler {

btes, code, err := services.DoRequest(ctx, srvs, method, query, body)
if err != nil {
sdkErr := sdk.Error{
return sdk.NewError(sdk.Error{
Status: code,
Message: err.Error(),
}
return sdkErr
}, err)
}

return service.Write(w, btes, code, "application/json")
Expand Down
4 changes: 2 additions & 2 deletions engine/api/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (api *API) getApplicationsHandler() service.Handler {
//Load the specific user
u, err = user.LoadUserWithoutAuth(api.mustDB(), requestedUserName)
if err != nil {
if err == sql.ErrNoRows {
if sdk.Cause(err) == sql.ErrNoRows {
return sdk.ErrUserNotFound
}
return sdk.WrapError(err, "unable to load user '%s'", requestedUserName)
Expand Down Expand Up @@ -605,7 +605,7 @@ func (api *API) deleteApplicationHandler() service.Handler {

app, err := application.LoadByName(api.mustDB(), api.Cache, projectKey, applicationName, getUser(ctx))
if err != nil {
if err != sdk.ErrApplicationNotFound {
if !sdk.ErrorIs(err, sdk.ErrApplicationNotFound) {
log.Warning("deleteApplicationHandler> Cannot load application %s: %s\n", applicationName, err)
}
return err
Expand Down
2 changes: 1 addition & 1 deletion engine/api/application/dao.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func unwrap(db gorp.SqlExecutor, store cache.Store, u *sdk.User, opts []LoadOpti
}

for _, f := range opts {
if err := (*f)(db, store, &app, u); err != nil && err != sql.ErrNoRows {
if err := (*f)(db, store, &app, u); err != nil && sdk.Cause(err) != sql.ErrNoRows {
return nil, sdk.WrapError(err, "application.unwrap")
}
}
Expand Down
28 changes: 14 additions & 14 deletions engine/api/application/dao_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ import (

var (
loadDefaultDependencies = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
if err := loadVariables(db, store, app, u); err != nil && err != sql.ErrNoRows {
if err := loadVariables(db, store, app, u); err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "application.loadDefaultDependencies %s", app.Name)
}
if err := loadTriggers(db, store, app, u); err != nil && err != sql.ErrNoRows {
if err := loadTriggers(db, store, app, u); err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "application.loadDefaultDependencies %s", app.Name)
}
return nil
}

loadVariables = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
variables, err := GetAllVariableByID(db, app.ID)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load variables for application %d", app.ID)
}
app.Variable = variables
Expand All @@ -35,7 +35,7 @@ var (

loadVariablesWithClearPassword = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
variables, err := GetAllVariableByID(db, app.ID, WithClearPassword())
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load variables for application %d", app.ID)
}
app.Variable = variables
Expand All @@ -44,7 +44,7 @@ var (

loadPipelines = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
pipelines, err := GetAllPipelinesByID(db, app.ID)
if err != nil && err != sdk.ErrNoAttachedPipeline {
if err != nil && !sdk.ErrorIs(err, sdk.ErrNoAttachedPipeline) {
return sdk.WrapError(err, "Unable to load pipelines for application %d", app.ID)
}
app.Pipelines = pipelines
Expand All @@ -61,13 +61,13 @@ var (
appPip := &app.Pipelines[i]
var err error
appPip.Triggers, err = trigger.LoadTriggersByAppAndPipeline(db, app.ID, appPip.Pipeline.ID)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load trigger for application %d, pipeline %s(%d)", app.ID, appPip.Pipeline.Name, appPip.Pipeline.ID)
}
for i := range appPip.Triggers {
trig := &appPip.Triggers[i]
a, err := LoadByID(db, store, trig.DestApplication.ID, u, &loadPipelines)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load trigger for application %d, pipeline %s(%d)", app.ID, appPip.Pipeline.Name, appPip.Pipeline.ID)
}
trig.DestApplication = *a
Expand All @@ -85,7 +85,7 @@ var (
}

loadGroups = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
if err := LoadGroupByApplication(db, app); err != nil && err != sql.ErrNoRows {
if err := LoadGroupByApplication(db, app); err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load group permission for application %d", app.ID)
}
return nil
Expand All @@ -99,7 +99,7 @@ var (

loadHooks = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
h, err := hook.LoadApplicationHooks(db, app.ID)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load hooks for application %d", app.ID)
}
app.Hooks = h
Expand All @@ -109,7 +109,7 @@ var (
loadNotifs = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
var err error
app.Notifications, err = notification.LoadAllUserNotificationSettings(db, app.ID)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load notifications for application %d", app.ID)
}
return nil
Expand All @@ -118,7 +118,7 @@ var (
loadDeploymentStrategies = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
var err error
app.DeploymentStrategies, err = LoadDeploymentStrategies(db, app.ID, false)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load deployment strategies for application %d", app.ID)
}
return nil
Expand All @@ -127,7 +127,7 @@ var (
loadIcon = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
var err error
app.Icon, err = LoadIcon(db, app.ID)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load icon")
}
return nil
Expand All @@ -136,7 +136,7 @@ var (
loadVulnerabilities = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
var err error
app.Vulnerabilities, err = LoadVulnerabilities(db, app.ID)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load vulnerabilities")
}
return nil
Expand All @@ -145,7 +145,7 @@ var (
loadDeploymentStrategiesWithClearPassword = func(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u *sdk.User) error {
var err error
app.DeploymentStrategies, err = LoadDeploymentStrategies(db, app.ID, true)
if err != nil && err != sql.ErrNoRows {
if err != nil && sdk.Cause(err) != sql.ErrNoRows {
return sdk.WrapError(err, "Unable to load deployment strategies for application %d", app.ID)
}
return nil
Expand Down
14 changes: 6 additions & 8 deletions engine/api/application_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,14 @@ func (api *API) postApplicationImportHandler() service.Handler {

newApp, msgList, globalError := application.ParseAndImport(tx, api.Cache, proj, eapp, force, project.DecryptWithBuiltinKey, getUser(ctx))
msgListString := translate(r, msgList)

if globalError != nil {
myError, ok := globalError.(sdk.Error)
if ok {
log.Warning("postApplicationImportHandler> Unable to import application %s : %v", eapp.Name, myError)
sdkErr := sdk.ExtractHTTPError(myError, r.Header.Get("Accept-Language"))
msgListString = append(msgListString, sdkErr.Message)
return service.WriteJSON(w, msgListString, sdkErr.Status)
globalError = sdk.WrapError(globalError, "Unable to import application %s", eapp.Name)
if sdk.ErrorIsUnknown(globalError) {
return globalError
}
return sdk.WrapError(globalError, "postApplicationImportHandler> Unable import application %s", eapp.Name)
log.Warning("%v", globalError)
sdkErr := sdk.ExtractHTTPError(globalError, r.Header.Get("Accept-Language"))
return service.WriteJSON(w, append(msgListString, sdkErr.Message), sdkErr.Status)
}

if err := tx.Commit(); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion engine/api/auth/ldapclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ func (c *LDAPClient) searchAndInsertOrUpdateUser(db gorp.SqlExecutor, username s

//User
var newUser = false
if err == sql.ErrNoRows {
if sdk.Cause(err) == sql.ErrNoRows {
newUser = true
u = &sdk.User{
Admin: false,
Expand Down
2 changes: 1 addition & 1 deletion engine/api/auth/persistent_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func getUserPersistentSession(ctx context.Context, db gorp.SqlExecutor, store se
token, err := user.LoadPersistentSessionToken(db, key)
if err != nil {
log.Warning("getUserPersistentSession> Unable to load user by token %s: %v", key, err)
if err == sql.ErrNoRows {
if sdk.Cause(err) == sql.ErrNoRows {
if err := store.Delete(key); err != nil {
log.Error("getUserPersistentSession> Unable to delete session %v", key)
}
Expand Down
2 changes: 1 addition & 1 deletion engine/api/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func (api *API) getPipelineBuildJobHandler() service.Handler {

j, err := pipeline.GetPipelineBuildJob(api.mustDB(), api.Cache, id)
if err != nil {
if err == sql.ErrNoRows {
if sdk.Cause(err) == sql.ErrNoRows {
err = sdk.ErrPipelineBuildNotFound
}
return sdk.WrapError(err, "Unable to load pipeline build job id")
Expand Down
2 changes: 1 addition & 1 deletion engine/api/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (api *API) deleteEnvironmentHandler() service.Handler {

env, errEnv := environment.LoadEnvironmentByName(api.mustDB(), projectKey, environmentName)
if errEnv != nil {
if errEnv != sdk.ErrNoEnvironment {
if !sdk.ErrorIs(errEnv, sdk.ErrNoEnvironment) {
log.Warning("deleteEnvironmentHandler> Cannot load environment %s: %s\n", environmentName, errEnv)
}
return errEnv
Expand Down
Loading

0 comments on commit 2e9a792

Please sign in to comment.