Go: proposal: Go 2: fmt: don't recover panics except for dereferencing nil fmt.Stringer receivers

[bcmills]

When a type passed to a fmt function implements fmt.Stringer, the fmt package recovers any panics that the String method produces.

That's helpful for the common case of nil pointers (https://play.golang.org/p/YbmlVGcfPvO), but in other cases masks real, important bugs — and can lead to unexpected deadlocks (https://play.golang.org/p/GjOv8M75GqG; see also #25245).

I propose that, for Go 2, the fmt package should only recover a panic from a fmt.Stringer if:

• the value underlying the fmt.Stringer is a nil value of a pointer type, and
• the panic occurred as a result of dereferencing the receiver (not due to an explicit panic call or a dereference of some other value).

Enforcing the latter condition may require deeper runtime support.

[robpike]

The fmt package tries hard to do _something_ always. The idea is that if you're printing something, and there's a problem, the problem is likely due to code that hasn't run before and/or is hard to get to run at all, so it's better to print something than crash or return error.

I'd like to keep it that way. It's worth not crashing a program because a log statement has a bad argument, for example. I admit this can cause problems sometimes, but all decisions do, and your proposal adds complexity and removes utility in the aid of relatively rare cases. The tradeoff doesn't feel right.

[bcmills]

The idea is that if you're printing something, and there's a problem, the problem is likely due to code that hasn't run before and/or is hard to get to run at all, so it's better to print something than crash or return error.

If the problem is “due to code that hasn't run before and/or is hard to get to run at all,” that seems to make it even more important to preserve the stack trace that led to it — which is exactly the information that recover destroys.

[bcmills]

The recover in fmt is especially insidious because — unlike the recover added for #28242 — it does not result in a user-visible error.

(Users often omit error checks for fmt calls, and even if they did check, the fmt functions don't return an error on panic anyway: https://play.golang.org/p/x92v-MWuzPF)

[robpike]

If the problem is “due to code that hasn't run before and/or is hard to get to run at all,” that seems to make it even more important to preserve the stack trace that led to it — which is exactly the information that recover destroys.

I know what you're trying to say, but I disagree. I'd rather see bogus output from a rare log statement than crash a production job.

I do not want to change this behavior in the fmt package.

[mvdan]

Also worth noting that text/template now recovers panics encountered while calling template functions: https://github.com/golang/go/issues/28242

One of the big points in favor was that fmt did something very similar. So if fmt changes in Go2, perhaps other pieces of the standard library like text/template should be reconsidered too.

There's one big difference, however. If fmt recovers a panic, it can be easy for it to go unnoticed, whereas Template.Execute will simply return the recovered panic as an error. That should be much more obvious.

[mvdan]

I just realised that my last comment was completely redundant given @bcmills' earlier comment; apologies for not reading the thread properly.

[nhooyr]

Why don't we make fmt show the stack trace as well instead of just the panic value? That would be a reasonable middle ground.

[nhooyr]

E.g. this code (https://play.golang.org/p/g0_AlCKKOa3) right now produces

%!s(PANIC=runtime error: invalid memory address or nil pointer dereference)

Along with the error message, it'd include a full stack trace to make it easier to debug.