Skip to content
This repository was archived by the owner on Mar 12, 2020. It is now read-only.

Friendly errors#16

Open
mason-fish wants to merge 2 commits into
docker-archive:masterfrom
mason-fish:friendly_errors
Open

Friendly errors#16
mason-fish wants to merge 2 commits into
docker-archive:masterfrom
mason-fish:friendly_errors

Conversation

@mason-fish
Copy link
Copy Markdown
Contributor

Adds a generic LicensingError to facilitate friendlier error message returns, while also exposing a helper func GetErrorCause() to retrieve the more verbose original. The simpler error return also digs into the underlying cause to attempt to retrieve a "detail" message.

Copy link
Copy Markdown
Contributor

@dhiltgen dhiltgen left a comment

Choose a reason for hiding this comment

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

It feels like clients might still wind up parsing error strings to see what happened...

Comment thread client.go
}
type LicensingError struct {
Msg string
Cause error
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might be helpful to put some comments on this to explain what Msg and Cause are as well as how this LicensingError should be used by clients of the library.

Comment thread client.go
func (l *LicensingError) Error() string {
// make an attempt to recover more detail for more specificity otherwise fallback on msg
if detail, ok := errors.HTTPDetail(l.Cause); ok && detail != "" {
return fmt.Sprintf("%s: %s", l.Msg, detail)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you give some examples of how this might look?

I'm still trying to wrap my head around how clients of the lib will use this, and if there might be a simpler model here perhaps.

Comment thread client.go
return err, false
}

func (l *LicensingError) GetCause() error {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might want to consider implementing the Causer interface (which is what pkg/errors implement); https://github.com/pkg/errors/blob/master/errors.go#L32-L39

Comment thread lib/errors/herror.go
@@ -22,6 +24,35 @@ func HTTPStatus(err error) (status int, ok bool) {
return http.StatusInternalServerError, false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants