go: text/template: provide full stacktrace when the function call panics

At the moment, the code is:

// safeCall runs fun.Call(args), and returns the resulting value and error, if
// any. If the call panics, the panic value is returned as an error.
func safeCall(fun reflect.Value, args []reflect.Value) (val reflect.Value, err error) {
	defer func() {
		if r := recover(); r != nil {
			if e, ok := r.(error); ok {
				err = e
			} else {
				err = fmt.Errorf("%v", r)
			}
		}
	}()
	ret := fun.Call(args)
	if len(ret) == 2 && !ret[1].IsNil() {
		return ret[0], ret[1].Interface().(error)
	}
	return ret[0], nil
}

If the function in template panics: {{.TheUnstableFunc}} , users and developers only know that it panics, but there is no stracktrace.

In a complex system, the template functions could also be complex, without stacktrace, it’s difficult to locate the root problem.

It’s really helpful to provide a full stacktrace when the template function panics.

We have encounters a related bug today, the TheUnstableFunc has concurrency bug, template package only reports runtime error: slice bounds out of range [2:1], it costs a lot of time for debugging the bug.

Thank you.

About this issue

Original URL
State: open
Created a year ago
Reactions: 1
Comments: 24 (12 by maintainers)

Most upvoted comments

I feel it’s a bad precedent to start including true stack traces in errors. If you need to do something - and it still seems to me that you don’t - an opt-in option that disables error recover feels right.

robpike on Aug 22, 2023

I think that some version of option 3 seems workable. If safeCall recovers a panic, it would return an error that includes a stack trace at the point of the panic. I think that would be reasonably backward compatible.

ianlancetaylor on Aug 22, 2023

I don’t think we know exactly what to do. Do you want to try writing a trial implementation?

@ianlancetaylor what do you think about these possible approaches?

IMO the option 4 (reuse the option function: Option("onpanic=nop")) seems the best approach?

wxiaoguang on Aug 14, 2023

Just to add a data point, I have printstack snippet in my editor that expands into:

	defer func() {
		if e := recover(); e != nil {
			log.Printf("** PANIC: %v", e)
			debug.PrintStack()
			panic(e)
		}
	}()

and it’s there solely for use with template functions, and every time it’s a nuisance and takes a bunch of guesswork to debug, and every time I wish template didn’t ever recover from panics.

I’d welcome an option to disable recover in safeCall for text/template specifically. I never want those panics to be recovered from, not even in production, so would just disable recovery permanently.

Like others noted, other panic recoveries (in net/http, for example) are not a problem, this really is specific to templates.

andreyvit on Jul 14, 2023

I would rather not make this change for a number of reasons, compatibility being one. But to find your bug you can either fork the package to add the traceback code (easy), delete the defer block (even easier), or just use a debugger (maybe easy, depending on the environment), which will in effect breakpoint when the trap happens before the template package catches it.

In other words, it is your responsibility to find the bug, not the library’s. And really, it should be very easy to do.

robpike on Apr 6, 2023