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

[wxiaoguang]

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.

[seankhliao]

cc @robpike

[ianlancetaylor]

I don't see any reason for this to be a proposal, so taking it out of the proposal process.

Can you show us a complete, standalone example that demonstrates the problem? I don't understand it from the description. Thanks.

[wxiaoguang]

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.

The background is: we have a very complex function to be called in template: {{.TheComplexFunc}} , in TheComplexFunc, there are a lot of function calls including TheBuggyFunc, which has a concurrency problem, like this (just for example, to show it is complex):

func (t *Type) TheComplexFunc() {
    func1()...
    func2()...
    while ... {
         if (func3()) {
            TheBuggyFunc()
         } else {
             func4()
         }
    }
    func5()
}

Then we call the TheComplexFunc in template:

html code ...
{{$t.TheComplexFunc ....}}
html code ...

Since the TheBuggyFunc has concurrency bug, on high concurrent requests, it panics with slice bounds out of range [2:1].

But in error log, we only see: executing "my_template" at <$t.TheComplexFunc>: error calling TheComplexFunc: runtime error: slice bounds out of range [2:1]

TheComplexFunc is quite complex, it's unclear where the panic happens. I know that there could be some methods to help to locate the problem, but if we can see the full stacktrace in error:

executing "my_template" at <$t.TheComplexFunc>: error calling TheComplexFunc: runtime error: slice bounds out of range [2:1]

/framework.go:123 cases.String()
/my-code.go:123 TheComplexFunc()
/my-code.go:456 TheComplexFunc()
/template.go:... execute()

We can find the problem in first time.

Related issue: go-gitea/gitea#23872

[ianlancetaylor]

It's impossible to be sure without an example, but I think you are referring to https://go.googlesource.com/go/+/refs/heads/master/src/text/template/funcs.go#355 . When a template calls a function, and the function panics, the template turns that into an error. That means that the default stack trace will not appear.

I don't think that we can change that behavior today. That would be too likely to break existing working code.

[wxiaoguang]

I don't think that we can change that behavior today. That would be too likely to break existing working code.

To avoid breaking, how about allowing developers to use a self-defined panic handler?

func safeCall(fun reflect.Value, args []reflect.Value) (val reflect.Value, err error) {
	defer func() {
		if r := recover(); r != nil {
			rErr := t.panicHandler(r) // <------ HERE
			if e, ok := rErr.(error); ok {
				err = e
			} else {
				err = fmt.Errorf("%v", rErr)
			}
		}
	}()
	...
}


t.SetPanicHandler(func (v any) error {
	// we have a chance to handle the panic
})

t.Execute(...)

[wxiaoguang]

Hi @ianlancetaylor , what do you think about this proposal?

[robpike]

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.

[wxiaoguang]

compatibility being one

Sorry but I didn't see any compatibility problem here, we just need to add a new function, nothing breaks.

If there is any compatibility problem, please let me know.

But to find your bug you can either ...........

If a developer could catch the panic stacktrace in first time, why they should do so much unnecessary work? Especially for a production rare crash. A clear stacktrace saves everyone's time. It's really helpful.

it is your responsibility to find the bug

It is developer's responsibility to find bugs, but it's framework / language's responsibility to help developers to find bug, instead of hiding important clues / just returning an unclear error message.

I would say the template package do wrong here, because it doesn't follow Golang's philosophy: use panic stracktrace to locate the incorrect code.

And really, it should be very easy to do.

In many cases, not that easy. If framework could help, it is easier.

---

Could you please re-consider about this proposal? Thank you very much.