I started to write this up here, but realized it would make a good blog post, so wrote it on my blog: https://npf.io/2015/10/errors/
The tl;dr: figuring out how to return errors from sub-functions is hard… do you use fmt.Errorf to add context, or return the original error to retain type information? Or use a third party library to wrap your errors?
I think the right answer is - it depends. There’s no one right answer for all code.
What do you do? Do you usually just return err? Do you use a custom errors library?
Perhaps, when there is context to add to a notExistError, a function returning the error adds a layer of abstraction, and thus telling users that they should use os.IsNotExists for the returned error is an abstraction leak. I’d define a new IsSomethingErr func that matches the abstraction layer
service, err := StartService()
// opens a config file
// but at this level of abstraction
// we don't know that configs
// are stored on file system.
// maybe in future versions
// it will be fetched via http.
if IsConfigNotExistErr(err) {
...
}
I am not sure I fully understand your func (c Config) Save() error example. What is checkDefault there?
Hey @NateFinch,
I’ve used juju’s errgo library awhile back and it was helpful. Thank you.
I don’t use custom errors libraries when writing a library but do indulge myself when writing tools.
From a library consumer perspective, I like too receive typed errors (from the package they originate) or a named error for stuff like EOF. Then handle every possible error state close to the origin. That makes context less important.
And lately I’ve been experimenting by enabling my typed errors with methods such as Retry, Fatal, etc… Which adds functional context to the error too.
At the risk of blowing my own horn, I believe that if you want to inspect errors, you should do so by behavior not type. Specifically the concept of returning typed errors is brittle and fails completely when errors are gift wrapped, so should be avoided.
I believe that errors are just values and they can be anything they need to be. Adding behavior can be critical. Also, the API has a responsibility to provider the user enough context about the error so they can make an informed decision about the integrity of the system and how to move on.
Hi Bill, I think you’re arguing the opposite of my position. I hold that the caller should expect the error value returned to be opaque, we don’t know its type, or its value, only that it conforms to the error interface and the fact that it is non nil means something went wrong.
From that I argue that rather than attempting to inspect this error by type, ie assert it to a specific type, then mess around in its innards to make some judgement about the seriousness of the error, callers should, if they need to inspect the error by behaviour, ie. assert the error conforms to an interface, and then if it does, use the methods on that interface – the behaviour, to base their judgement of the error.
The use of libraries like Canonical’s errgo and errors package (please don’t ask why we have two libraries for apparently the same thing), make it impossible for the caller to retrieve the original error value, thereby making it impossible to type assert and do that dance; however I believe it is possible for gift wrapping libraries to permit their error wrapper types to implement the common interface behaviours, and thus support this assertion by behaviour model I propose.
Hi Dave, I agree with the model you propose and have been experimenting with it without knowing how to call it.
Maybe I stumbled upon it through some of your writing but haven’t found much else.
And with common terms like errors, types, interfaces I don’t wonder why but i would like to see it become an idiom.
With regards to the gift wrapping libraries. When I create custom typed errors I export an unwrap interface and the method.
That way I can always assert for the interface and if it is satisfied unwrap the underlying error but there’s probably a better way.
Please take a look at the example of *url.Error implementing net.Error. In that case there is no requirement to unwrap, *url.Error is a net.Error implementation so it has Timeout and Temporary behaviours and you can use those directly.
I love the Temporary() method that is provided by the net package on the custom error types. I don’t care what the type of error is being returned, I just want to know if the error is a temporary error or not. Or I need to know if we are at EOF. This is the best, does the error contain this behavior or does the error represent this state.
I break things down in four parts of context:
Error as context, when only one kind of error will be returned.
Error variables as context, when multiple kinds of errors will be returned.
Type as context, when multiple kinds of errors will be returned and error variables does provide enough context.
Behavior as context, when errors provide behaviors that provide the information you need.
I think the position is to avoid having the user use “type as context” and try to add behaviors to your custom error types. I agree with that but I would just add I would not want to force behavior on the custom error type just to have this. But maybe in the future I will come to realize that every custom error type can have behavior that is not forced and adds value to the user.
In practice I don’t think this is a large set of behaviours. In fact, so far every time is issue has come up the reason has been the ability to detect and not log temporary errors.
So, sure, you can have a wrapper that implements Temporary() and Timeout(), but you can’t have a wrapper that satisfies os.IsNotExist… which was mostly the point of my post. And if you want to create a ConfigNotExistErr, then you’ve just added complexity to your API…
In my case, this came up because I was making an error with fmt.Errorf in lumberjack: lumberjack/lumberjack.go at v2.0 · natefinch/lumberjack · GitHub and someone was pointing the package at a directory that didn’t have the correct permissions. This probably is not something you’d normally handle programmatically, it’s really just an incorrect configuration. But I could see a similar problem coming up that could be handled programmatically, if only that error hadn’t been wrapped. Or maybe not - I haven’t had anyone else complain about the wrapped errors.
I reckon if there was one more common interface, say
type exists interface {
NotFound() bool
}
Together with Timeout and Temporary that would cover in excess of 90% of use cases, enough to further discourage the use of error checking via type assertion.
Hi Dave and Nate, is it being suggested that every custom error type declare Timeout, Temporary and now possibly Found and Authorized?
When I think about the custom error types in the json package, I don’t see how these methods apply to the context of the errors you could get back. Maybe you could use !Found() on an unmsrshal error but what about when a pointer is not passed into the function?
This is where I am getting stuck with this idea.
However, now I understand why the default error type is an unexported type with the unexported string field. This is brilliant.
The idea of inspecting errors by behavior sounds great but the specific example of the Temporary seems suspicious to me. How does the code returning the error know what the application considers temporary?
Not precisely. Lack of the method implied false. Or at least unknown.
Nope. You only need to implement them if they could apply to your error. If they don’t make sense, don’t.
The code to check does a type assertion first, and not having the method means generally the same as false except for not found.
I’d like to see these added to the standard library in the appropriate errors it returns and possibly add checking functions somewhere (errors package?). I think that would do a lot to speed adaption, since it’s tough to propose a standard if it’s not used by the std lib… But the nice thing is that it is a backward compatible change afaict.
