Wrapping original issue in rest.Error

[Dragomir-Ivanov]

There is an rest.Error type that captures all unrecognized errors from Hooks:

    if err = rsrc.Update(ctx, item, original); err != nil {
        e = NewError(err)
        return e.Code, nil, e
    }

func NewError(err error) *Error {
switch err {
...
default:
        return &Error{520, err.Error(), nil}
    }
}

However sometimes, enriched errors are used(preserving call-trace), that become useless once stringed through err.Error(). Can we preseve original error like this:

// Error defines a REST error with optional per fields error details.
type Error struct {
    // Code defines the error code to be used for the error and for the HTTP
    // status.
    Code int
    // Message is the error message.
    Message string
    // Issues holds per fields errors if any.
    Issues map[string][]interface{}
    Err    error // Or even Errs []error
}

So I can use the errors in func (r ResponseFormatter) FormatError( and send them to say Sentry or logs.

This might be a breaking change if someone is not following go vet recommendations(like in rest-layer source code :)

[smyrman]

We could maybe change the conversion of errors to rest.Error to rely on errors.As (new in Go 1.13).

Then you can implement your own error types as needed, and implement the following method to asign itself to a rest.Error:

func (err MyError) As(target interface{}) bool {
    v, ok := target.(*http.Error)
    if !ok { return false }
    // set v. values
    return true
}

We could for conveniences provide something like this in the rest package:

type codeError struct{
   code int
   err error
}

func StatusError(code int, err error) error {
     return codeError{code: code, err: err}
}

func (err codeError) Unwrap() error {
    return err.err
}

func (err codeError) Error() string {
    if err.err == nil {
        return http.StatusText(err.code)
    }
    return err.Error()
}

func (err codeError) As(target interface{}) bool {
    v, ok := target.(*http.Error)
    if !ok { return false }
    v.Code = err.Code()
    v.Message = err.Error()
    return true

If we do the conversion to rest.Error in the error formatter, then your "original" error could be logged to sentry first.

[smyrman]

There are other ways to do this, this is just one method.

[Dragomir-Ivanov]

Thank you @smyrman for the detailed response. Let me think about it a little.

[smyrman]

Likewise we can match against errors from resource. (e.g. resource.ErrNotFound) using errors.Is, so that we will match even when they are wrapped by other errors, and thus be able to get a rest.Error instance with more context in the error message.

To wrap an error in the most plain way possible, you can use e.g. fmt.Errorf("prefix: %w", resource.ErrNotFound) or fmt.Errorf("%w: suffix", resource.ErrNotFound) in hooks etc.

Should change relevant places where errors are returned to return non-pointer errors.

[Dragomir-Ivanov]

Actually I am using another error package for hooks: github.com/rotisserie/eris, which can record file:location for each error invocation/wrap, thus getting exhaustive traces for debugging. Maybe we can still migrate rest-later to Go 1.13 errors, on order to propagate errors from hooks.

[Dragomir-Ivanov]

Hi @smyrman , sorry for the big delay. Regarding this issue, can you take a look at this patch: https://github.com/Dragomir-Ivanov/rest-layer/commit/2e9eb16c7b0b390fa41baedb3c4da03073b8ca8c

It seems to work, without introducing new types. We can change the NewError function name if you like.

[smyrman]

Which issue does this patch solve? It seams to return exactly the same as before, excpet that the NewError function returns two values, where one is now a generic error type?