Improve template error reporting (#23396)

There are multiple duplicate reports of errors during template rendering
due to broken custom templates.

Unfortunately the error returned here is somewhat difficult for users to
understand and it doesn't return the context of the error.

This PR attempts to parse the error returned by the template renderer to
add in some further context including the filename of the template AND
the preceding lines within that template file.

Ref #23274

---------

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2023-03-20 20:56:48 +00:00 committed by GitHub
parent 529bac1950
commit 8f3aaef374
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 21 deletions

View File

@ -16,8 +16,10 @@ import (
"net/http" "net/http"
"net/url" "net/url"
"path" "path"
"regexp"
"strconv" "strconv"
"strings" "strings"
texttemplate "text/template"
"time" "time"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
@ -213,6 +215,8 @@ func (ctx *Context) RedirectToFirst(location ...string) {
ctx.Redirect(setting.AppSubURL + "/") ctx.Redirect(setting.AppSubURL + "/")
} }
var templateExecutingErr = regexp.MustCompile(`^template: (.*):([1-9][0-9]*):([1-9][0-9]*): executing (?:"(.*)" at <(.*)>: )?`)
// HTML calls Context.HTML and renders the template to HTTP response // HTML calls Context.HTML and renders the template to HTTP response
func (ctx *Context) HTML(status int, name base.TplName) { func (ctx *Context) HTML(status int, name base.TplName) {
log.Debug("Template: %s", name) log.Debug("Template: %s", name)
@ -228,6 +232,34 @@ func (ctx *Context) HTML(status int, name base.TplName) {
ctx.PlainText(http.StatusInternalServerError, "Unable to find status/500 template") ctx.PlainText(http.StatusInternalServerError, "Unable to find status/500 template")
return return
} }
if execErr, ok := err.(texttemplate.ExecError); ok {
if groups := templateExecutingErr.FindStringSubmatch(err.Error()); len(groups) > 0 {
errorTemplateName, lineStr, posStr := groups[1], groups[2], groups[3]
target := ""
if len(groups) == 6 {
target = groups[5]
}
line, _ := strconv.Atoi(lineStr) // Cannot error out as groups[2] is [1-9][0-9]*
pos, _ := strconv.Atoi(posStr) // Cannot error out as groups[3] is [1-9][0-9]*
filename, filenameErr := templates.GetAssetFilename("templates/" + errorTemplateName + ".tmpl")
if filenameErr != nil {
filename = "(template) " + errorTemplateName
}
if errorTemplateName != string(name) {
filename += " (subtemplate of " + string(name) + ")"
}
err = fmt.Errorf("%w\nin template file %s:\n%s", err, filename, templates.GetLineFromTemplate(errorTemplateName, line, target, pos))
} else {
filename, filenameErr := templates.GetAssetFilename("templates/" + execErr.Name + ".tmpl")
if filenameErr != nil {
filename = "(template) " + execErr.Name
}
if execErr.Name != string(name) {
filename += " (subtemplate of " + string(name) + ")"
}
err = fmt.Errorf("%w\nin template file %s", err, filename)
}
}
ctx.ServerError("Render failed", err) ctx.ServerError("Render failed", err)
} }
} }

View File

