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

@@ -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

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

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

templates/devtest/tmplerr-sub.tmpl

@@ -0,0 +1,3 @@
+sub template triggers an executing error
+
+		{{.locale.NoSuch "asdf"}}

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" .}}

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>

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>
+
+	<div class="ui container gt-mt-5">
 		{{if .ErrorMsg}}
 			<p>{{.locale.Tr "error.occurred"}}:</p>
-		<pre style="text-align: left">{{.ErrorMsg}}</pre>
+			<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" .}}

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

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