From 13644f4ecb04677524c13e6360d48842e7cf9e65 Mon Sep 17 00:00:00 2001
From: Amy Gale Ruth Bowersox <amy@erbosoft.com>
Date: Sun, 19 Oct 2025 16:06:08 -0600
Subject: [PATCH] improved error page handling by tapping into Echo and its
 middleware

---
 errors.go          | 41 +++++++++++++++++++++++++++++++++++++++++
 logging.go         | 15 ++++++++++++++-
 main.go            |  5 ++++-
 ui/render_wrap.go  | 10 +++++-----
 ui/views/error.jet |  2 +-
 5 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/errors.go b/errors.go
index 87834ec..529cdca 100644
--- a/errors.go
+++ b/errors.go
@@ -10,7 +10,12 @@
 package main
 
 import (
+	"fmt"
+	"net/http"
+
 	"git.erbosoft.com/amy/amsterdam/ui"
+	"github.com/labstack/echo/v4"
+	log "github.com/sirupsen/logrus"
 )
 
 /* NotImplPage is used for all TODO links, to show that something hasn't yet been implemented.
@@ -27,3 +32,39 @@ func NotImplPage(ctxt ui.AmContext) (string, any, error) {
 	ctxt.VarMap().Set("path", ctxt.URLPath())
 	return "framed_template", "notimpl.jet", nil
 }
+
+/* AmErrorHandler handles all the mundane HTTP errors generated by the Echo engine.
+ * Parameters:
+ *     err - The error to be handled.
+ *     c - The Echo context error is being handled on.
+ */
+func AmErrorHandler(err error, c echo.Context) {
+	if c.Response().Committed {
+		return
+	}
+
+	errCode := http.StatusInternalServerError
+	if he, ok := err.(*echo.HTTPError); ok {
+		errCode = he.Code
+	}
+
+	amctxt := ui.AmContextFromEchoContext(c)
+	if amctxt == nil {
+		var qerr error
+		amctxt, qerr = ui.AmCreateContext(c)
+		if qerr != nil {
+			log.Errorf("failed to create AmContext in error handler: %v", qerr)
+			c.String(errCode, fmt.Sprintf("error %d: %v", errCode, err))
+			return
+		}
+	}
+	amctxt.SetLeftMenu("top")
+	amctxt.SetRC(errCode)
+	amctxt.VarMap().Set("error", err.Error())
+	// TODO: come up with a way to shift templates and titles for different error codes
+	amctxt.VarMap().Set("amsterdam_pageTitle", "Amsterdam Internal Server Error")
+	cerr := ui.AmSendPageData(c, amctxt, "framed_template", "error.jet")
+	if cerr != nil {
+		log.Errorf("Error rendering error %d (%v): %v", errCode, err, cerr)
+	}
+}
diff --git a/logging.go b/logging.go
index 5ad737a..5fd4d71 100644
--- a/logging.go
+++ b/logging.go
@@ -11,7 +11,10 @@
 package main
 
 import (
+	"bufio"
+	"bytes"
 	"io"
+	"strings"
 	"time"
 
 	"github.com/labstack/echo/v4"
@@ -119,8 +122,18 @@ func LogrusMiddleware(next echo.HandlerFunc) echo.HandlerFunc {
 			"uri":        req.RequestURI,
 			"status":     res.Status,
 			"latency_ms": stop.Sub(start).Milliseconds(),
-			"user_agent": req.UserAgent(),
 		}).Info("handled request")
 		return err
 	}
 }