@ -118,7 +118,7 @@ func handleGenericTemplateError(err error) (string, []interface{}) {
lineNumber, _ := strconv.Atoi(lineNumberStr) lineNumber, _ := strconv.Atoi(lineNumberStr)
line := getLineFromAsset(templateName, lineNumber, "") line := GetLineFromTemplate(templateName, lineNumber, "", -1)
return "PANIC: Unable to compile templates!\n%s in template file %s at line %d:\n\n%s\nStacktrace:\n\n%s", []interface{}{message, filename, lineNumber, log.NewColoredValue(line, log.Reset), log.Stack(2)} return "PANIC: Unable to compile templates!\n%s in template file %s at line %d:\n\n%s\nStacktrace:\n\n%s", []interface{}{message, filename, lineNumber, log.NewColoredValue(line, log.Reset), log.Stack(2)}
} }
@ -140,7 +140,7 @@ func handleNotDefinedPanicError(err error) (string, []interface{}) {
lineNumber, _ := strconv.Atoi(lineNumberStr) lineNumber, _ := strconv.Atoi(lineNumberStr)
line := getLineFromAsset(templateName, lineNumber, functionName) line := GetLineFromTemplate(templateName, lineNumber, functionName, -1)
return "PANIC: Unable to compile templates!\nUndefined function %q in template file %s at line %d:\n\n%s", []interface{}{functionName, filename, lineNumber, log.NewColoredValue(line, log.Reset)} return "PANIC: Unable to compile templates!\nUndefined function %q in template file %s at line %d:\n\n%s", []interface{}{functionName, filename, lineNumber, log.NewColoredValue(line, log.Reset)}
} }
@ -161,7 +161,7 @@ func handleUnexpected(err error) (string, []interface{}) {
lineNumber, _ := strconv.Atoi(lineNumberStr) lineNumber, _ := strconv.Atoi(lineNumberStr)
line := getLineFromAsset(templateName, lineNumber, unexpected) line := GetLineFromTemplate(templateName, lineNumber, unexpected, -1)
return "PANIC: Unable to compile templates!\nUnexpected %q in template file %s at line %d:\n\n%s", []interface{}{unexpected, filename, lineNumber, log.NewColoredValue(line, log.Reset)} return "PANIC: Unable to compile templates!\nUnexpected %q in template file %s at line %d:\n\n%s", []interface{}{unexpected, filename, lineNumber, log.NewColoredValue(line, log.Reset)}
} }
@ -181,14 +181,15 @@ func handleExpectedEnd(err error) (string, []interface{}) {
lineNumber, _ := strconv.Atoi(lineNumberStr) lineNumber, _ := strconv.Atoi(lineNumberStr)
line := getLineFromAsset(templateName, lineNumber, unexpected) line := GetLineFromTemplate(templateName, lineNumber, unexpected, -1)
return "PANIC: Unable to compile templates!\nMissing end with unexpected %q in template file %s at line %d:\n\n%s", []interface{}{unexpected, filename, lineNumber, log.NewColoredValue(line, log.Reset)} return "PANIC: Unable to compile templates!\nMissing end with unexpected %q in template file %s at line %d:\n\n%s", []interface{}{unexpected, filename, lineNumber, log.NewColoredValue(line, log.Reset)}
} }
const dashSeparator = "----------------------------------------------------------------------\n" const dashSeparator = "----------------------------------------------------------------------\n"
func getLineFromAsset(templateName string, targetLineNum int, target string) string { // GetLineFromTemplate returns a line from a template with some context
func GetLineFromTemplate(templateName string, targetLineNum int, target string, position int) string {
bs, err := GetAsset("templates/" + templateName + ".tmpl") bs, err := GetAsset("templates/" + templateName + ".tmpl")
if err != nil { if err != nil {
return fmt.Sprintf("(unable to read template file: %v)", err) return fmt.Sprintf("(unable to read template file: %v)", err)
@ -229,24 +230,30 @@ func getLineFromAsset(templateName string, targetLineNum int, target string) str
// If there is a provided target to look for in the line add a pointer to it // If there is a provided target to look for in the line add a pointer to it
// e.g. ^^^^^^^ // e.g. ^^^^^^^
if target != "" { if target != "" {
idx := bytes.Index(lineBs, []byte(target)) targetPos := bytes.Index(lineBs, []byte(target))
if targetPos >= 0 {
if idx >= 0 { position = targetPos
// take the current line and replace preceding text with whitespace (except for tab)
for i := range lineBs[:idx] {
if lineBs[i] != '\t' {
lineBs[i] = ' '
}
}
// write the preceding "space"
_, _ = sb.Write(lineBs[:idx])
// Now write the ^^ pointer
_, _ = sb.WriteString(strings.Repeat("^", len(target)))
_ = sb.WriteByte('\n')
} }
} }
if position >= 0 {
// take the current line and replace preceding text with whitespace (except for tab)
for i := range lineBs[:position] {
if lineBs[i] != '\t' {
lineBs[i] = ' '
}
}
// write the preceding "space"
_, _ = sb.Write(lineBs[:position])
// Now write the ^^ pointer
targetLen := len(target)
if targetLen == 0 {
targetLen = 1
}
_, _ = sb.WriteString(strings.Repeat("^", targetLen))
_ = sb.WriteByte('\n')
}
// Finally write the footer // Finally write the footer
sb.WriteString(dashSeparator) sb.WriteString(dashSeparator)