-
Notifications
You must be signed in to change notification settings - Fork 8
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
provide an errors package #5
Comments
Can it basically just wrap the pkg/errors package? |
It might yes. |
But the errors pacakge is also pretty tiny so may not be worth it, or maybe I'll see whats up with the Go2 proposal and use that api. |
I think it might be nice to try and switch to using go 1.13+ error wrapping/handling in place of using github.com/pkg/errors (https://blog.golang.org/go1.13-errors) I am a bit curious what context.Context is being used for where it would be needed for wrapped errors, wouldn't the caller generally be providing the context and be able to use it for logging purposes? |
@detiber the issue is that both If we had a context (not context.Context) that mutates its state instead of returning a copy the leaf function would be able to add |
Related to the issue of avoiding key conflicts on contexts, you can leverage a pointer variable to handle it, you can find an example of this in controller-runtime: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/log/log.go#L43, but that does assume using I'm likely missing some background on the types of issues being seen, but my (very likely naive) thought is that custom error types could accomplish the same with the use of errors.Is/errors.As (either built-in or pkg/errors). The type itself or additional fields on the type of the error should be able to provide additional context related to the conditions that caused the error if the caller cares about those things. |
It should mimic github.com/pkg/errors with the following additions:
add WithContext() and an accompanying method/interface to pull them out (so we can log them)
New/Errorf should also implement stackTracer
Should provide a StackTrace func that goes through all the errors implementing Cause and returns the last one that implements stackTracer
The text was updated successfully, but these errors were encountered: