From 1c8bc4081a4f4d0d921ac218cb724ce97924d410 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Fri, 14 Apr 2023 13:19:11 +0800
Subject: [PATCH] Show friendly 500 error page to users and developers (#24110)

Close #24104

This also introduces many tests to cover many complex error handling
functions.

### Before

The details are never shown in production.

<details>

![image](https://user-images.githubusercontent.com/2114189/231805004-13214579-4fbe-465a-821c-be75c2749097.png)

</details>

### After

The details could be shown to site admin users. It is safe.

![image](https://user-images.githubusercontent.com/2114189/231803912-d5660994-416f-4b27-a4f1-a4cc962091d4.png)
---
 modules/context/context.go             |  44 +---
 modules/templates/htmlrenderer.go      | 267 +++++++++++++------------
 modules/templates/htmlrenderer_test.go | 106 ++++++++++
 templates/base/head_script.tmpl        |   1 +
 templates/devtest/tmplerr-sub.tmpl     |   3 +
 templates/devtest/tmplerr.tmpl         |  12 ++
 templates/status/404.tmpl              |   2 +-
 templates/status/500.tmpl              |  41 +++-
 web_src/css/helpers.css                |   5 +-
 web_src/js/bootstrap.js                |  12 +-
 10 files changed, 313 insertions(+), 180 deletions(-)
 create mode 100644 modules/templates/htmlrenderer_test.go
 create mode 100644 templates/devtest/tmplerr-sub.tmpl
 create mode 100644 templates/devtest/tmplerr.tmpl

diff --git a/modules/context/context.go b/modules/context/context.go
index cee533e42a..2507cc10c0 100644
--- a/modules/context/context.go
+++ b/modules/context/context.go
@@ -16,10 +16,8 @@ import (
 	"net/http"
 	"net/url"
 	"path"
-	"regexp"
 	"strconv"
 	"strings"
-	texttemplate "text/template"
 	"time"
 
 	"code.gitea.io/gitea/models/db"
@@ -216,7 +214,7 @@ 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 <(.*)>: )?`)
+const tplStatus500 base.TplName = "status/500"
 
 // HTML calls Context.HTML and renders the template to HTTP response
 func (ctx *Context) HTML(status int, name base.TplName) {
@@ -229,34 +227,11 @@ func (ctx *Context) HTML(status int, name base.TplName) {
 		return strconv.FormatInt(time.Since(tmplStartTime).Nanoseconds()/1e6, 10) + "ms"
 	}
 	if err := ctx.Render.HTML(ctx.Resp, status, string(name), templates.BaseVars().Merge(ctx.Data)); err != nil {
-		if status == http.StatusInternalServerError && name == base.TplName("status/500") {
+		if status == http.StatusInternalServerError && name == tplStatus500 {
 			ctx.PlainText(http.StatusInternalServerError, "Unable to find HTML templates, the template system is not initialized, or Gitea can't find your template files.")
 			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]*
-				assetLayerName := templates.AssetFS().GetFileLayerName(errorTemplateName + ".tmpl")
-				filename := fmt.Sprintf("(%s) %s", assetLayerName, errorTemplateName)
-				if errorTemplateName != string(name) {
-					filename += " (subtemplate of " + string(name) + ")"
-				}
-				err = fmt.Errorf("failed to render %s, error: %w:\n%s", filename, err, templates.GetLineFromTemplate(errorTemplateName, line, target, pos))
-			} else {
-				assetLayerName := templates.AssetFS().GetFileLayerName(execErr.Name + ".tmpl")
-				filename := fmt.Sprintf("(%s) %s", assetLayerName, execErr.Name)
-				if execErr.Name != string(name) {
-					filename += " (subtemplate of " + string(name) + ")"
-				}
-				err = fmt.Errorf("failed to render %s, error: %w", filename, err)
-			}
-		}
+		err = fmt.Errorf("failed to render template: %s, error: %s", name, templates.HandleTemplateRenderingError(err))
 		ctx.ServerError("Render failed", err)
 	}
 }
@@ -324,24 +299,25 @@ func (ctx *Context) serverErrorInternal(logMsg string, logErr error) {
 			return
 		}
 
-		if !setting.IsProd {
+		// it's safe to show internal error to admin users, and it helps
+		if !setting.IsProd || (ctx.Doer != nil && ctx.Doer.IsAdmin) {
 			ctx.Data["ErrorMsg"] = logErr
 		}
 	}
 
 	ctx.Data["Title"] = "Internal Server Error"
-	ctx.HTML(http.StatusInternalServerError, base.TplName("status/500"))
+	ctx.HTML(http.StatusInternalServerError, tplStatus500)
 }
 
 // NotFoundOrServerError use error check function to determine if the error
 // is about not found. It responds with 404 status code for not found error,
 // or error context description for logging purpose of 500 server error.
-func (ctx *Context) NotFoundOrServerError(logMsg string, errCheck func(error) bool, err error) {
-	if errCheck(err) {
-		ctx.notFoundInternal(logMsg, err)
+func (ctx *Context) NotFoundOrServerError(logMsg string, errCheck func(error) bool, logErr error) {
+	if errCheck(logErr) {
+		ctx.notFoundInternal(logMsg, logErr)
 		return
 	}
-	ctx.serverErrorInternal(logMsg, err)
+	ctx.serverErrorInternal(logMsg, logErr)
 }
 
 // PlainTextBytes renders bytes as plain text
diff --git a/modules/templates/htmlrenderer.go b/modules/templates/htmlrenderer.go
index 26dd365e4c..833c2acdca 100644
--- a/modules/templates/htmlrenderer.go
+++ b/modules/templates/htmlrenderer.go
@@ -4,6 +4,7 @@
 package templates
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"errors"
@@ -18,19 +19,13 @@ import (
 	"sync/atomic"
 	texttemplate "text/template"
 
+	"code.gitea.io/gitea/modules/assetfs"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 )
 
-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 (.*)`)
-)
+var rendererKey interface{} = "templatesHtmlRenderer"
 
 type HTMLRender struct {
 	templates atomic.Pointer[template.Template]
@@ -107,11 +102,12 @@ func HTMLRenderer(ctx context.Context) (context.Context, *HTMLRender) {
 
 	renderer := &HTMLRender{}
 	if err := renderer.CompileTemplates(); err != nil {
-		wrapFatal(handleNotDefinedPanicError(err))
-		wrapFatal(handleUnexpected(err))
-		wrapFatal(handleExpectedEnd(err))
-		wrapFatal(handleGenericTemplateError(err))
-		log.Fatal("HTMLRenderer error: %v", err)
+		p := &templateErrorPrettier{assets: AssetFS()}
+		wrapFatal(p.handleFuncNotDefinedError(err))
+		wrapFatal(p.handleUnexpectedOperandError(err))
+		wrapFatal(p.handleExpectedEndError(err))
+		wrapFatal(p.handleGenericTemplateError(err))
+		log.Fatal("HTMLRenderer CompileTemplates error: %v", err)
 	}
 	if !setting.IsProd {
 		go AssetFS().WatchLocalChanges(ctx, func() {
@@ -123,148 +119,153 @@ func HTMLRenderer(ctx context.Context) (context.Context, *HTMLRender) {
 	return context.WithValue(ctx, rendererKey, renderer), renderer
 }
 
-func wrapFatal(format string, args []interface{}) {
-	if format == "" {
+func wrapFatal(msg string) {
+	if msg == "" {
 		return
 	}
-	log.FatalWithSkip(1, format, args...)
+	log.FatalWithSkip(1, "Unable to compile templates, %s", msg)
 }
 
-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 := fmt.Sprintf("%s (provided by %s)", templateName, AssetFS().GetFileLayerName(templateName+".tmpl"))
-	lineNumber, _ := strconv.Atoi(lineNumberStr)
-	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)}
+type templateErrorPrettier struct {
+	assets *assetfs.LayeredFS
 }
 
-func handleNotDefinedPanicError(err error) (string, []interface{}) {
-	groups := notDefinedError.FindStringSubmatch(err.Error())
+var reGenericTemplateError = regexp.MustCompile(`^template: (.*):([0-9]+): (.*)`)
+
+func (p *templateErrorPrettier) handleGenericTemplateError(err error) string {
+	groups := reGenericTemplateError.FindStringSubmatch(err.Error())
 	if len(groups) != 4 {
-		return "", nil
+		return ""
 	}
-
-	templateName, lineNumberStr, functionName := groups[1], groups[2], groups[3]
-	functionName, _ = strconv.Unquote(`"` + functionName + `"`)
-	filename := fmt.Sprintf("%s (provided by %s)", templateName, AssetFS().GetFileLayerName(templateName+".tmpl"))
-	lineNumber, _ := strconv.Atoi(lineNumberStr)
-	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)}
+	tmplName, lineStr, message := groups[1], groups[2], groups[3]
+	return p.makeDetailedError(message, tmplName, lineStr, -1, "")
 }
 
-func handleUnexpected(err error) (string, []interface{}) {
-	groups := unexpectedError.FindStringSubmatch(err.Error())
-	if len(groups) != 4 {
-		return "", nil
-	}
+var reFuncNotDefinedError = regexp.MustCompile(`^template: (.*):([0-9]+): (function "(.*)" not defined)`)
 
-	templateName, lineNumberStr, unexpected := groups[1], groups[2], groups[3]
+func (p *templateErrorPrettier) handleFuncNotDefinedError(err error) string {
+	groups := reFuncNotDefinedError.FindStringSubmatch(err.Error())
+	if len(groups) != 5 {
+		return ""
+	}
+	tmplName, lineStr, message, funcName := groups[1], groups[2], groups[3], groups[4]
+	funcName, _ = strconv.Unquote(`"` + funcName + `"`)
+	return p.makeDetailedError(message, tmplName, lineStr, -1, funcName)
+}
+
+var reUnexpectedOperandError = regexp.MustCompile(`^template: (.*):([0-9]+): (unexpected "(.*)" in operand)`)
+
+func (p *templateErrorPrettier) handleUnexpectedOperandError(err error) string {
+	groups := reUnexpectedOperandError.FindStringSubmatch(err.Error())
+	if len(groups) != 5 {
+		return ""
+	}
+	tmplName, lineStr, message, unexpected := groups[1], groups[2], groups[3], groups[4]
 	unexpected, _ = strconv.Unquote(`"` + unexpected + `"`)
-	filename := fmt.Sprintf("%s (provided by %s)", templateName, AssetFS().GetFileLayerName(templateName+".tmpl"))
-	lineNumber, _ := strconv.Atoi(lineNumberStr)
-	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 p.makeDetailedError(message, tmplName, lineStr, -1, unexpected)
 }
 
-func handleExpectedEnd(err error) (string, []interface{}) {
-	groups := expectedEndError.FindStringSubmatch(err.Error())
-	if len(groups) != 4 {
-		return "", nil
+var reExpectedEndError = regexp.MustCompile(`^template: (.*):([0-9]+): (expected end; found (.*))`)
+
+func (p *templateErrorPrettier) handleExpectedEndError(err error) string {
+	groups := reExpectedEndError.FindStringSubmatch(err.Error())
+	if len(groups) != 5 {
+		return ""
 	}
-
-	templateName, lineNumberStr, unexpected := groups[1], groups[2], groups[3]
-	filename := fmt.Sprintf("%s (provided by %s)", templateName, AssetFS().GetFileLayerName(templateName+".tmpl"))
-	lineNumber, _ := strconv.Atoi(lineNumberStr)
-	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)}
+	tmplName, lineStr, message, unexpected := groups[1], groups[2], groups[3], groups[4]
+	return p.makeDetailedError(message, tmplName, lineStr, -1, unexpected)
 }
 
-const dashSeparator = "----------------------------------------------------------------------\n"
+var (
+	reTemplateExecutingError    = regexp.MustCompile(`^template: (.*):([1-9][0-9]*):([1-9][0-9]*): (executing .*)`)
+	reTemplateExecutingErrorMsg = regexp.MustCompile(`^executing "(.*)" at <(.*)>: `)
+)
 
-// GetLineFromTemplate returns a line from a template with some context
-func GetLineFromTemplate(templateName string, targetLineNum int, target string, position int) string {
-	bs, err := AssetFS().ReadFile(templateName + ".tmpl")
+func (p *templateErrorPrettier) handleTemplateRenderingError(err error) string {
+	if groups := reTemplateExecutingError.FindStringSubmatch(err.Error()); len(groups) > 0 {
+		tmplName, lineStr, posStr, msgPart := groups[1], groups[2], groups[3], groups[4]
+		target := ""
+		if groups = reTemplateExecutingErrorMsg.FindStringSubmatch(msgPart); len(groups) > 0 {
+			target = groups[2]
+		}
+		return p.makeDetailedError(msgPart, tmplName, lineStr, posStr, target)
+	} else if execErr, ok := err.(texttemplate.ExecError); ok {
+		layerName := p.assets.GetFileLayerName(execErr.Name + ".tmpl")
+		return fmt.Sprintf("asset from: %s, %s", layerName, err.Error())
+	} else {
+		return err.Error()
+	}
+}
+
+func HandleTemplateRenderingError(err error) string {
+	p := &templateErrorPrettier{assets: AssetFS()}
+	return p.handleTemplateRenderingError(err)
+}
+
+const dashSeparator = "----------------------------------------------------------------------"
+
+func (p *templateErrorPrettier) makeDetailedError(errMsg, tmplName string, lineNum, posNum any, target string) string {
+	code, layer, err := p.assets.ReadLayeredFile(tmplName + ".tmpl")
 	if err != nil {
-		return fmt.Sprintf("(unable to read template file: %v)", err)
+		return fmt.Sprintf("template error: %s, and unable to find template file %q", errMsg, tmplName)
 	}
-
-	sb := &strings.Builder{}
-
-	// Write the header
-	sb.WriteString(dashSeparator)
-
-	var lineBs []byte
-
-	// Iterate through the lines from the asset file to find the target line
-	for start, currentLineNum := 0, 1; currentLineNum <= targetLineNum && start < len(bs); currentLineNum++ {
-		// Find the next new line
-		end := bytes.IndexByte(bs[start:], '\n')
-
-		// adjust the end to be a direct pointer in to []byte
-		if end < 0 {
-			end = len(bs)
-		} else {
-			end += start
-		}
-
-		// set lineBs to the current line []byte
-		lineBs = bs[start:end]
-
-		// move start to after the current new line position
-		start = end + 1
-
-		// Write 2 preceding lines + the target line
-		if targetLineNum-currentLineNum < 3 {
-			_, _ = sb.Write(lineBs)
-			_ = sb.WriteByte('\n')
-		}
+	line, err := util.ToInt64(lineNum)
+	if err != nil {
+		return fmt.Sprintf("template error: %s, unable to parse template %q line number %q", errMsg, tmplName, lineNum)
 	}
-
-	// FIXME: this algorithm could provide incorrect results and mislead the developers.
-	// For example: Undefined function "file" in template .....
-	//     {{Func .file.Addition file.Deletion .file.Addition}}
-	//             ^^^^          ^(the real error is here)
-	// The pointer is added to the first one, but the second one is the real incorrect one.
-	//
-	// If there is a provided target to look for in the line add a pointer to it
-	// e.g.                                                        ^^^^^^^
-	if target != "" {
-		targetPos := bytes.Index(lineBs, []byte(target))
-		if targetPos >= 0 {
-			position = targetPos
-		}
+	pos, err := util.ToInt64(posNum)
+	if err != nil {
+		return fmt.Sprintf("template error: %s, unable to parse template %q pos number %q", errMsg, tmplName, posNum)
 	}
-	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] = ' '
-			}
-		}
+	detail := extractErrorLine(code, int(line), int(pos), target)
 
-		// 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')
+	var msg string
+	if pos >= 0 {
+		msg = fmt.Sprintf("template error: %s:%s:%d:%d : %s", layer, tmplName, line, pos, errMsg)
+	} else {
+		msg = fmt.Sprintf("template error: %s:%s:%d : %s", layer, tmplName, line, errMsg)
 	}
-
-	// Finally write the footer
-	sb.WriteString(dashSeparator)
-
-	return sb.String()
+	return msg + "\n" + dashSeparator + "\n" + detail + "\n" + dashSeparator
+}
+
+func extractErrorLine(code []byte, lineNum, posNum int, target string) string {
+	b := bufio.NewReader(bytes.NewReader(code))
+	var line []byte
+	var err error
+	for i := 0; i < lineNum; i++ {
+		if line, err = b.ReadBytes('\n'); err != nil {
+			if i == lineNum-1 && errors.Is(err, io.EOF) {
+				err = nil
+			}
+			break
+		}
+	}
+	if err != nil {
+		return fmt.Sprintf("unable to find target line %d", lineNum)
+	}
+
+	line = bytes.TrimRight(line, "\r\n")
+	var indicatorLine []byte
+	targetBytes := []byte(target)
+	targetLen := len(targetBytes)
+	for i := 0; i < len(line); {
+		if posNum == -1 && target != "" && bytes.HasPrefix(line[i:], targetBytes) {
+			for j := 0; j < targetLen && i < len(line); j++ {
+				indicatorLine = append(indicatorLine, '^')
+				i++
+			}
+		} else if i == posNum {
+			indicatorLine = append(indicatorLine, '^')
+			i++
+		} else {
+			if line[i] == '\t' {
+				indicatorLine = append(indicatorLine, '\t')
+			} else {
+				indicatorLine = append(indicatorLine, ' ')
+			}
+			i++
+		}
+	}
+	// if the indicatorLine only contains spaces, trim it together
+	return strings.TrimRight(string(line)+"\n"+string(indicatorLine), " \t\r\n")
 }
diff --git a/modules/templates/htmlrenderer_test.go b/modules/templates/htmlrenderer_test.go
new file mode 100644
index 0000000000..2a74b74c23
--- /dev/null
+++ b/modules/templates/htmlrenderer_test.go
@@ -0,0 +1,106 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package templates
+
+import (
+	"errors"
+	"html/template"
+	"os"
+	"strings"
+	"testing"
+
+	"code.gitea.io/gitea/modules/assetfs"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestExtractErrorLine(t *testing.T) {
+	cases := []struct {
+		code   string
+		line   int
+		pos    int
+		target string
+		expect string
+	}{
+		{"hello world\nfoo bar foo bar\ntest", 2, -1, "bar", `
+foo bar foo bar
+    ^^^     ^^^
+`},
+
+		{"hello world\nfoo bar foo bar\ntest", 2, 4, "bar", `
+foo bar foo bar
+    ^
+`},
+
+		{
+			"hello world\nfoo bar foo bar\ntest", 2, 4, "",
+			`
+foo bar foo bar
+    ^
+`,
+		},
+
+		{
+			"hello world\nfoo bar foo bar\ntest", 5, 0, "",
+			`unable to find target line 5`,
+		},
+	}
+
+	for _, c := range cases {
+		actual := extractErrorLine([]byte(c.code), c.line, c.pos, c.target)
+		assert.Equal(t, strings.TrimSpace(c.expect), strings.TrimSpace(actual))
+	}
+}
+
+func TestHandleError(t *testing.T) {
+	dir := t.TempDir()
+
+	p := &templateErrorPrettier{assets: assetfs.Layered(assetfs.Local("tmp", dir))}
+
+	test := func(s string, h func(error) string, expect string) {
+		err := os.WriteFile(dir+"/test.tmpl", []byte(s), 0o644)
+		assert.NoError(t, err)
+		tmpl := template.New("test")
+		_, err = tmpl.Parse(s)
+		assert.Error(t, err)
+		msg := h(err)
+		assert.EqualValues(t, strings.TrimSpace(expect), strings.TrimSpace(msg))
+	}
+
+	test("{{", p.handleGenericTemplateError, `
+template error: tmp:test:1 : unclosed action
+----------------------------------------------------------------------
+{{
+----------------------------------------------------------------------
+`)
+
+	test("{{Func}}", p.handleFuncNotDefinedError, `
+template error: tmp:test:1 : function "Func" not defined
+----------------------------------------------------------------------
+{{Func}}
+  ^^^^
+----------------------------------------------------------------------
+`)
+
+	test("{{'x'3}}", p.handleUnexpectedOperandError, `
+template error: tmp:test:1 : unexpected "3" in operand
+----------------------------------------------------------------------
+{{'x'3}}
+     ^
+----------------------------------------------------------------------
+`)
+
+	// no idea about how to trigger such strange error, so mock an error to test it
+	err := os.WriteFile(dir+"/test.tmpl", []byte("god knows XXX"), 0o644)
+	assert.NoError(t, err)
+	expectedMsg := `
+template error: tmp:test:1 : expected end; found XXX
+----------------------------------------------------------------------
+god knows XXX
+          ^^^
+----------------------------------------------------------------------
+`
+	actualMsg := p.handleExpectedEndError(errors.New("template: test:1: expected end; found XXX"))
+	assert.EqualValues(t, strings.TrimSpace(expectedMsg), strings.TrimSpace(actualMsg))
+}
diff --git a/templates/base/head_script.tmpl b/templates/base/head_script.tmpl
index 670d146b56..d19fae629b 100644
--- a/templates/base/head_script.tmpl
+++ b/templates/base/head_script.tmpl
@@ -6,6 +6,7 @@ If you introduce mistakes in it, Gitea JavaScript code wouldn't run correctly.
 <script>
 	window.addEventListener('error', function(e) {window._globalHandlerErrors=window._globalHandlerErrors||[]; window._globalHandlerErrors.push(e);});
 	window.config = {
+		initCount: (window.config?.initCount ?? 0) + 1,
 		appUrl: '{{AppUrl}}',
 		appSubUrl: '{{AppSubUrl}}',
 		assetVersionEncoded: encodeURIComponent('{{AssetVersion}}'), // will be used in URL construction directly
diff --git a/templates/devtest/tmplerr-sub.tmpl b/templates/devtest/tmplerr-sub.tmpl
new file mode 100644
index 0000000000..c4c8516307
--- /dev/null
+++ b/templates/devtest/tmplerr-sub.tmpl
@@ -0,0 +1,3 @@
+sub template triggers an executing error
+
+		{{.locale.NoSuch "asdf"}}
diff --git a/templates/devtest/tmplerr.tmpl b/templates/devtest/tmplerr.tmpl
new file mode 100644
index 0000000000..2fe3f1effd
--- /dev/null
+++ b/templates/devtest/tmplerr.tmpl
@@ -0,0 +1,12 @@
+{{template "base/head" .}}
+<div class="page-content devtest">
+	<div class="gt-df">
+		<div style="width: 80%; ">
+			hello hello hello hello hello hello hello hello hello hello
+		</div>
+		<div style="width: 20%;">
+			{{template "devtest/tmplerr-sub" .}}
+		</div>
+	</div>
+</div>
+{{template "base/footer" .}}
diff --git a/templates/status/404.tmpl b/templates/status/404.tmpl
index 89fb01fb20..a55e2fbad9 100644
--- a/templates/status/404.tmpl
+++ b/templates/status/404.tmpl
@@ -1,5 +1,5 @@
 {{template "base/head" .}}
-<div role="main" aria-label="{{.Title}}" class="page-content ui container center gt-full-screen-width {{if .IsRepo}}repository{{end}}">
+<div role="main" aria-label="{{.Title}}" class="page-content ui container center gt-w-screen {{if .IsRepo}}repository{{end}}">
 	{{if .IsRepo}}{{template "repo/header" .}}{{end}}
 	<div class="ui container center">
 		<p style="margin-top: 100px"><img src="{{AssetUrlPrefix}}/img/404.png" alt="404"></p>
diff --git a/templates/status/500.tmpl b/templates/status/500.tmpl
index 50f19ec664..e8984aa18d 100644
--- a/templates/status/500.tmpl
+++ b/templates/status/500.tmpl
@@ -1,13 +1,36 @@
 {{template "base/head" .}}
-<div role="main" aria-label="{{.Title}}" class="page-content ui container gt-full-screen-width center">
-	<p style="margin-top: 100px"><img src="{{AssetUrlPrefix}}/img/500.png" alt="500"></p>
+<div role="main" aria-label="{{.Title}}" class="page-content gt-w-screen status-page-500">
+	<p class="gt-mt-5 center"><img src="{{AssetUrlPrefix}}/img/500.png" alt="Internal Server Error"></p>
 	<div class="ui divider"></div>
-	<br>
-	{{if .ErrorMsg}}
-		<p>{{.locale.Tr "error.occurred"}}:</p>
-		<pre style="text-align: left">{{.ErrorMsg}}</pre>
-	{{end}}
-	{{if .ShowFooterVersion}}<p>{{.locale.Tr "admin.config.app_ver"}}: {{AppVer}}</p>{{end}}
-	{{if .IsAdmin}}<p>{{.locale.Tr "error.report_message"  | Safe}}</p>{{end}}
+
+	<div class="ui container gt-mt-5">
+		{{if .ErrorMsg}}
+			<p>{{.locale.Tr "error.occurred"}}:</p>
+			<pre class="gt-whitespace-pre-wrap">{{.ErrorMsg}}</pre>
+		{{end}}
+
+		<div class="center gt-mt-5">
+			{{if .ShowFooterVersion}}<p>{{.locale.Tr "admin.config.app_ver"}}: {{AppVer}}</p>{{end}}
+			{{if .IsAdmin}}<p>{{.locale.Tr "error.report_message"  | Safe}}</p>{{end}}
+		</div>
+	</div>
 </div>
+{{/* when a sub-template triggers an 500 error, its parent template has been partially rendered,
+then the 500 page will be rendered after that partially rendered page, the HTML/JS are totally broken.
+so use this inline script to try to move it to main viewport */}}
+<script type="module">
+const embedded = document.querySelector('.page-content .page-content.status-page-500');
+if (embedded) {
+	// move footer to main view
+	const footer = document.querySelector('footer');
+	if (footer) document.querySelector('body').append(footer);
+	// move the 500 error page content to main view
+	const embeddedParent = embedded.parentNode;
+	let main = document.querySelector('.page-content');
+	main = main ?? document.querySelector('body');
+	main.prepend(document.createElement('hr'));
+	main.prepend(embedded);
+	embeddedParent.remove(); // remove the unrelated 500-page elements (eg: the duplicate nav bar)
+}
+</script>
 {{template "base/footer" .}}
diff --git a/web_src/css/helpers.css b/web_src/css/helpers.css
index dcec79fcf6..8d64bd751b 100644
--- a/web_src/css/helpers.css
+++ b/web_src/css/helpers.css
@@ -46,8 +46,8 @@
   text-overflow: ellipsis !important;
 }
 
-.gt-full-screen-width { width: 100vw !important; }
-.gt-full-screen-height { height: 100vh !important; }
+.gt-w-screen { width: 100vw !important; }
+.gt-h-screen { height: 100vh !important; }
 
 .gt-rounded { border-radius: var(--border-radius) !important; }
 .gt-rounded-top { border-radius: var(--border-radius) var(--border-radius) 0 0 !important; }
@@ -202,6 +202,7 @@
 
 .gt-shrink-0 { flex-shrink: 0 !important; }
 .gt-whitespace-nowrap { white-space: nowrap !important; }
+.gt-whitespace-pre-wrap { white-space: pre-wrap !important; }
 
 @media (max-width: 767px) {
   .gt-db-small { display: block !important; }
diff --git a/web_src/js/bootstrap.js b/web_src/js/bootstrap.js
index 54b7c62887..4f88c600f5 100644
--- a/web_src/js/bootstrap.js
+++ b/web_src/js/bootstrap.js
@@ -20,6 +20,10 @@ export function showGlobalErrorMessage(msg) {
  * @param {ErrorEvent} e
  */
 function processWindowErrorEvent(e) {
+  if (window.config.initCount > 1) {
+    // the page content has been loaded many times, the HTML/JS are totally broken, don't need to show error message
+    return;
+  }
   if (!e.error && e.lineno === 0 && e.colno === 0 && e.filename === '' && window.navigator.userAgent.includes('FxiOS/')) {
     // At the moment, Firefox (iOS) (10x) has an engine bug. See https://github.com/go-gitea/gitea/issues/20240
     // If a script inserts a newly created (and content changed) element into DOM, there will be a nonsense error event reporting: Script error: line 0, col 0.
@@ -33,7 +37,13 @@ function initGlobalErrorHandler() {
   if (!window.config) {
     showGlobalErrorMessage(`Gitea JavaScript code couldn't run correctly, please check your custom templates`);
   }
-
+  if (window.config.initCount > 1) {
+    // when a sub-templates triggers an 500 error, its parent template has been partially rendered,
+    // then the 500 page will be rendered after that partially rendered page, which will cause the initCount > 1
+    // in this case, the page is totally broken, so do not do any further error handling
+    console.error('initGlobalErrorHandler: Gitea global config system has already been initialized, there must be something else wrong');
+    return;
+  }
   // we added an event handler for window error at the very beginning of <script> of page head
   // the handler calls `_globalHandlerErrors.push` (array method) to record all errors occur before this init
   // then in this init, we can collect all error events and show them