Skip to content

Commit

Permalink
Improve template error reporting (#23396) (#23600)
Browse files Browse the repository at this point in the history 
Backport #23396 by @zeripath

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>
Co-authored-by: zeripath <art27@cantab.net>
  • Loading branch information
@GiteaBot @zeripath
GiteaBot and zeripath authored Mar 20, 2023
1 parent 0732ba3 commit a3b9171
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 18 deletions.
32 changes: 32 additions & 0 deletions modules/context/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ import (
"net/http"
"net/url"
"path"
"regexp"
"strconv"
"strings"
texttemplate "text/template"
"time"

"code.gitea.io/gitea/models/db"
Expand Down Expand Up @@ -213,6 +215,8 @@ func (ctx *Context) RedirectToFirst(location ...string) {
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
func (ctx *Context) HTML(status int, name base.TplName) {
log.Debug("Template: %s", name)
Expand All @@ -228,6 +232,34 @@ func (ctx *Context) HTML(status int, name base.TplName) {
ctx.PlainText(http.StatusInternalServerError, "Unable to find status/500 template")
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)
}
}
Expand Down
43 changes: 25 additions & 18 deletions modules/templates/htmlrenderer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func handleGenericTemplateError(err error) (string, []interface{}) {

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)}
}
Expand All @@ -140,7 +140,7 @@ func handleNotDefinedPanicError(err error) (string, []interface{}) {

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)}
}
Expand All @@ -161,7 +161,7 @@ func handleUnexpected(err error) (string, []interface{}) {

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)}
}
Expand All @@ -181,14 +181,15 @@ func handleExpectedEnd(err error) (string, []interface{}) {

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)}
}

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")
if err != nil {
return fmt.Sprintf("(unable to read template file: %v)", err)
Expand Down Expand Up @@ -229,23 +230,29 @@ 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
// e.g. ^^^^^^^
if target != "" {
idx := bytes.Index(lineBs, []byte(target))

if idx >= 0 {
// take the current line and replace preceding text with whitespace (except for tab)
for i := range lineBs[:idx] {
if lineBs[i] != '\t' {
lineBs[i] = ' '
}
targetPos := bytes.Index(lineBs, []byte(target))
if targetPos >= 0 {
position = targetPos
}
}
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[:idx])
// write the preceding "space"
_, _ = sb.Write(lineBs[:position])

// Now write the ^^ pointer
_, _ = sb.WriteString(strings.Repeat("^", len(target)))
_ = sb.WriteByte('\n')
// Now write the ^^ pointer
targetLen := len(target)
if targetLen == 0 {
targetLen = 1
}
_, _ = sb.WriteString(strings.Repeat("^", targetLen))
_ = sb.WriteByte('\n')
}

// Finally write the footer
Expand Down

0 comments on commit a3b9171

Please sign in to comment.