diff --git a/README.md b/README.md
index 8173e92..24a93d5 100644
--- a/README.md
+++ b/README.md
@@ -2932,6 +2932,12 @@ local record instead of nothing. What that placement gives up is
recovery of a panic in the six entries above it, none of which does
more than set a header or start a timer.
+Each admin page route group (`/pages`, `/user/*`, `/sources`,
+`/source/*`) starts with its own **Recoverer** and, if `SENTRY_DSN` is
+set, its own **Sentry** error reporting. That Recoverer answers a panic
+with the `500` error page in the normal layout; the global one keeps
+the plain-text `500` for every other route.
+
Additionally, form endpoints (`/pages`, `/user/*`, `/sources`,
`/source/*`) apply a **MaxBodySize** middleware that limits
POST/PUT/PATCH request bodies to 1 MB. It is registered ahead of the
diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go
index bef9dff..73efd54 100644
--- a/internal/handlers/auth.go
+++ b/internal/handlers/auth.go
@@ -36,7 +36,7 @@ func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
err := r.ParseForm()
if err != nil {
h.log.Error("failed to parse form", "error", err)
- http.Error(w, "Bad request", http.StatusBadRequest)
+ h.renderError(w, r, http.StatusBadRequest)
return
}
@@ -166,11 +166,7 @@ func (h *Handlers) authenticateUser(
valid, err := database.VerifyPassword(password, user.Password)
if err != nil {
- h.log.Error("failed to verify password", "error", err)
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.serverError(w, r, "failed to verify password", err)
return user, err
}
@@ -242,24 +238,14 @@ func (h *Handlers) createAuthenticatedSession(
) error {
oldSess, err := h.session.Get(r)
if err != nil {
- h.log.Error("failed to get session", "error", err)
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.serverError(w, r, "failed to get session", err)
return err
}
sess, err := h.session.Regenerate(r, w, oldSess)
if err != nil {
- h.log.Error(
- "failed to regenerate session", "error", err,
- )
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.serverError(w, r, "failed to regenerate session", err)
return err
}
@@ -268,11 +254,7 @@ func (h *Handlers) createAuthenticatedSession(
err = h.session.Save(r, w, sess)
if err != nil {
- h.log.Error("failed to save session", "error", err)
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.serverError(w, r, "failed to save session", err)
return err
}
diff --git a/internal/handlers/delivery_replay.go b/internal/handlers/delivery_replay.go
index ee9425b..d7a6f4f 100644
--- a/internal/handlers/delivery_replay.go
+++ b/internal/handlers/delivery_replay.go
@@ -105,9 +105,7 @@ func (h *Handlers) HandleDeliveryReplay() http.HandlerFunc {
// middleware, which runs before CSRF parses the form.
err := r.ParseForm()
if err != nil {
- http.Error(
- w, "Bad request", http.StatusBadRequest,
- )
+ h.renderError(w, r, http.StatusBadRequest)
return
}
@@ -124,14 +122,14 @@ func (h *Handlers) replayDelivery(
webhook database.Webhook,
) {
if !h.dbMgr.DBExists(webhook.ID) {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
if err != nil {
- h.serverError(w, "failed to get webhook database", err)
+ h.serverError(w, r, "failed to get webhook database", err)
return
}
@@ -173,7 +171,7 @@ func (h *Handlers) loadReplaySource(
&original, "id = ?", chi.URLParam(r, "deliveryID"),
).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return nil, false
}
@@ -195,7 +193,7 @@ func (h *Handlers) queueReplay(
)
if err != nil {
h.serverError(
- w, "failed to count in-flight deliveries", err,
+ w, r, "failed to count in-flight deliveries", err,
)
return
@@ -212,7 +210,7 @@ func (h *Handlers) queueReplay(
err = webhookDB.
First(&event, "id = ?", original.EventID).Error
if err != nil {
- h.serverError(w, "failed to load event for replay", err)
+ h.serverError(w, r, "failed to load event for replay", err)
return
}
@@ -222,7 +220,7 @@ func (h *Handlers) queueReplay(
)
if err != nil {
h.serverError(
- w, "failed to create replay delivery", err,
+ w, r, "failed to create replay delivery", err,
)
return
diff --git a/internal/handlers/error_page_test.go b/internal/handlers/error_page_test.go
new file mode 100644
index 0000000..1b87fe9
--- /dev/null
+++ b/internal/handlers/error_page_test.go
@@ -0,0 +1,53 @@
+package handlers_test
+
+import (
+ "context"
+ "html/template"
+ "net/http"
+ "net/http/httptest"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "sneak.berlin/go/webhooker/internal/handlers"
+)
+
+// TestErrorPage_RenderFailureKeepsStatus proves that an error page
+// which cannot render answers with the status it was reporting, as
+// plain text, and is not attempted again: a page whose own render
+// fails reaches the error page, and the error page failing as well
+// ends there with the 500.
+func TestErrorPage_RenderFailureKeepsStatus(t *testing.T) {
+ t.Parallel()
+
+ var h *handlers.Handlers
+
+ app := newTestApp(t, &h)
+ app.RequireStart()
+
+ t.Cleanup(app.RequireStop)
+
+ // .Status is an int, so asking it for a field fails the render.
+ failing := `{{.Status.Missing}}`
+ h.AddTemplateForTest("error.html", template.Must(
+ template.New("error").Parse(failing),
+ ))
+ h.AddTemplateForTest("failing.html", template.Must(
+ template.New("failing").Parse(`{{.Data.Missing}}`),
+ ))
+
+ req := httptest.NewRequestWithContext(
+ context.Background(), http.MethodGet, "/", nil,
+ )
+
+ w := httptest.NewRecorder()
+ h.HandleErrorPage(http.StatusNotFound).ServeHTTP(w, req)
+
+ assert.Equal(t, http.StatusNotFound, w.Code)
+ assert.Equal(t, "Not Found\n", w.Body.String())
+
+ w = httptest.NewRecorder()
+ h.RenderTemplateForTest(w, req, "failing.html", 0)
+
+ assert.Equal(t, http.StatusInternalServerError, w.Code)
+ assert.Equal(t, "Internal Server Error\n", w.Body.String())
+}
diff --git a/internal/handlers/event_body.go b/internal/handlers/event_body.go
index d46b9a5..cf2452f 100644
--- a/internal/handlers/event_body.go
+++ b/internal/handlers/event_body.go
@@ -52,7 +52,7 @@ func (h *Handlers) HandleEventBodyDownload() http.HandlerFunc {
// steered by a client.
eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -103,21 +103,21 @@ func (h *Handlers) serveEventBody(
eventID string,
) {
if !h.dbMgr.DBExists(webhook.ID) {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
if err != nil {
- h.serverError(w, "failed to get webhook database", err)
+ h.serverError(w, r, "failed to get webhook database", err)
return
}
body, found, err := eventBody(webhookDB, webhook.ID, eventID)
if err != nil {
- h.serverError(w, "failed to read event body", err)
+ h.serverError(w, r, "failed to read event body", err)
return
}
@@ -130,7 +130,7 @@ func (h *Handlers) serveEventBody(
// row and the whole body is served, or it does not and the
// response is a clean 404.
if !found {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
diff --git a/internal/handlers/event_resubmit.go b/internal/handlers/event_resubmit.go
index 21e54b8..46df054 100644
--- a/internal/handlers/event_resubmit.go
+++ b/internal/handlers/event_resubmit.go
@@ -99,7 +99,7 @@ func (h *Handlers) HandleEventResubmit() http.HandlerFunc {
// middleware, which runs before CSRF parses the form.
err := r.ParseForm()
if err != nil {
- http.Error(w, "Bad request", http.StatusBadRequest)
+ h.renderError(w, r, http.StatusBadRequest)
return
}
@@ -120,20 +120,20 @@ func (h *Handlers) resubmitEvent(
// alphabet rather than from the request.
eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
if !h.dbMgr.DBExists(webhook.ID) {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
if err != nil {
- h.serverError(w, "failed to get webhook database", err)
+ h.serverError(w, r, "failed to get webhook database", err)
return
}
@@ -147,7 +147,7 @@ func (h *Handlers) resubmitEvent(
webhookDB, webhook.ID, eventID.String(),
)
if err != nil {
- h.serverError(w, "failed to load event to resubmit", err)
+ h.serverError(w, r, "failed to load event to resubmit", err)
return
}
@@ -155,7 +155,7 @@ func (h *Handlers) resubmitEvent(
// A miss is a 404 whether the event was reaped, belongs to
// another webhook, or never existed.
if !found {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -207,7 +207,7 @@ func (h *Handlers) queueResubmit(
// inactive one is skipped rather than refused.
targets, err := h.loadActiveTargets(webhook.ID)
if err != nil {
- h.serverError(w, "failed to query targets", err)
+ h.serverError(w, r, "failed to query targets", err)
return
}
@@ -225,7 +225,7 @@ func (h *Handlers) queueResubmit(
targets,
)
if err != nil {
- h.serverError(w, "failed to store resubmitted event", err)
+ h.serverError(w, r, "failed to store resubmitted event", err)
return
}
diff --git a/internal/handlers/export_test.go b/internal/handlers/export_test.go
index 4612e94..dfd9018 100644
--- a/internal/handlers/export_test.go
+++ b/internal/handlers/export_test.go
@@ -1,9 +1,11 @@
package handlers
import (
+ "context"
"html/template"
"log/slog"
"net/http"
+ "net/http/httptest"
"sneak.berlin/go/webhooker/internal/database"
)
@@ -63,12 +65,20 @@ func (s *Handlers) LoadEventLogViewsForTest(
page int,
) []EventLogView {
views, _, _ := s.loadEventsWithDeliveries(
- w, webhook, nil, page,
+ w, newRequestForTest(), webhook, nil, page,
)
return views
}
+// newRequestForTest is the request the helpers here pass on for
+// callers that have none: it is used only to render the error page.
+func newRequestForTest() *http.Request {
+ return httptest.NewRequestWithContext(
+ context.Background(), http.MethodGet, "/", nil,
+ )
+}
+
// AddTemplateForTest registers a template under a page name so that
// the handlers_test package can drive the render path with a
// template of its own.
@@ -122,5 +132,5 @@ func (s *Handlers) BuildDatabaseTargetConfigForTest(
w http.ResponseWriter,
expiry string,
) (string, error) {
- return s.buildDatabaseTargetConfig(w, expiry)
+ return s.buildDatabaseTargetConfig(w, newRequestForTest(), expiry)
}
diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go
index ad3bcd5..3aadaa9 100644
--- a/internal/handlers/handlers.go
+++ b/internal/handlers/handlers.go
@@ -135,6 +135,7 @@ func New(
"source_edit.html": parsePageTemplate("source_edit.html"),
"source_logs.html": parsePageTemplate("source_logs.html"),
"target_edit.html": parsePageTemplate("target_edit.html"),
+ "error.html": parsePageTemplate("error.html"),
}
lc.Append(fx.Hook{
@@ -146,6 +147,16 @@ func New(
return s, nil
}
+// HandleErrorPage returns a handler that answers every request with
+// the error page for status. The router uses it for unknown paths, the
+// CSRF middleware for a refused form, and each admin page route
+// group's recoverer for a panic.
+func (s *Handlers) HandleErrorPage(status int) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ s.renderError(w, r, status)
+ }
+}
+
func (s *Handlers) respondJSON(
w http.ResponseWriter,
_ *http.Request,
@@ -163,15 +174,76 @@ func (s *Handlers) respondJSON(
}
}
-// serverError logs an error and sends a 500 response.
+// serverError logs an error and answers with the 500 error page.
func (s *Handlers) serverError(
- w http.ResponseWriter, msg string, err error,
+ w http.ResponseWriter, r *http.Request, msg string, err error,
) {
s.log.Error(msg, "error", err)
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ s.renderError(w, r, http.StatusInternalServerError)
+}
+
+// renderError answers with status and the error page: the normal
+// layout, one fixed line explaining the status, and a link back to the
+// webhook list, or to sign-in when nobody is signed in.
+//
+// It renders the page itself rather than through renderTemplate,
+// whose own failure comes here. If the error page cannot render
+// either, the answer is the same status in plain text: never a second
+// attempt, and never a different status.
+func (s *Handlers) renderError(
+ w http.ResponseWriter,
+ r *http.Request,
+ status int,
+) {
+ // The page names the signed-in user, and some error pages are
+ // served outside the routes where NoCache runs.
+ w.Header().Set("Cache-Control", "no-store")
+
+ data := s.pageData(r, map[string]any{
+ "Status": status,
+ "StatusText": http.StatusText(status),
+ "Message": errorPageText(status),
+ })
+
+ var buf bytes.Buffer
+
+ err := s.templates["error.html"].Execute(&buf, data)
+ if err != nil {
+ s.log.Error("failed to render error page", "error", err)
+ http.Error(w, http.StatusText(status), status)
+
+ return
+ }
+
+ w.Header().Set("Content-Type", "text/html; charset=utf-8")
+ w.WriteHeader(status)
+
+ _, err = buf.WriteTo(w)
+ if err != nil {
+ s.log.Error("failed to write error page", "error", err)
+ }
+}
+
+// errorPageText is the line the error page shows for status. It is
+// fixed per status, so the page tells the reader no more than the
+// plain-text answers it replaced did.
+func errorPageText(status int) string {
+ switch status {
+ case http.StatusBadRequest:
+ return "The request could not be read."
+ case http.StatusForbidden:
+ return "The request was refused. If it came from a form " +
+ "left open for a long time, reload the page and try " +
+ "again."
+ case http.StatusNotFound:
+ return "There is nothing here. It may have been deleted, " +
+ "or the address may be wrong."
+ case http.StatusServiceUnavailable:
+ return "The server is busy. Please try again in a moment."
+ default: // http.StatusInternalServerError
+ return "Something went wrong on the server. Please try " +
+ "again."
+ }
}
// UserInfo represents user information for templates
@@ -224,14 +296,17 @@ func (s *Handlers) renderTemplate(
"template not found",
"template", pageTemplate,
)
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ s.renderError(w, r, http.StatusInternalServerError)
return
}
+ s.executeTemplate(w, r, tmpl, s.pageData(r, data))
+}
+
+// pageData adds the fields the shared layout renders to a page's own
+// data.
+func (s *Handlers) pageData(r *http.Request, data any) any {
userInfo := s.getUserInfo(r)
csrfToken := middleware.CSRFToken(r)
@@ -245,19 +320,16 @@ func (s *Handlers) renderTemplate(
m["User"] = userInfo
m["CSRFToken"] = csrfToken
m["Version"] = version
- s.executeTemplate(w, tmpl, m)
- return
+ return m
}
- wrapper := templateDataWrapper{
+ return templateDataWrapper{
User: userInfo,
CSRFToken: csrfToken,
Version: version,
Data: data,
}
-
- s.executeTemplate(w, tmpl, wrapper)
}
// executeTemplate renders the template into a buffer and writes to
@@ -270,6 +342,7 @@ func (s *Handlers) renderTemplate(
// this reason.
func (s *Handlers) executeTemplate(
w http.ResponseWriter,
+ r *http.Request,
tmpl *template.Template,
data any,
) {
@@ -280,10 +353,7 @@ func (s *Handlers) executeTemplate(
s.log.Error(
"failed to execute template", "error", err,
)
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ s.renderError(w, r, http.StatusInternalServerError)
return
}
diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go
index 3e9874a..a18c914 100644
--- a/internal/handlers/handlers_test.go
+++ b/internal/handlers/handlers_test.go
@@ -307,10 +307,14 @@ func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
t, http.StatusInternalServerError, w.Code,
"a failed render must report a 500",
)
- assert.Equal(
- t, "Internal server error\n", w.Body.String(),
+ assert.NotContains(
+ t, w.Body.String(), partialPageMarker,
"the response must carry no part of the aborted page",
)
+ assert.Contains(
+ t, w.Body.String(), "500 Internal Server Error",
+ "a failed render must answer with the error page",
+ )
}
func TestBuildDatabaseTargetConfig_Valid(t *testing.T) {
diff --git a/internal/handlers/profile.go b/internal/handlers/profile.go
index 9d929b4..71fc163 100644
--- a/internal/handlers/profile.go
+++ b/internal/handlers/profile.go
@@ -1,7 +1,6 @@
package handlers
import (
- "context"
"net/http"
"github.com/go-chi/chi"
@@ -37,14 +36,14 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
err := r.ParseForm()
if err != nil {
h.log.Error("failed to parse form", "error", err)
- http.Error(w, "Bad request", http.StatusBadRequest)
+ h.renderError(w, r, http.StatusBadRequest)
return
}
successMessage, errorMessage, handled := h.applyPasswordChange(
- r.Context(),
w,
+ r,
sessionUsername,
// PostFormValue, not FormValue: the credential must
// come from the body, never from the query string.
@@ -66,12 +65,12 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
// applyPasswordChange verifies the current password and, on success,
// persists a fresh hash for the user, reusing the same helpers that
// bootstrap the admin user. It returns the success and error messages
-// to display on the profile page. On an internal failure it writes a
-// 500 response itself and returns handled=false, signalling the caller
+// to display on the profile page. On an internal failure it writes the
+// error page itself and returns handled=false, signalling the caller
// to stop without re-rendering the page.
func (h *Handlers) applyPasswordChange(
- ctx context.Context,
w http.ResponseWriter,
+ r *http.Request,
username, currentPassword, newPassword, confirmPassword string,
) (string, string, bool) {
// This endpoint verifies one password and hashes another, at
@@ -79,15 +78,10 @@ func (h *Handlers) applyPasswordChange(
// endpoint uses. The bound is per hash, not per endpoint: leaving
// this path outside it would leave a hole in it. The slot is held
// across both hashes.
- release, ok := h.mw.BeginPasswordVerification(ctx)
+ release, ok := h.mw.BeginPasswordVerification(r.Context())
if !ok {
h.log.Warn("password verification capacity exhausted")
- http.Error(
- w,
- "The server is busy verifying credentials. "+
- "Please try again.",
- http.StatusServiceUnavailable,
- )
+ h.renderError(w, r, http.StatusServiceUnavailable)
return "", "", false
}
@@ -103,7 +97,7 @@ func (h *Handlers) applyPasswordChange(
).First(&user).Error
if err != nil {
h.serverError(
- w, "failed to load user for password change", err,
+ w, r, "failed to load user for password change", err,
)
return "", "", false
@@ -113,7 +107,7 @@ func (h *Handlers) applyPasswordChange(
currentPassword, user.Password,
)
if err != nil {
- h.serverError(w, "failed to verify password", err)
+ h.serverError(w, r, "failed to verify password", err)
return "", "", false
}
@@ -132,7 +126,7 @@ func (h *Handlers) applyPasswordChange(
hashedPassword, err := database.HashPassword(newPassword)
if err != nil {
- h.serverError(w, "failed to hash new password", err)
+ h.serverError(w, r, "failed to hash new password", err)
return "", "", false
}
@@ -141,7 +135,7 @@ func (h *Handlers) applyPasswordChange(
"password", hashedPassword,
).Error
if err != nil {
- h.serverError(w, "failed to update password", err)
+ h.serverError(w, r, "failed to update password", err)
return "", "", false
}
@@ -162,7 +156,7 @@ func (h *Handlers) profileOwnerOrDeny(
) (string, string, bool) {
requestedUsername := chi.URLParam(r, "username")
if requestedUsername == "" {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return "", "", false
}
@@ -172,7 +166,7 @@ func (h *Handlers) profileOwnerOrDeny(
// unexpected retrieval error.
sess, err := h.session.Get(r)
if err != nil {
- h.serverError(w, "failed to get session", err)
+ h.serverError(w, r, "failed to get session", err)
return "", "", false
}
@@ -180,10 +174,7 @@ func (h *Handlers) profileOwnerOrDeny(
sessionUsername, ok := h.session.GetUsername(sess)
if !ok {
h.log.Error("authenticated session missing username")
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.renderError(w, r, http.StatusInternalServerError)
return "", "", false
}
@@ -191,17 +182,14 @@ func (h *Handlers) profileOwnerOrDeny(
sessionUserID, ok := h.session.GetUserID(sess)
if !ok {
h.log.Error("authenticated session missing user ID")
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.renderError(w, r, http.StatusInternalServerError)
return "", "", false
}
// Only allow users to act on their own profile.
if requestedUsername != sessionUsername {
- http.Error(w, "Forbidden", http.StatusForbidden)
+ h.renderError(w, r, http.StatusForbidden)
return "", "", false
}
diff --git a/internal/handlers/profile_test.go b/internal/handlers/profile_test.go
index c68a35e..446acad 100644
--- a/internal/handlers/profile_test.go
+++ b/internal/handlers/profile_test.go
@@ -128,7 +128,9 @@ func TestUserRoute_Unauthenticated_RedirectedByMiddleware(t *testing.T) {
var sess *session.Session
- app := newTestApp(t, &log, &cfg, &sess)
+ var h *handlers.Handlers
+
+ app := newTestApp(t, &log, &cfg, &sess, &h)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -139,7 +141,7 @@ func TestUserRoute_Unauthenticated_RedirectedByMiddleware(t *testing.T) {
router := chi.NewRouter()
router.Route("/user/{username}", func(r chi.Router) {
- r.Use(mw.CSRF())
+ r.Use(mw.CSRF(h.HandleErrorPage(http.StatusForbidden)))
r.Use(mw.RequireAuth())
r.Get("/", func(w http.ResponseWriter, _ *http.Request) {
handlerReached = true
diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go
index 9147c1f..cae1649 100644
--- a/internal/handlers/source_management.go
+++ b/internal/handlers/source_management.go
@@ -149,13 +149,7 @@ func (h *Handlers) HandleSourceList() http.HandlerFunc {
"user_id = ?", userID,
).Order("created_at DESC").Find(&webhooks).Error
if err != nil {
- h.log.Error(
- "failed to list webhooks", "error", err,
- )
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.serverError(w, r, "failed to list webhooks", err)
return
}
@@ -249,9 +243,7 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
// middleware, which runs before CSRF parses the form.
err := r.ParseForm()
if err != nil {
- http.Error(
- w, "Bad request", http.StatusBadRequest,
- )
+ h.renderError(w, r, http.StatusBadRequest)
return
}
@@ -311,7 +303,7 @@ func (h *Handlers) createWebhookWithEntrypoint(
err := h.commitWebhook(webhook)
if err != nil {
- h.serverError(w, "failed to create webhook", err)
+ h.serverError(w, r, "failed to create webhook", err)
return
}
@@ -388,7 +380,7 @@ func (h *Handlers) HandleSourceDetail() http.HandlerFunc {
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -420,7 +412,7 @@ func (h *Handlers) renderSourceDetail(
if h.dbMgr.DBExists(webhook.ID) {
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
if err != nil {
- h.serverError(w, "failed to get webhook database", err)
+ h.serverError(w, r, "failed to get webhook database", err)
return
}
@@ -429,7 +421,7 @@ func (h *Handlers) renderSourceDetail(
webhookDB, webhook.ID, singleHTTPTargetID(targets),
)
if err != nil {
- h.serverError(w, "failed to load recent events", err)
+ h.serverError(w, r, "failed to load recent events", err)
return
}
@@ -482,7 +474,7 @@ func (h *Handlers) HandleSourceEdit() http.HandlerFunc {
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -517,7 +509,7 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -526,9 +518,7 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
// middleware, which runs before CSRF parses the form.
err = r.ParseForm()
if err != nil {
- http.Error(
- w, "Bad request", http.StatusBadRequest,
- )
+ h.renderError(w, r, http.StatusBadRequest)
return
}
@@ -582,7 +572,7 @@ func (h *Handlers) applyWebhookEdit(
err := h.db.DB().Save(webhook).Error
if err != nil {
- h.serverError(w, "failed to update webhook", err)
+ h.serverError(w, r, "failed to update webhook", err)
return
}
@@ -612,7 +602,7 @@ func (h *Handlers) HandleSourceDelete() http.HandlerFunc {
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -639,7 +629,7 @@ func (h *Handlers) deleteWebhookResources(
// be removed by hand; deleted history cannot be recovered.
err := h.commitWebhookDeletion(&webhook)
if err != nil {
- h.serverError(w, "failed to delete webhook", err)
+ h.serverError(w, r, "failed to delete webhook", err)
return
}
@@ -665,7 +655,7 @@ func (h *Handlers) deleteWebhookResources(
// redirecting as though everything succeeded: the file
// needs removing by hand, and the logged error names it.
h.serverError(
- w, "failed to delete webhook event database", err,
+ w, r, "failed to delete webhook event database", err,
)
return
@@ -809,7 +799,7 @@ func (h *Handlers) ownedWebhook(
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return database.Webhook{}, false
}
@@ -831,7 +821,7 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
// Without the map every delivery renders through a
// zero redactor, so failing the page is the only
// safe answer.
- h.serverError(w, "failed to load targets", err)
+ h.serverError(w, r, "failed to load targets", err)
return
}
@@ -839,7 +829,7 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
page := h.parsePage(r)
evts, total, ok := h.loadEventsWithDeliveries(
- w, webhook, targets, page,
+ w, r, webhook, targets, page,
)
if !ok {
return
@@ -949,6 +939,7 @@ func (h *Handlers) parsePage(r *http.Request) int {
// caller must then render nothing further.
func (h *Handlers) loadEventsWithDeliveries(
w http.ResponseWriter,
+ r *http.Request,
webhook database.Webhook,
targetMap map[string]eventLogTarget,
page int,
@@ -962,7 +953,7 @@ func (h *Handlers) loadEventsWithDeliveries(
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
if err != nil {
h.serverError(
- w, "failed to get webhook database", err,
+ w, r, "failed to get webhook database", err,
)
return nil, 0, false
@@ -999,7 +990,7 @@ func (h *Handlers) loadEventsWithDeliveries(
)
if err != nil {
h.serverError(
- w, "failed to load delivery attempts", err,
+ w, r, "failed to load delivery attempts", err,
)
return nil, 0, false
@@ -1008,7 +999,7 @@ func (h *Handlers) loadEventsWithDeliveries(
resubmits, err := resubmitCounts(webhookDB, eventIDs)
if err != nil {
h.serverError(
- w, "failed to count event resubmissions", err,
+ w, r, "failed to count event resubmissions", err,
)
return nil, 0, false
@@ -1231,7 +1222,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -1240,9 +1231,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
// middleware, which runs before CSRF parses the form.
err = r.ParseForm()
if err != nil {
- http.Error(
- w, "Bad request", http.StatusBadRequest,
- )
+ h.renderError(w, r, http.StatusBadRequest)
return
}
@@ -1258,7 +1247,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
err = h.db.DB().Create(entrypoint).Error
if err != nil {
- h.serverError(w, "failed to create entrypoint", err)
+ h.serverError(w, r, "failed to create entrypoint", err)
return
}
@@ -1289,7 +1278,7 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -1298,9 +1287,7 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
// middleware, which runs before CSRF parses the form.
err = r.ParseForm()
if err != nil {
- http.Error(
- w, "Bad request", http.StatusBadRequest,
- )
+ h.renderError(w, r, http.StatusBadRequest)
return
}
@@ -1371,7 +1358,7 @@ func (h *Handlers) processTargetCreate(
err = h.db.DB().Create(target).Error
if err != nil {
- h.serverError(w, "failed to create target", err)
+ h.serverError(w, r, "failed to create target", err)
return
}
@@ -1465,7 +1452,7 @@ func (h *Handlers) buildTargetConfig(
case database.TargetTypeSlack:
return h.buildSlackTargetConfig(w, r, in.URL)
case database.TargetTypeDatabase:
- return h.buildDatabaseTargetConfig(w, in.Expiry)
+ return h.buildDatabaseTargetConfig(w, r, in.Expiry)
case database.TargetTypeLog:
return "", nil
default:
@@ -1515,7 +1502,7 @@ func (h *Handlers) buildHTTPTargetConfig(
return "", err
}
- return marshalTargetConfig(w, delivery.HTTPTargetConfig{
+ return h.marshalTargetConfig(w, r, delivery.HTTPTargetConfig{
URL: in.URL,
Headers: headers,
Timeout: timeout,
@@ -1537,7 +1524,7 @@ func (h *Handlers) buildSlackTargetConfig(
return "", err
}
- return marshalTargetConfig(w, delivery.SlackTargetConfig{
+ return h.marshalTargetConfig(w, r, delivery.SlackTargetConfig{
WebhookURL: targetURL,
})
}
@@ -1591,16 +1578,14 @@ func (h *Handlers) validateTargetURL(
// marshalTargetConfig serialises a target configuration for storage,
// writing a 500 itself if it cannot.
-func marshalTargetConfig(
+func (h *Handlers) marshalTargetConfig(
w http.ResponseWriter,
+ r *http.Request,
cfg any,
) (string, error) {
configBytes, err := json.Marshal(cfg)
if err != nil {
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.serverError(w, r, "failed to encode target config", err)
return "", err
}
@@ -1616,6 +1601,7 @@ func marshalTargetConfig(
// expiry yields an empty config (the keep-forever default).
func (h *Handlers) buildDatabaseTargetConfig(
w http.ResponseWriter,
+ r *http.Request,
expiry string,
) (string, error) {
expiry = strings.TrimSpace(expiry)
@@ -1634,8 +1620,8 @@ func (h *Handlers) buildDatabaseTargetConfig(
return "", err
}
- return marshalTargetConfig(
- w, map[string]any{"expiry": expiry},
+ return h.marshalTargetConfig(
+ w, r, map[string]any{"expiry": expiry},
)
}
@@ -1689,7 +1675,7 @@ func (h *Handlers) deleteChildResource(
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
@@ -1699,11 +1685,7 @@ func (h *Handlers) deleteChildResource(
childID, webhook.ID,
).Delete(model)
if result.Error != nil {
- h.log.Error(errMsg, "error", result.Error)
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.serverError(w, r, errMsg, result.Error)
return
}
@@ -1793,18 +1775,14 @@ func (h *Handlers) toggleChildResource(
"id = ? AND user_id = ?", sourceID, userID,
).First(&webhook).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return
}
err = toggleFn(webhook.ID, childID)
if err != nil {
- h.log.Error(errMsg, "error", err)
- http.Error(
- w, "Internal server error",
- http.StatusInternalServerError,
- )
+ h.serverError(w, r, errMsg, err)
return
}
diff --git a/internal/handlers/target_edit.go b/internal/handlers/target_edit.go
index 8ff03b5..31e0eaa 100644
--- a/internal/handlers/target_edit.go
+++ b/internal/handlers/target_edit.go
@@ -88,9 +88,7 @@ func (h *Handlers) HandleTargetEditSubmit() http.HandlerFunc {
// middleware, which runs before CSRF parses the form.
err := r.ParseForm()
if err != nil {
- http.Error(
- w, "Bad request", http.StatusBadRequest,
- )
+ h.renderError(w, r, http.StatusBadRequest)
return
}
@@ -157,7 +155,7 @@ func (h *Handlers) applyTargetEdit(
err = h.db.DB().Save(target).Error
if err != nil {
- h.serverError(w, "failed to update target", err)
+ h.serverError(w, r, "failed to update target", err)
return
}
@@ -220,7 +218,7 @@ func (h *Handlers) ownedTarget(
chi.URLParam(r, "targetID"), webhook.ID,
).First(&target).Error
if err != nil {
- http.NotFound(w, r)
+ h.renderError(w, r, http.StatusNotFound)
return database.Webhook{}, nil, false
}
diff --git a/internal/handlers/webhook.go b/internal/handlers/webhook.go
index 1889ec5..a65ab05 100644
--- a/internal/handlers/webhook.go
+++ b/internal/handlers/webhook.go
@@ -88,14 +88,14 @@ func (h *Handlers) processWebhookRequest(
headersJSON, err := json.Marshal(r.Header)
if err != nil {
- h.serverError(w, "failed to serialize headers", err)
+ h.receiverError(w, "failed to serialize headers", err)
return
}
targets, err := h.loadActiveTargets(entrypoint.WebhookID)
if err != nil {
- h.serverError(w, "failed to query targets", err)
+ h.receiverError(w, "failed to query targets", err)
return
}
@@ -196,7 +196,7 @@ func (h *Handlers) createAndDeliverEvent(
targets,
)
if err != nil {
- h.serverError(w, "failed to store webhook event", err)
+ h.receiverError(w, "failed to store webhook event", err)
return
}
@@ -204,6 +204,19 @@ func (h *Handlers) createAndDeliverEvent(
h.finishWebhookResponse(w, event, entrypoint, tasks)
}
+// receiverError logs an error and answers the sender with a plain-text
+// 500. The receiver's answers are for programs, so it never sends the
+// error page the web UI uses.
+func (h *Handlers) receiverError(
+ w http.ResponseWriter, msg string, err error,
+) {
+ h.log.Error(msg, "error", err)
+ http.Error(
+ w, "Internal server error",
+ http.StatusInternalServerError,
+ )
+}
+
// eventSource carries the fields a new event is built from. The
// receiver fills it from the live request; the resubmit handler fills
// it from a stored event. Both then go through createAndFanOut, so an
diff --git a/internal/middleware/csrf.go b/internal/middleware/csrf.go
index 03a27a4..9e74138 100644
--- a/internal/middleware/csrf.go
+++ b/internal/middleware/csrf.go
@@ -19,7 +19,7 @@ func CSRFToken(r *http.Request) string {
// key to sign a CSRF cookie and validates a masked token submitted via
// the "csrf_token" form field (or the "X-CSRF-Token" header) on
// POST/PUT/PATCH/DELETE requests. Requests with an invalid or missing
-// token receive a 403 Forbidden response.
+// token are logged and answered by forbidden, which must write the 403.
//
// The middleware detects the client-facing transport protocol
// per-request via reqtls.IsTLS, the single TLS predicate the session
@@ -36,7 +36,9 @@ func CSRFToken(r *http.Request) string {
// Two gorilla/csrf instances are maintained — one with Secure cookies
// (for TLS) and one without (for plaintext HTTP) — because the
// csrf.Secure option is set at creation time, not per-request.
-func (m *Middleware) CSRF() func(http.Handler) http.Handler {
+func (m *Middleware) CSRF(
+ forbidden http.Handler,
+) func(http.Handler) http.Handler {
csrfErrorHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// CSRF is registered ahead of RequireAuth on every route
// group that uses it, so this WARN is reachable by an
@@ -57,7 +59,7 @@ func (m *Middleware) CSRF() func(http.Handler) http.Handler {
"remote_addr", r.RemoteAddr,
"reason", csrf.FailureReason(r),
)
- http.Error(w, "Forbidden - invalid CSRF token", http.StatusForbidden)
+ forbidden.ServeHTTP(w, r)
})
key := m.session.GetKey()
diff --git a/internal/middleware/csrf_test.go b/internal/middleware/csrf_test.go
index f1660ed..6cf12f3 100644
--- a/internal/middleware/csrf_test.go
+++ b/internal/middleware/csrf_test.go
@@ -18,6 +18,12 @@ import (
// csrfCookieName is the gorilla/csrf cookie name.
const csrfCookieName = "_gorilla_csrf"
+// forbidden stands in for the error page the server hands CSRF to
+// answer a refused request with.
+func forbidden(w http.ResponseWriter, _ *http.Request) {
+ w.WriteHeader(http.StatusForbidden)
+}
+
// csrfGetToken performs a GET request through the CSRF middleware
// and returns the token and cookies.
func csrfGetToken(
@@ -98,7 +104,7 @@ func TestCSRF_GETSetsToken(t *testing.T) {
var gotToken string
- handler := m.CSRF()(http.HandlerFunc(
+ handler := m.CSRF(http.HandlerFunc(forbidden))(http.HandlerFunc(
func(_ http.ResponseWriter, r *http.Request) {
gotToken = middleware.CSRFToken(r)
},
@@ -120,7 +126,7 @@ func TestCSRF_POSTWithValidToken(t *testing.T) {
t.Parallel()
m, _ := testMiddleware(t, config.EnvironmentDev)
- csrfMW := m.CSRF()
+ csrfMW := m.CSRF(http.HandlerFunc(forbidden))
getReq := httptest.NewRequestWithContext(
context.Background(),
@@ -152,7 +158,7 @@ func csrfPOSTWithoutTokenTest(
t.Helper()
m, _ := testMiddleware(t, env)
- csrfMW := m.CSRF()
+ csrfMW := m.CSRF(http.HandlerFunc(forbidden))
// GET to establish the CSRF cookie
getHandler := csrfMW(http.HandlerFunc(
@@ -209,7 +215,7 @@ func TestCSRF_POSTWithInvalidToken(t *testing.T) {
t.Parallel()
m, _ := testMiddleware(t, config.EnvironmentDev)
- csrfMW := m.CSRF()
+ csrfMW := m.CSRF(http.HandlerFunc(forbidden))
// GET to establish the CSRF cookie
getHandler := csrfMW(http.HandlerFunc(
@@ -265,7 +271,7 @@ func TestCSRF_GETDoesNotValidate(t *testing.T) {
var called bool
- handler := m.CSRF()(http.HandlerFunc(
+ handler := m.CSRF(http.HandlerFunc(forbidden))(http.HandlerFunc(
func(_ http.ResponseWriter, _ *http.Request) {
called = true
},
@@ -328,7 +334,7 @@ func csrfTookStrictPath(
t.Helper()
m, _ := testMiddleware(t, env)
- csrfMW := m.CSRF()
+ csrfMW := m.CSRF(http.HandlerFunc(forbidden))
newReq := func(method string) *http.Request {
r := httptest.NewRequestWithContext(
@@ -477,7 +483,7 @@ func TestCSRF_ProdMode_PlaintextHTTP_POSTWithValidToken(
t.Parallel()
m, _ := testMiddleware(t, config.EnvironmentProd)
- csrfMW := m.CSRF()
+ csrfMW := m.CSRF(http.HandlerFunc(forbidden))
getReq := httptest.NewRequestWithContext(
context.Background(),
@@ -517,7 +523,7 @@ func TestCSRF_ProdMode_BehindProxy_POSTWithValidToken(
t.Parallel()
m, _ := testMiddleware(t, config.EnvironmentProd)
- csrfMW := m.CSRF()
+ csrfMW := m.CSRF(http.HandlerFunc(forbidden))
getReq := httptest.NewRequestWithContext(
context.Background(),
@@ -562,7 +568,7 @@ func TestCSRF_ProdMode_DirectTLS_POSTWithValidToken(
t.Parallel()
m, _ := testMiddleware(t, config.EnvironmentProd)
- csrfMW := m.CSRF()
+ csrfMW := m.CSRF(http.HandlerFunc(forbidden))
getReq := httptest.NewRequestWithContext(
context.Background(),
diff --git a/internal/middleware/logbound_test.go b/internal/middleware/logbound_test.go
index 649370c..2c850d9 100644
--- a/internal/middleware/logbound_test.go
+++ b/internal/middleware/logbound_test.go
@@ -260,7 +260,9 @@ func logSites() map[string]logSite {
) http.Handler {
t.Helper()
- return m.CSRF()(unreachable(t))
+ return m.CSRF(http.HandlerFunc(forbidden))(
+ unreachable(t),
+ )
},
send: postNoToken,
wantStatus: http.StatusForbidden,
diff --git a/internal/middleware/recoverer.go b/internal/middleware/recoverer.go
index 8dcb76e..780e192 100644
--- a/internal/middleware/recoverer.go
+++ b/internal/middleware/recoverer.go
@@ -109,7 +109,8 @@ func (w *recoverResponseWriter) Unwrap() http.ResponseWriter {
// Recoverer returns middleware that turns a handler panic into one
// structured ERROR record and a 500, rather than a dropped
-// connection.
+// connection. The 500 is page when page is not nil, and plain text
+// when it is nil or when page panics before writing anything.
//
// It replaces chi's middleware.Recoverer, which does neither on a
// current Go release. chi v1.5.5's pretty-printer scans the stack for
@@ -136,9 +137,13 @@ func (w *recoverResponseWriter) Unwrap() http.ResponseWriter {
//
// Unlike http.Error on its own, it deletes any Set-Cookie the handler
// set before panicking, because a request that failed must not hand
-// the client a credential; every other header is left to http.Error.
+// the client a credential. It touches no other header: when page
+// answers, every other header the handler set goes out with it, apart
+// from any page sets itself; otherwise they are left to http.Error.
// See https://git.eeqj.de/sneak/webhooker/issues/193.
-func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
+func (s *Middleware) Recoverer(
+ page http.Handler,
+) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(
w http.ResponseWriter,
@@ -171,6 +176,14 @@ func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
rw.Header().Del("Set-Cookie")
+ if page != nil {
+ s.servePage(rw, r, page)
+ }
+
+ if rw.committed {
+ return
+ }
+
http.Error(
rw,
http.StatusText(
@@ -185,6 +198,27 @@ func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
}
}
+// servePage answers with page. A panic in page itself is logged and
+// recovered here, so the Recoverer can still send its plain 500.
+func (s *Middleware) servePage(
+ w http.ResponseWriter,
+ r *http.Request,
+ page http.Handler,
+) {
+ defer func() {
+ rvr := recover()
+ if rvr != nil {
+ s.log.Error("error page panic",
+ "panic", logfield.Truncate(
+ fmt.Sprint(rvr), maxPanicValueBytes,
+ ),
+ )
+ }
+ }()
+
+ page.ServeHTTP(w, r)
+}
+
// logPanic writes the record. Every field it can grow is truncated to
// a fixed budget, so MaxPanicLogLineBytes holds.
//
diff --git a/internal/middleware/recoverer_test.go b/internal/middleware/recoverer_test.go
index fac5cc4..485d6e1 100644
--- a/internal/middleware/recoverer_test.go
+++ b/internal/middleware/recoverer_test.go
@@ -76,7 +76,7 @@ func newRecovererProbe(
// Logging outside so the recovered 500 is the status it records.
router.Use(chimw.RequestID)
router.Use(m.Logging())
- router.Use(m.Recoverer())
+ router.Use(m.Recoverer(nil))
router.Get("/probe", handler)
serverErrors := new(bytes.Buffer)
@@ -637,7 +637,7 @@ func TestRecovererKeepsResponseControllerWorking(t *testing.T) {
m, _ := capturingMiddleware(t)
- handler := m.Recoverer()(http.HandlerFunc(
+ handler := m.Recoverer(nil)(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte("chunk"))
@@ -672,3 +672,59 @@ func TestRecovererKeepsResponseControllerWorking(t *testing.T) {
assert.Equal(t, http.StatusOK, resp.StatusCode)
assert.Equal(t, "chunk", string(body))
}
+
+// TestRecovererAnswersWithThePage covers a recoverer given a page:
+// the panic is logged as before, and the 500 is that page.
+func TestRecovererAnswersWithThePage(t *testing.T) {
+ t.Parallel()
+
+ m, logs := capturingMiddleware(t)
+
+ page := http.HandlerFunc(
+ func(w http.ResponseWriter, _ *http.Request) {
+ w.WriteHeader(http.StatusInternalServerError)
+ _, _ = w.Write([]byte("the error page"))
+ },
+ )
+
+ w := httptest.NewRecorder()
+ m.Recoverer(page)(http.HandlerFunc(panicProbe)).ServeHTTP(
+ w, httptest.NewRequestWithContext(
+ t.Context(), http.MethodGet, "/", nil,
+ ),
+ )
+
+ assert.Equal(t, http.StatusInternalServerError, w.Code)
+ assert.Equal(t, "the error page", w.Body.String())
+ assert.Contains(t, logs.String(), `"msg":"handler panic"`)
+ assert.Contains(t, logs.String(), panicMarker)
+}
+
+// TestRecovererFallsBackWhenThePagePanics covers a page that panics
+// before writing anything: both panics are logged, and the client
+// still gets the plain 500.
+func TestRecovererFallsBackWhenThePagePanics(t *testing.T) {
+ t.Parallel()
+
+ m, logs := capturingMiddleware(t)
+
+ const pagePanic = "QQERRORPAGEPANICQQ"
+
+ page := http.HandlerFunc(
+ func(http.ResponseWriter, *http.Request) {
+ panic(pagePanic)
+ },
+ )
+
+ w := httptest.NewRecorder()
+ m.Recoverer(page)(http.HandlerFunc(panicProbe)).ServeHTTP(
+ w, httptest.NewRequestWithContext(
+ t.Context(), http.MethodGet, "/", nil,
+ ),
+ )
+
+ assert.Equal(t, http.StatusInternalServerError, w.Code)
+ assert.Equal(t, "Internal Server Error\n", w.Body.String())
+ assert.Contains(t, logs.String(), panicMarker)
+ assert.Contains(t, logs.String(), pagePanic)
+}
diff --git a/internal/server/error_page_test.go b/internal/server/error_page_test.go
new file mode 100644
index 0000000..69cac8d
--- /dev/null
+++ b/internal/server/error_page_test.go
@@ -0,0 +1,220 @@
+package server_test
+
+import (
+ "context"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strconv"
+ "testing"
+
+ "github.com/getsentry/sentry-go"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ "sneak.berlin/go/webhooker/internal/config"
+ "sneak.berlin/go/webhooker/internal/server"
+)
+
+// The link back the error page offers: to the webhook list for a
+// signed-in user, to sign-in for anyone else.
+const (
+ backToWebhooks = `` +
+ `Back to webhooks`
+ backToSignIn = `` +
+ `Sign in`
+)
+
+// assertErrorPage checks that w is the error page for status, in the
+// normal layout, offering link.
+func assertErrorPage(
+ t *testing.T,
+ w *httptest.ResponseRecorder,
+ status int,
+ link string,
+) {
+ t.Helper()
+
+ body := w.Body.String()
+
+ assert.Equal(t, status, w.Code)
+ assert.Equal(
+ t, "text/html; charset=utf-8", w.Header().Get("Content-Type"),
+ )
+ assert.Equal(t, "no-store", w.Header().Get("Cache-Control"))
+ assert.Contains(t, body, `