-
Notifications
You must be signed in to change notification settings - Fork 18k
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
proposal: fmt: add an additional %j verb to wrap errors #58107
Comments
Ignoring for the moment any discussion of the need for the proposed functionality, it might suffice to implement some flags for the |
Is the proposed
|
Yes it's the same behaviour, to be honest I didn't know that flag on |
It's not clear to me where this proposal would be useful. If the error is coming from a third-party library (or even the standard library), what's their motivation for using If the error is coming from your own code, then you can just use your own error type instead of type wrappedError struct {
message string
err error
}
func (we wrappedError) Error() string {
return we.message
}
func (we wrappedError) Unwrap() error {
return we.err
}
func WrapError(message string, err error) error {
return wrappedError{message: message, err: err}
}
func updateProfile() error {
if err := storeProfile(); err != nil {
return WrapError("can't update profile", err)
}
return nil
} There's also the option to just put a newline in your format string, which might be sufficient for what you are trying to do anyway. func updateProfile() error {
if err := storeProfile(); err != nil {
return fmt.Errorf("can't update profile\n%w", err)
}
return nil
} |
Using
|
This seems too special. If you want custom semantics, it is easy to write a function or error implementation that creates those semantics. %w answers a fundamental question: does it unwrap or not. I don't think %j has a place here: it doesn't even format. |
This proposal has been added to the active column of the proposals project |
func storeProfile() error {
if err := writeOnDisk(); err != nil {
return fmt.Errorf("can't store profile %w", err)
} When wrapping an error (which you should definitely always do!) you should provide context which is not already available to the caller. In this case "store profile" is what the caller has called, so it's not useful as an error prefix. Instead, that prefix should be "can't write on disk" which is information that the caller has no access to. And every prefix should in general terminate with a colon, so in this case |
Based on the discussion above, this proposal seems like a likely decline. |
What about errors that wrap multiple errors ? Should we use multiple prefix ? It'll look very messy. |
No change in consensus, so declined. |
The problem
Currently the error handling in go is a bit weird to me. Let's see this with an example.
Imagine a simple program that updates a profile and then tries to store it on disk :
In this example, the write operation on disk fails and the error is surfaced an printed from the main function. If we try to log it we will have the following message:
It lacks a bit of context and it's not a good way to handle an error.
So a better way to handle the error would be to wrap it to add a bit of context :
With this program, we will have the following message :
Not very clear until we try to format it using the
fmt
package. But in my opinion it doesn't make sense to spend time to find out the right printing format for an error message and in the future it will be more difficult since soon we could wrap multiple errors.So let's imagine a basic (dirty) function that follow the error's chain and print them, like a stacktrace we will see the errors stacked from context to details:
Now if we try to print the previous error with this function we will have the following trace:
The trace is not very clear, the same error is surfaced several times because the
fmt
package add the child error message to its own.Actually the best trace one's could be expect is the following one:
The proposal
Add a
%j
verb in the fmt package that wrap an error but does not include it in the parent error message. With it my example will trace the following message :The text was updated successfully, but these errors were encountered: