Skip to content

Add nicer error handling on template compile errors #21350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions modules/templates/dynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@ func GetAsset(name string) ([]byte, error) {
return os.ReadFile(filepath.Join(setting.StaticRootPath, name))
}

// GetAssetFilename returns the filename of the provided asset
func GetAssetFilename(name string) (string, error) {
filename := filepath.Join(setting.CustomPath, name)
_, err := os.Stat(filename)
if err != nil && !os.IsNotExist(err) {
return filename, err
} else if err == nil {
return filename, nil
}

filename = filepath.Join(setting.StaticRootPath, name)
_, err = os.Stat(filename)
return filename, err
}

// walkTemplateFiles calls a callback for each template asset
func walkTemplateFiles(callback func(path, name string, d fs.DirEntry, err error) error) error {
if err := walkAssetDir(filepath.Join(setting.CustomPath, "templates"), true, callback); err != nil && !os.IsNotExist(err) {
Expand Down
172 changes: 171 additions & 1 deletion modules/templates/htmlrenderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
package templates

import (
"bytes"
"context"
"fmt"
"regexp"
"strconv"
"strings"

"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
Expand All @@ -14,7 +19,14 @@ import (
"github.com/unrolled/render"
)

var rendererKey interface{} = "templatesHtmlRendereer"
var (
rendererKey interface{} = "templatesHtmlRenderer"

templateError = regexp.MustCompile(`^template: (.*):([0-9]+): (.*)`)
notDefinedError = regexp.MustCompile(`^template: (.*):([0-9]+): function "(.*)" not defined`)
unexpectedError = regexp.MustCompile(`^template: (.*):([0-9]+): unexpected "(.*)" in operand`)
expectedEndError = regexp.MustCompile(`^template: (.*):([0-9]+): expected end; found (.*)`)
)

// HTMLRenderer returns the current html renderer for the context or creates and stores one within the context for future use
func HTMLRenderer(ctx context.Context) (context.Context, *render.Render) {
Expand All @@ -32,6 +44,25 @@ func HTMLRenderer(ctx context.Context) (context.Context, *render.Render) {
}
log.Log(1, log.DEBUG, "Creating "+rendererType+" HTML Renderer")

compilingTemplates := true
defer func() {
if !compilingTemplates {
return
}

panicked := recover()
if panicked == nil {
return
}

// OK try to handle the panic...
err, ok := panicked.(error)
if ok {
handlePanicError(err)
}
log.Fatal("PANIC: Unable to compile templates: %v\nStacktrace:\n%s", panicked, log.Stack(2))
}()

renderer := render.New(render.Options{
Extensions: []string{".tmpl"},
Directory: "templates",
Expand All @@ -42,6 +73,7 @@ func HTMLRenderer(ctx context.Context) (context.Context, *render.Render) {
IsDevelopment: false,
DisableHTTPErrorRendering: true,
})
compilingTemplates = false
if !setting.IsProd {
watcher.CreateWatcher(ctx, "HTML Templates", &watcher.CreateWatcherOpts{
PathsCallback: walkTemplateFiles,
Expand All @@ -50,3 +82,141 @@ func HTMLRenderer(ctx context.Context) (context.Context, *render.Render) {
}
return context.WithValue(ctx, rendererKey, renderer), renderer
}

func handlePanicError(err error) {
wrapFatal(handleNotDefinedPanicError(err))
wrapFatal(handleUnexpected(err))
wrapFatal(handleExpectedEnd(err))
wrapFatal(handleGenericTemplateError(err))
}

func wrapFatal(format string, args ...interface{}) {
if format == "" {
return
}
log.Fatal(format, args...)
}

func handleGenericTemplateError(err error) (string, []interface{}) {
groups := templateError.FindStringSubmatch(err.Error())
if len(groups) != 4 {
return "", nil
}

templateName, lineNumberStr, message := groups[1], groups[2], groups[3]

filename, assetErr := GetAssetFilename("templates/" + templateName + ".tmpl")
if assetErr != nil {
return "", nil
}

lineNumber, _ := strconv.Atoi(lineNumberStr)

line := getLineFromAsset(templateName, lineNumber, "")

return "PANIC: Unable to compile templates due to: %s in template file %s at line %d:\n%s\nStacktrace:\n%s", []interface{}{message, filename, lineNumber, log.NewColoredValue(line, log.Reset), log.Stack(2)}
}

func handleNotDefinedPanicError(err error) (string, []interface{}) {
groups := notDefinedError.FindStringSubmatch(err.Error())
if len(groups) != 4 {
return "", nil
}

templateName, lineNumberStr, functionName := groups[1], groups[2], groups[3]

functionName, _ = strconv.Unquote(`"` + functionName + `"`)

filename, assetErr := GetAssetFilename("templates/" + templateName + ".tmpl")
if assetErr != nil {
return "", nil
}

lineNumber, _ := strconv.Atoi(lineNumberStr)

line := getLineFromAsset(templateName, lineNumber, functionName)

return "PANIC: Unable to compile templates due to undefined function %q in template file %s at line %d:\n%s\nStacktrace:\n%s", []interface{}{functionName, filename, lineNumber, log.NewColoredValue(line, log.Reset), log.Stack(2)}
}

func handleUnexpected(err error) (string, []interface{}) {
groups := unexpectedError.FindStringSubmatch(err.Error())
if len(groups) != 4 {
return "", nil
}

templateName, lineNumberStr, unexpected := groups[1], groups[2], groups[3]
unexpected, _ = strconv.Unquote(`"` + unexpected + `"`)

filename, assetErr := GetAssetFilename("templates/" + templateName + ".tmpl")
if assetErr != nil {
return "", nil
}

lineNumber, _ := strconv.Atoi(lineNumberStr)

line := getLineFromAsset(templateName, lineNumber, unexpected)

return "PANIC: Unable to compile templates due to unexpected %q in template file %s at line %d:\n%s\nStacktrace:\n%s", []interface{}{unexpected, filename, lineNumber, log.NewColoredValue(line, log.Reset), log.Stack(2)}
}

func handleExpectedEnd(err error) (string, []interface{}) {
groups := expectedEndError.FindStringSubmatch(err.Error())
if len(groups) != 4 {
return "", nil
}

templateName, lineNumberStr, unexpected := groups[1], groups[2], groups[3]

filename, assetErr := GetAssetFilename("templates/" + templateName + ".tmpl")
if assetErr != nil {
return "", nil
}

lineNumber, _ := strconv.Atoi(lineNumberStr)

line := getLineFromAsset(templateName, lineNumber, unexpected)

return "PANIC: Unable to compile templates due to missing end with unexpected %q in template file %s at line %d:\n%s\nStacktrace:\n%s", []interface{}{unexpected, filename, lineNumber, log.NewColoredValue(line, log.Reset), log.Stack(2)}
}

func getLineFromAsset(templateName string, lineNumber int, functionName string) string {
bs, err := GetAsset("templates/" + templateName + ".tmpl")
if err != nil {
return fmt.Sprintf("(unable to read template file: %v)", err)
}

sb := &strings.Builder{}
start := 0
var lineBs []byte
for i := 0; i < lineNumber && start < len(bs); i++ {
end := bytes.IndexByte(bs[start:], '\n')
if end < 0 {
end = len(bs)
} else {
end += start
}
lineBs = bs[start:end]
if lineNumber-i < 4 {
_, _ = sb.Write(lineBs)
_ = sb.WriteByte('\n')
}
start = end + 1
}

if functionName != "" {
idx := strings.Index(string(lineBs), functionName)
for i := range lineBs[:idx] {
if lineBs[i] != '\t' {
lineBs[i] = ' '
}
}
_, _ = sb.Write(lineBs[:idx])
if idx >= 0 {
_, _ = sb.WriteString(strings.Repeat("^", len(functionName)))
}
_ = sb.WriteByte('\n')
}

return strings.Repeat("-", 70) + "\n" + sb.String() + strings.Repeat("-", 70)
}
11 changes: 11 additions & 0 deletions modules/templates/static.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ func GlobalModTime(filename string) time.Time {
return timeutil.GetExecutableModTime()
}

// GetAssetFilename returns the filename of the provided asset
func GetAssetFilename(name string) (string, error) {
_, err := os.Stat(filepath.Join(setting.CustomPath, name))
if err != nil && !os.IsNotExist(err) {
return name, err
} else if err == nil {
return filepath.Join(setting.CustomPath, name), nil
}
return "(builtin) " + name, nil
}

// GetAsset get a special asset, only for chi
func GetAsset(name string) ([]byte, error) {
bs, err := os.ReadFile(filepath.Join(setting.CustomPath, name))
Expand Down