Replace pkg/errors with standard Go 1.13 error wrapping.

[Dirbaio]

Describe the bug

Fosite uses pkg/errors throughout the code to find the cause of errors. The issue is that errors.Cause does not unwrap errors wrapped in Go 1.13 style (Unwrap method), just the ones in custom pkg/errors style (Cause method). The maintainers have found that unwrapping both causes breaking changes, so they (understandably) have no plans to add that. See issue.

Additionally, the pkg/errors package is in maintenance mode, as stated in their readme.

To Reproduce

One example on where this can result in uninituitive behavior:

• From a fosite.Store method, return fosite.ErrNotFound, but somehow wrapped with a Go 1.13-compliant wrapper (maybe some extra context, maybe a library that attaches stack traces to the errors...)
• Fosite will check if errors.Cause(err) == fosite.ErrNotFound.
• errors.Cause(err) will not return the fosite.ErrNotFound because it can't unwrap "through" the Go1.13 wrapping. The check will fail.
• Fosite returns the wrong oauth error to the client

Ideally fosite should use Go 1.13 wrapping, because that's what the entire ecosystem is moving to. Of course that would be a breaking change but in my opinion it's worth doing pre-1.0 while you still can?

[aeneasr]

Yeah, I think errors.Is is better than errors.Cause. I still want to keep pkg/errors for error creation because Go std errors don't support stack traces.

[Dirbaio]

About stack traces: I also ran into issues with that because we're using a different stacktrace library, and it wasn't picking up fosite's.

Do you think it'd be possible to allow the user to customize which stacktracer is used?

Something like

fosite.SetStackTracer(func(err error) error {
   return errors.WithStack(err);
})

that would save that func in a global variable inside fosite, and all the places in fosite that want to wrap an error with a stack trace would call that.

Another advantage would be that users that don't want stacktraces (for performance) can do that too.

[mitar]

Hm, I also prefer keeping stack trace around.

I am using a custom function which inspects if something is causer or just wrapped when navigating errors and it works pretty well. Not sure why fosite would have to change here?

[aeneasr]

About stack traces: I also ran into issues with that because we're using a different stacktrace library, and it wasn't picking up fosite's.

Do you think it'd be possible to allow the user to customize which stacktracer is used?

Typically one would solve that with an interface / type assertion in Go and it would be expected for the downstream code to be able to handle that, not upstream.

[mitar]

So in #479 I changed that we now everywhere use errors.Is and errors.As. They use standard Go 1.13 Is and As internally, so this issue should be resolved now. (because pkg/errors is compatible with standard Is and As those calls will now traverse everything.)

Does this satisfy your needs @Dirbaio? Can you test?

[aeneasr]

I think it does! :)