+
+// LogrusPanicLogging is a log function hooked into the recovery middleware.
+func LogrusPanicLogging(c echo.Context, err error, stack []byte) error {
+	log.Errorf("[PANIC RECOVERY] %v", err)
+	scanner := bufio.NewScanner(bytes.NewReader(stack))
+	for scanner.Scan() {
+		line := strings.ReplaceAll(scanner.Text(), "\t", "    ")
+		log.Error(line)
+	}
+	return scanner.Err()
+}
diff --git a/main.go b/main.go
index fd013d6..b139d17 100644
--- a/main.go
+++ b/main.go
@@ -33,8 +33,11 @@ func setupEcho() *echo.Echo {
 	e := echo.New()
 	e.Logger = &EchoLogrusAdapter{}
 	e.Renderer = &ui.TemplateRenderer{}
+	e.HTTPErrorHandler = AmErrorHandler
 	if !config.CommandLine.DebugPanic {
-		e.Use(middleware.Recover())
+		e.Use(middleware.RecoverWithConfig(middleware.RecoverConfig{
+			LogErrorFunc: LogrusPanicLogging,
+		}))
 	} else {
 		log.Warn("WARNING: --debug-panic in effect - DO NOT use this in production!")
 	}
diff --git a/ui/render_wrap.go b/ui/render_wrap.go
index b72bf8d..3781b4b 100644
--- a/ui/render_wrap.go
+++ b/ui/render_wrap.go
@@ -21,7 +21,7 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-func sendPageData(ctxt echo.Context, amctxt AmContext, command string, data any) error {
+func AmSendPageData(ctxt echo.Context, amctxt AmContext, command string, data any) error {
 	var err error
 	switch command {
 	case "bytes":
@@ -103,7 +103,7 @@ func AmWrap(myfunc func(AmContext) (string, any, error)) echo.HandlerFunc {
 			amctxt.VarMap().Set("amsterdam_pageTitle", "IP Address Banned")
 			amctxt.VarMap().Set("message", banmsg)
 			amctxt.SetRC(http.StatusForbidden)
-			return sendPageData(ctxt, amctxt, "framed_template", "ipban.jet")
+			return AmSendPageData(ctxt, amctxt, "framed_template", "ipban.jet")
 		}
 
 		// Check for cookie login.
@@ -123,7 +123,7 @@ func AmWrap(myfunc func(AmContext) (string, any, error)) echo.HandlerFunc {
 					}
 					if !user.VerifyEMail {
 						// bounce to E-mail verification before we go anywhere
-						return sendPageData(ctxt, amctxt, "redirect",
+						return AmSendPageData(ctxt, amctxt, "redirect",
 							"/verify?tgt="+url.QueryEscape(ctxt.Request().URL.Path))
 					}
 				} else {
@@ -140,7 +140,7 @@ func AmWrap(myfunc func(AmContext) (string, any, error)) echo.HandlerFunc {
 				ctxt.Logger().Errorf("Session save error: %v", err)
 				return err
 			}
-			err = sendPageData(ctxt, amctxt, what, rc)
+			err = AmSendPageData(ctxt, amctxt, what, rc)
 			if err != nil {
 				ctxt.Logger().Errorf("Rendering error: %v", err)
 			}
@@ -148,7 +148,7 @@ func AmWrap(myfunc func(AmContext) (string, any, error)) echo.HandlerFunc {
 			ctxt.Logger().Errorf("Page function error: %v", err)
 			_, rc, _ = ErrorPage(amctxt, err)
 			amctxt.SetRC(http.StatusInternalServerError)
-			newerr := sendPageData(ctxt, amctxt, "framed_template", rc)
+			newerr := AmSendPageData(ctxt, amctxt, "framed_template", rc)
 			err = newerr
 		}
 		return err
diff --git a/ui/views/error.jet b/ui/views/error.jet
index e220937..53e5607 100644
--- a/ui/views/error.jet
+++ b/ui/views/error.jet
@@ -15,7 +15,7 @@
                 The Amsterdam server encountered an error: {{ CapitalizeString(error) }}.
             </p>
             <p class="text-black text-sm mb-4">
-                <a href="/">Click here</a> to return to the home page.
+                <a href="/">Click here to return to the home page.</a>
             </p>
         </div>
     </div>