tl;dr: Don't. Use a value receiver. 99% of the time you can just delete
the * on the receiver and it'll still work. If you really must use a
pointer, please handle the case where you're called with a nil receiver.
I spent most of today trying to understand why my new hook command was
producing this output:
error: %!v(PANIC=runtime error: invalid memory address or nil pointer
dereference)
It took me a while to figure out that this is what fmt.Printf("error:
%v\n", err) outputs when err's Error() method panics. If you're using %s
or %v to print a value (or use Println which implicitly uses %v), then fmt
will look for a String() method or Error() method on the value to call, and
use the output of that for the value's string output. If that method
panics, fmt prints the panic in the way you see above (everything after the
PANIC=).
Of course, the problem here is that there's no type being written, and
since it was an error interface, it could almost anything. Using %#v skips
calling the Error/String methods and prints out the values in a go format,
which told me this was a juju/errors.Err value, wrapping an API params
Error value which was a nil pointer. When we call Error() on an
errors.Err, we call Error() on the cause explicitly, which was panicking.
Here's a minimal reproduction http://play.golang.org/p/ncNVrza-hn (you'll
have to copy it to a local file and go run it, since the playground won't
run code external to the stdlib).
So what's sort of interesting is that printing the error before it gets
Traced works fine, but after the trace it is not fine. The errors.Err's
Error() function looks like it's explicitly calling the Error() method on
the wrapped Cause error, which is probably the problem. fmt.Printf must
use some reflection magic to avoid doing that.
Now, the root cause of this particular bug is actually my own mistake.
Line 21 should check if orig is nil and then assign nil explicitly to err
if it is. Then errors.Trace would be able to tell that the error is nil
and would just return nil itself, instead of thinking it's a valid error
and wrapping it.
However, you can sidestep this entirely by doing one of two things: either
just make the Error() (or String()) method use a value receiver.. in which
case this code would produce this output:
%!v(PANIC=value method main.MyError.Error called using nil *MyError pointer)
(you can try it with the repro code I linked to)
This printout is a lot more helpful and useful and obvious than the other
"nil pointer" printout.
OR
Just handle a nil receiver:
func (e *MyError) Error() string {
if e == nil {
return "<nil MyError>"
}
return e.Message
}
(note that it is dereferencing the pointer to e to access the Message field
which causes the panic. Calling a method on a nil pointer is totally fine
and will not panic if the code inside does not try to derefence the pointer
to get to a field).
Grepping through our code, I see a lot of pointer receivers on Error and
String methods (45 and 77 respectively). I think we should at least change
all of these to be value methods (unless that's not possible. That's a
trivial change, and gives a much more useful printout when the pointer is
nil.
-Nate
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
