Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate call stack for sdk error and preserve stack trace #3310

Merged
merged 8 commits into from
Oct 5, 2018

Conversation

richardlt
Copy link
Member

@richardlt richardlt commented Sep 10, 2018

  1. Description
  • Printing call stack and stack trace automatically in wrapped errors.
  • Add UUID on error returned from API.
  • Show error uuid in ctl when an error occured.
  • Create admin endpoint to get error from Graylog by uuid.
  • Create ctl command to get error by uuid.
  1. Log strategy
  • For a simple error: catched by the router and associated with a unknown sdk.Error, then logged with a UUID.
  • For a sdk.Error: catched by the router the logged with a UUID.
  • For a wrapped simple error: same as simple error but logged with stack trace and call stack.
  • For a wrapped sdk.Error: same as sdk.Error but logged with stack trace and call stack.
  1. Err return strategy
  • All error from lib should be wrapped like sdk.WithStack(err) or sdk.WrapError(err, format, values...), WithStack and WrapError do not override existing callers on already wrapped errors. This will generate unknown http error (500).
  • Create new error from existing sdk.Error like sdk.NewError(ErrForbidden, err). This will generated a predefined http error with stack trace. New errors can also be wrapped to add more details.
  • Return sdk.Error{} or predefined sdk.Error like ErrForbidden. This will generated a http error that will be logged with a uuid but without stack trace or call stack.
  • If a simple error is return it will be catched be the router and returns as an unknown error (500).
    @ovh/cds

@richardlt richardlt changed the title WIP feat(sdk): Update new error and wrap to preserve root stack trace and… WIP Generate call stack for sdk error and preserve stack trace Sep 10, 2018
sdk/error.go Outdated
}
fallthrough
case 's':
io.WriteString(s, fmt.Sprintf("%s: %s", w.stack.String(), w.Cause().Error()))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error return value of io.WriteString is not checked

}

_ = WriteJSON(w, sdkErr, errProcessed.Status)
// safely ignore error returned by WriteJSON
WriteJSON(w, httpErr, httpErr.Status)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error return value of WriteJSON is not checked

@richardlt richardlt force-pushed the feat-error-with-stack branch 4 times, most recently from 438368e to 4960938 Compare September 28, 2018 11:47
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@ovh ovh deleted a comment from ovh-cds Sep 28, 2018
@richardlt richardlt force-pushed the feat-error-with-stack branch 2 times, most recently from 5dec0b0 to 7890e36 Compare September 28, 2018 15:08
@ovh-cds
Copy link
Collaborator

ovh-cds commented Sep 30, 2018

CDS Report build-ui#6240.0 ✘

  • Compile

    • Build UI ✔
    • TU All -
    • TU model ✔
    • TU service ✘
    • TU shared ✔
    • TU views ✔
      Unit Tests Report
  • HeadlessChrome 0.0.0 (Linux 0.0.0)

  • HeadlessChrome 0.0.0 (Linux 0.0.0).2185480

    • CDS: pipeline Store should create/update and delete a job ✘

    • CDS: project Store should add/update/delete a variable ✘

    • CDS: project Store should add/update/delete a permission ✘

  • HeadlessChrome 0.0.0 (Linux 0.0.0).2185482

  • HeadlessChrome 0.0.0 (Linux 0.0.0).2185481

@richardlt richardlt changed the title WIP Generate call stack for sdk error and preserve stack trace Generate call stack for sdk error and preserve stack trace Sep 30, 2018
Copy link
Member

@fsamin fsamin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,
you should have a look in the worker commands error management, it's a mess

cli/cobra.go Outdated
@@ -14,6 +14,7 @@ import (

"github.com/fsamin/go-dump"
"github.com/olekukonko/tablewriter"
"github.com/ovh/cds/sdk"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad import position

"time"

"github.com/gorilla/mux"
"github.com/ovh/cds/sdk"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad import position

sdk/error.go Show resolved Hide resolved
@bnjjj bnjjj merged commit dc10dfc into master Oct 5, 2018
@fsamin fsamin deleted the feat-error-with-stack branch November 15, 2018 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants