fix(errors): emit JSON envelope from WriteHTTPError + harden fetch callers (#114)

* fix(errors): emit JSON envelope from WriteHTTPError + harden fetch callers (#112)

errors.WriteHTTPError historically wrote `text/plain` via http.Error, so
any API handler that returned an error through it (Logs, Services,
Certificates, ...) sent a body like "Failed to fetch logs. Please try
again later." Frontend JS that unconditionally `response.json()`s the
body then threw `Unexpected token 'F', "Failed to "... is not valid
JSON`, which the Logs and Services pages rendered verbatim as a hostile
error toast.

Change WriteHTTPError to emit the same `{"success": false, "message":
"..."}` envelope that Handler.jsonError() produces, with
`Content-Type: application/json`. Frontend callers that JSON.parse the
body now get a structured error they can surface cleanly.

Also harden the two visibly-affected pages (Logs, Services) to:
  - check response.ok before parsing
  - prefer the JSON envelope's .message field for the user-facing toast
  - fall back to response.statusText for non-JSON / malformed bodies

Add regression tests for the new wire format in
internal/framework/errors.

Phase 1 slice of #112.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(errors): address Copilot review on WriteHTTPError JSON envelope (#112)

Copilot flagged three callers that would render the new
{"success":false,"message":...} envelope opaquely:

  1. services.templ service-control: did `await response.text()` and
     toasted the raw blob.
  2. certificates.templ downloadCertificate: same.
  3. security.templ blockIP / unblockIP: same.

Also flagged the 2xx-non-JSON fallback I added in logs.templ and
services.templ as too permissive — silently rendering a non-JSON 2xx
response would hide an auth-redirect HTML page or proxy error.

Fixes:

  * security.templ now defines an `extractErrorMessage(response)`
    helper that parses the JSON envelope when content-type advertises
    JSON, falling back to text otherwise. blockIP / unblockIP route
    through it.
  * services.templ service-control inlines the same JSON-first
    pattern.
  * certificates.templ downloadCertificate inlines the same pattern.
  * logs.templ + services.templ success-path: a 2xx with a non-JSON
    body now throws a "non-JSON response" error instead of being
    silently rendered as log lines / empty service list.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: sarg3nt

gearbox/internal/framework/errors/errors.go

@@ -26,6 +26,7 @@ package errors
 import (
 	"context"
 	"database/sql"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"log/slog"
@@ -168,6 +169,15 @@ func IgnoreNotFound(err error) error {
 }
 
 // WriteHTTPError writes an AppError to an HTTP response and logs it.
+//
+// The wire format is a JSON envelope: `{"success": false, "message": "..."}`
+// with `Content-Type: application/json`. This matches the shape produced by
+// Handler.jsonError() and lets the frontend `response.json()` parse error
+// responses just like success responses — historically this used
+// `http.Error` (plain text), which caused JS callers that unconditionally
+// `JSON.parse` the body to throw `Unexpected token 'F', "Failed to "...`
+// (see issue #112 for the user-visible symptom on the Logs and Services
+// pages).
 func WriteHTTPError(w http.ResponseWriter, logger *slog.Logger, err error) {
 	var appErr *AppError
 
@@ -194,8 +204,19 @@ func WriteHTTPError(w http.ResponseWriter, logger *slog.Logger, err error) {
 
 	logger.LogAttrs(context.Background(), slog.LevelError, "HTTP error", logAttrs...)
 
-	// Write sanitized error to client
-	http.Error(w, appErr.UserMessage, appErr.Code)
+	// Write sanitized error to client as a JSON envelope so JS callers can
+	// `response.json()` it without throwing on plain-text bodies.
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(appErr.Code)
+	if encErr := json.NewEncoder(w).Encode(map[string]any{
+		"success": false,
+		"message": appErr.UserMessage,
+	}); encErr != nil {
+		// Encoding can fail only if the writer is broken — the header is
+		// already on the wire so we can't recover the response, just log.
+		logger.LogAttrs(context.Background(), slog.LevelError, "encode error envelope failed",
+			slog.String("error", encErr.Error()))
+	}
 }
 
 // SanitizeError converts any error to a user-safe message.

gearbox/internal/framework/errors/errors_writehttp_test.go

@@ -0,0 +1,64 @@
+package errors
+
+import (
+	"encoding/json"
+	"io"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// TestWriteHTTPError_JSONEnvelope guards the wire format consumers of
+// WriteHTTPError rely on — Logs page, Services page, and any other JS that
+// calls `response.json()` on both success and error responses. Historically
+// WriteHTTPError emitted `text/plain` ("Failed to ...") which caused
+// `JSON.parse` to throw `Unexpected token 'F'` in the browser; the fix in
+// issue #112 standardizes on a JSON envelope.
+func TestWriteHTTPError_JSONEnvelope(t *testing.T) {
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	w := httptest.NewRecorder()
+
+	WriteHTTPError(w, logger, Internal("fetch logs", io.EOF))
+
+	if got := w.Header().Get("Content-Type"); !strings.Contains(got, "application/json") {
+		t.Fatalf("Content-Type = %q, want application/json", got)
+	}
+	if got := w.Code; got != http.StatusInternalServerError {
+		t.Fatalf("Status = %d, want 500", got)
+	}
+
+	var body map[string]any
+	if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
+		t.Fatalf("response body is not JSON: %v\nbody=%q", err, w.Body.String())
+	}
+	if body["success"] != false {
+		t.Errorf("body.success = %v, want false", body["success"])
+	}
+	msg, _ := body["message"].(string)
+	if !strings.Contains(msg, "fetch logs") {
+		t.Errorf("body.message = %q, want it to contain 'fetch logs'", msg)
+	}
+}
+
+// TestWriteHTTPError_UnknownErrorWraps verifies that a plain error (not an
+// AppError) still serializes as a JSON envelope with the generic
+// "process request" wrapping that Internal() produces.
+func TestWriteHTTPError_UnknownErrorWraps(t *testing.T) {
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	w := httptest.NewRecorder()
+
+	WriteHTTPError(w, logger, io.ErrUnexpectedEOF)
+
+	if got := w.Header().Get("Content-Type"); !strings.Contains(got, "application/json") {
+		t.Fatalf("Content-Type = %q, want application/json", got)
+	}
+	var body map[string]any
+	if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
+		t.Fatalf("response body is not JSON: %v\nbody=%q", err, w.Body.String())
+	}
+	if body["success"] != false {
+		t.Errorf("body.success = %v, want false", body["success"])
+	}
+}

gearbox/internal/framework/templates/pages/certificates.templ

@@ -470,8 +470,23 @@ templ Certificates(user *models.User, servers []models.BoxConfig, perms *models.
 				const response = await fetch(`/api/${serverID}/certificates/${encodeURIComponent(domain)}/download`);
 
 				if (!response.ok) {
-					const text = await response.text();
-					throw new Error(text || `HTTP ${response.status}`);
+					// errors.WriteHTTPError now returns a JSON envelope; older
+					// http.Error paths still return text/plain. Try JSON first
+					// so the toast surfaces the structured .message field
+					// rather than the raw '{"success":false,...}' blob.
+					const ct = response.headers.get('content-type') || '';
+					let msg;
+					if (ct.includes('application/json')) {
+						try {
+							const body = await response.json();
+							msg = (body && (body.message || body.error)) || JSON.stringify(body);
+						} catch (_) {
+							msg = await response.text();
+						}
+					} else {
+						msg = await response.text();
+					}
+					throw new Error(msg || `HTTP ${response.status}`);
 				}
 
 				// Get filename from Content-Disposition header or use default

gearbox/internal/framework/templates/pages/logs.templ

@@ -489,14 +489,44 @@ templ Logs(user *models.User, servers []models.BoxConfig) {
 			const url = `/api/${serverID}/logs/${logName}?lines=${lines}`;
 
 			fetch(url)
-				.then(response => response.json())
+				.then(async response => {
+					// Backend returns a JSON envelope for both success and
+					// error (see errors.WriteHTTPError) — try to parse it and
+					// surface .message on error instead of letting JSON.parse
+					// failures bubble up as confusing "Unexpected token" toasts.
+					// A 2xx response with a non-JSON body is a protocol error
+					// (e.g. an auth-redirect HTML page leaking through) and
+					// must surface as such, not be rendered as log lines.
+					const contentType = response.headers.get('content-type') || '';
+					const isJSON = contentType.includes('application/json');
+					if (!response.ok) {
+						let msg = response.statusText || `HTTP ${response.status}`;
+						if (isJSON) {
+							try {
+								const body = await response.json();
+								if (body && (body.message || body.error)) {
+									msg = body.message || body.error;
+								}
+							} catch (_) { /* fall through */ }
+						}
+						throw new Error(msg);
+					}
+					if (!isJSON) {
+						throw new Error(`Unexpected non-JSON response from logs API (content-type=${contentType})`);
+					}
+					return response.json();
+				})
 				.then(data => {
 					rawLogContent = data.logs || 'No logs available';
 					displayLogs(rawLogContent);
 				})
 				.catch(error => {
-					document.getElementById('log-container').innerHTML =
-						'<div class="text-red-400">Error loading logs: ' + error.message + '</div>';
+					const container = document.getElementById('log-container');
+					container.replaceChildren();
+					const div = document.createElement('div');
+					div.className = 'text-red-400';
+					div.textContent = 'Error loading logs: ' + error.message;
+					container.appendChild(div);
 				});
 		}
 

gearbox/internal/framework/templates/pages/security.templ

@@ -200,6 +200,24 @@ templ SecurityPage(user *models.User, servers []models.BoxConfig, perms *models.
 		let blockedIPsData = [];
 		const canAction = { if perms != nil && perms.HasPermission(models.ComponentSecurity, models.PermissionAction) { "true" } else { "false" } } === 'true';
 
+		// extractErrorMessage normalizes the body of a non-2xx fetch response
+		// into a user-facing string. errors.WriteHTTPError now returns a JSON
+		// envelope `{success:false, message:...}`; older `http.Error` paths
+		// still return text/plain. Try JSON first so the toast surfaces
+		// .message instead of the raw envelope, then fall back to text.
+		async function extractErrorMessage(response) {
+			const ct = response.headers.get('content-type') || '';
+			if (ct.includes('application/json')) {
+				try {
+					const body = await response.json();
+					return (body && (body.message || body.error)) || JSON.stringify(body);
+				} catch (_) {
+					return await response.text();
+				}
+			}
+			return await response.text();
+		}
+
 		document.addEventListener('DOMContentLoaded', function() {
 			const serverInput = document.getElementById('default-server-id');
 			if (serverInput) {
@@ -363,8 +381,7 @@ templ SecurityPage(user *models.User, servers []models.BoxConfig, perms *models.
 				});
 
 				if (!response.ok) {
-					const error = await response.text();
-					throw new Error(error);
+					throw new Error(await extractErrorMessage(response));
 				}
 
 				// Clear form and refresh
@@ -391,8 +408,7 @@ templ SecurityPage(user *models.User, servers []models.BoxConfig, perms *models.
 				});
 
 				if (!response.ok) {
-					const error = await response.text();
-					throw new Error(error);
+					throw new Error(await extractErrorMessage(response));
 				}
 
 				await loadBlockedIPs();

gearbox/internal/framework/templates/pages/services.templ

@@ -213,7 +213,33 @@ templ Services(user *models.User, servers []models.BoxConfig) {
 			]);
 
 			fetch('/api/' + serverID + '/services?services=' + monitoredServices.join(','))
-				.then(function(response) { return response.json(); })
+				.then(async function(response) {
+					// Backend returns a JSON envelope for both success and
+					// error (see errors.WriteHTTPError); surface .message on
+					// error so the UI doesn't show raw 'Unexpected token' from
+					// JSON.parse on plain-text bodies. A 2xx response with a
+					// non-JSON body (e.g. an auth-redirect HTML page leaking
+					// through) is a protocol error and must surface as such,
+					// not be silently swallowed as an empty list.
+					const contentType = response.headers.get('content-type') || '';
+					const isJSON = contentType.includes('application/json');
+					if (!response.ok) {
+						let msg = response.statusText || ('HTTP ' + response.status);
+						if (isJSON) {
+							try {
+								const body = await response.json();
+								if (body && (body.message || body.error)) {
+									msg = body.message || body.error;
+								}
+							} catch (_) { /* fall through */ }
+						}
+						throw new Error(msg);
+					}
+					if (!isJSON) {
+						throw new Error('Unexpected non-JSON response from services API (content-type=' + contentType + ')');
+					}
+					return response.json();
+				})
 				.then(function(data) {
 					if (data.error) {
 						container.innerHTML = '<div class="col-span-full text-red-500 dark:text-red-400 p-4 bg-red-50 dark:bg-red-900/20 rounded">' + escapeHtml(data.error) + '</div>';
@@ -500,8 +526,23 @@ templ Services(user *models.User, servers []models.BoxConfig) {
 				});
 
 				if (!response.ok) {
-					const text = await response.text();
-					throw new Error(text || 'Failed to ' + action + ' service');
+					// errors.WriteHTTPError now returns a JSON envelope; older
+					// http.Error paths still return text/plain. Try JSON first
+					// so the toast surfaces the structured .message field
+					// rather than the raw '{"success":false,...}' blob.
+					const ct = response.headers.get('content-type') || '';
+					let msg;
+					if (ct.includes('application/json')) {
+						try {
+							const body = await response.json();
+							msg = (body && (body.message || body.error)) || JSON.stringify(body);
+						} catch (_) {
+							msg = await response.text();
+						}
+					} else {
+						msg = await response.text();
+					}
+					throw new Error(msg || 'Failed to ' + action + ' service');
 				}
 
 				const data = await response.json();