My approach for inspecting Go error values
Dave Cheney sort of recently wrote Don't just check errors, handle
them gracefully,
where he strongly suggested that basically you should never check
the actual values of errors. This is generally a great idea, but
sometimes you don't have a choice. For example, for a long time it
was the case that the only way to tell if your DNS lookup had hit
a temporary DNS error (such as 'no authoritative servers for this
domain are responding') or a permanent one ('this name doesn't
exist') was to examine the specific error that you received. While
net.DNSError had a .Temporary()
function, it didn't return true
in enough cases; you had to go digging deeper to know.
(This was Go issue 8434 and has since been fixed, although it took a while.)
When I had to work around this issue (in code that I suppose I should now remove), I was at least smart enough to try the official way first:
var serverrstr = "server misbehaving"
func isTemporary(err error) bool { if e, ok := err.(*net.DNSError); ok { if e.Temporary() || e.Err == serverrstr { return true } } return false }
Checking the official way first made it so that once this issue was
resolved, my code would immediately start relying on the official
way. Checking the error string only for net.DNSError
errors made
sure that I wouldn't get false positives from other error types,
which seemed like a good idea at the time.
When I wrote this code I felt reasonably smart about it; I thought
I'd done about as well as I could. Then Dave Cheney's article showed
me that I wasn't quite doing this right; as he says in one section
('Assert errors for behaviour, not type'), I should have really
checked for .Temporary()
through an interface instead of just
directly checking the error as a net.DNSError
. After all, maybe
someday net.LookupMX()
and company will return an additional type
of error in some circumstances that has a .Temporary()
method;
if that would happen, my code here wouldn't work right.
(I even put some comments in musing about the idea, but then rejected
it on the grounds that the current net
package code didn't do
that so there didn't seem to be any point. In retrospect that was
the wrong position to take, because I wasn't thinking about potential
future developments in the net
package.)
I'm conflicted over whether to cast to specific error types if you
have to check the actual error value in some way (as I do here). I
think it comes down to which way is safer for the code to fail. If
you check the value through error.Error()
, future changes in the
code you're calling may cause you to match on things that aren't
the specific error type you're expecting. Sometimes this will be
the right answer and sometimes it will be the wrong one, so you
have to weigh the harm of a false positive against the harm of a
false negative.
|
|