Skip to content

Commit 94a4264

Browse files
zeripathGiteaBot
authored andcommitted
Improve template error reporting (go-gitea#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 go-gitea#23274 --------- Signed-off-by: Andrew Thornton <[email protected]>
1 parent 0732ba3 commit 94a4264

File tree

2 files changed

+57
-18
lines changed

2 files changed

+57
-18
lines changed

modules/context/context.go

+32
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ import (
1717
"net/http"
1818
"net/url"
1919
"path"
20+
"regexp"
2021
"strconv"
2122
"strings"
23+
texttemplate "text/template"
2224
"time"
2325

2426
"code.gitea.io/gitea/models/db"
@@ -213,6 +215,8 @@ func (ctx *Context) RedirectToFirst(location ...string) {
213215
ctx.Redirect(setting.AppSubURL + "/")
214216
}
215217

218+
var templateExecutingErr = regexp.MustCompile(`^template: (.*):([1-9][0-9]*):([1-9][0-9]*): executing (?:"(.*)" at <(.*)>: )?`)
219+
216220
// HTML calls Context.HTML and renders the template to HTTP response
217221
func (ctx *Context) HTML(status int, name base.TplName) {
218222
log.Debug("Template: %s", name)
@@ -228,6 +232,34 @@ func (ctx *Context) HTML(status int, name base.TplName) {
228232
ctx.PlainText(http.StatusInternalServerError, "Unable to find status/500 template")
229233
return
230234
}
235+
if execErr, ok := err.(texttemplate.ExecError); ok {
236+
if groups := templateExecutingErr.FindStringSubmatch(err.Error()); len(groups) > 0 {
237+
errorTemplateName, lineStr, posStr := groups[1], groups[2], groups[3]
238+
target := ""
239+
if len(groups) == 6 {
240+
target = groups[5]
241+
}
242+
line, _ := strconv.Atoi(lineStr) // Cannot error out as groups[2] is [1-9][0-9]*
243+
pos, _ := strconv.Atoi(posStr) // Cannot error out as groups[3] is [1-9][0-9]*
244+
filename, filenameErr := templates.GetAssetFilename("templates/" + errorTemplateName + ".tmpl")
245+
if filenameErr != nil {
246+
filename = "(template) " + errorTemplateName
247+
}
248+
if errorTemplateName != string(name) {
249+
filename += " (subtemplate of " + string(name) + ")"
250+
}
251+
err = fmt.Errorf("%w\nin template file %s:\n%s", err, filename, templates.GetLineFromTemplate(errorTemplateName, line, target, pos))
252+
} else {
253+
filename, filenameErr := templates.GetAssetFilename("templates/" + execErr.Name + ".tmpl")
254+
if filenameErr != nil {
255+
filename = "(template) " + execErr.Name
256+
}
257+
if execErr.Name != string(name) {
258+
filename += " (subtemplate of " + string(name) + ")"
259+
}
260+
err = fmt.Errorf("%w\nin template file %s", err, filename)
261+
}
262+
}
231263
ctx.ServerError("Render failed", err)
232264
}
233265
}

modules/templates/htmlrenderer.go

+25-18
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func handleGenericTemplateError(err error) (string, []interface{}) {
118118

119119
lineNumber, _ := strconv.Atoi(lineNumberStr)
120120

121-
line := getLineFromAsset(templateName, lineNumber, "")
121+
line := GetLineFromTemplate(templateName, lineNumber, "", -1)
122122

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

141141
lineNumber, _ := strconv.Atoi(lineNumberStr)
142142

143-
line := getLineFromAsset(templateName, lineNumber, functionName)
143+
line := GetLineFromTemplate(templateName, lineNumber, functionName, -1)
144144

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

162162
lineNumber, _ := strconv.Atoi(lineNumberStr)
163163

164-
line := getLineFromAsset(templateName, lineNumber, unexpected)
164+
line := GetLineFromTemplate(templateName, lineNumber, unexpected, -1)
165165

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

182182
lineNumber, _ := strconv.Atoi(lineNumberStr)
183183

184-
line := getLineFromAsset(templateName, lineNumber, unexpected)
184+
line := GetLineFromTemplate(templateName, lineNumber, unexpected, -1)
185185

186186
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)}
187187
}
188188

189189
const dashSeparator = "----------------------------------------------------------------------\n"
190190

191-
func getLineFromAsset(templateName string, targetLineNum int, target string) string {
191+
// GetLineFromTemplate returns a line from a template with some context
192+
func GetLineFromTemplate(templateName string, targetLineNum int, target string, position int) string {
192193
bs, err := GetAsset("templates/" + templateName + ".tmpl")
193194
if err != nil {
194195
return fmt.Sprintf("(unable to read template file: %v)", err)
@@ -229,23 +230,29 @@ func getLineFromAsset(templateName string, targetLineNum int, target string) str
229230
// If there is a provided target to look for in the line add a pointer to it
230231
// e.g. ^^^^^^^
231232
if target != "" {
232-
idx := bytes.Index(lineBs, []byte(target))
233-
234-
if idx >= 0 {
235-
// take the current line and replace preceding text with whitespace (except for tab)
236-
for i := range lineBs[:idx] {
237-
if lineBs[i] != '\t' {
238-
lineBs[i] = ' '
239-
}
233+
targetPos := bytes.Index(lineBs, []byte(target))
234+
if targetPos >= 0 {
235+
position = targetPos
236+
}
237+
}
238+
if position >= 0 {
239+
// take the current line and replace preceding text with whitespace (except for tab)
240+
for i := range lineBs[:position] {
241+
if lineBs[i] != '\t' {
242+
lineBs[i] = ' '
240243
}
244+
}
241245

242-
// write the preceding "space"
243-
_, _ = sb.Write(lineBs[:idx])
246+
// write the preceding "space"
247+
_, _ = sb.Write(lineBs[:position])
244248

245-
// Now write the ^^ pointer
246-
_, _ = sb.WriteString(strings.Repeat("^", len(target)))
247-
_ = sb.WriteByte('\n')
249+
// Now write the ^^ pointer
250+
targetLen := len(target)
251+
if targetLen == 0 {
252+
targetLen = 1
248253
}
254+
_, _ = sb.WriteString(strings.Repeat("^", targetLen))
255+
_ = sb.WriteByte('\n')
249256
}
250257

251258
// Finally write the footer

0 commit comments

Comments
 (0)