From a3b917151cedfec5adecdaed777547721a61b930 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Mon, 20 Mar 2023 17:53:45 -0400 Subject: [PATCH] Improve template error reporting (#23396) (#23600) 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 Co-authored-by: zeripath --- modules/context/context.go | 32 ++++++++++++++++++++ modules/templates/htmlrenderer.go | 49 ++++++++++++++++++------------- 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/modules/context/context.go b/modules/context/context.go index f2c6b129fe..955c3d9a70 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -17,8 +17,10 @@ import ( "net/http" "net/url" "path" + "regexp" "strconv" "strings" + texttemplate "text/template" "time" "code.gitea.io/gitea/models/db" @@ -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) @@ -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) } } diff --git a/modules/templates/htmlrenderer.go b/modules/templates/htmlrenderer.go index 5a328043eb..96dc010796 100644 --- a/modules/templates/htmlrenderer.go +++ b/modules/templates/htmlrenderer.go @@ -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)} } @@ -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)} } @@ -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)} } @@ -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) @@ -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 // 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] = ' ' - } - } - - // write the preceding "space" - _, _ = sb.Write(lineBs[:idx]) - - // Now write the ^^ pointer - _, _ = sb.WriteString(strings.Repeat("^", len(target))) - _ = sb.WriteByte('\n') + 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[: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 sb.WriteString(dashSeparator)