Skip to content

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

Merged
sarg3nt merged 2 commits into
mainfrom
fix/issue-112-json-error-envelope
May 17, 2026
Merged

fix(errors): emit JSON envelope from WriteHTTPError + harden fetch callers#114
sarg3nt merged 2 commits into
mainfrom
fix/issue-112-json-error-envelope

Conversation

@sarg3nt

@sarg3nt sarg3nt commented May 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • errors.WriteHTTPError historically wrote text/plain via http.Error. Any API handler routing errors through it (Logs, Services, Certificates, …) sent bodies like Failed to fetch logs. Please try again later.. Frontend JS that unconditionally response.json()s the body threw Unexpected token 'F', "Failed to "... is not valid JSON, which the Logs/Services pages rendered as the hostile error toast users were seeing.
  • Change WriteHTTPError to emit the same {"success": false, "message": "..."} envelope Handler.jsonError produces, with Content-Type: application/json. Both helpers now share a wire format, and JS callers can parse error responses the same way they parse successes.
  • Harden the two visibly-affected pages (Logs, Services) so they check response.ok, prefer the JSON envelope's .message for the toast, and fall back to response.statusText for non-JSON bodies. (The catch handler in logs.templ now uses textContent / replaceChildren instead of building HTML, sidesteps the existing innerHTML pattern lint.)
  • Add a regression test in internal/framework/errors that asserts the new wire format.

Phase 1 slice of #112.

Test plan

  • go test -count=1 ./internal/framework/errors/... ./internal/framework/handler/... clean (new errors_writehttp_test.go + existing os_updates_error_test.go)
  • go vet ./... clean
  • go build ./... clean
  • On a live deployment, navigate to /logs and /services for a box where the agent can't serve those gears; confirm the error toast surfaces the agent's actual message (e.g. "Failed to fetch logs. Please try again later.") instead of Unexpected token 'F'.

🤖 Generated with Claude Code

…llers (#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>
Copilot AI review requested due to automatic review settings May 17, 2026 05:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR standardizes WriteHTTPError responses as JSON envelopes and updates Logs/Services frontend fetch handling to display backend error messages instead of JSON parse failures.

Changes:

  • WriteHTTPError now emits {"success": false, "message": "..."} with application/json.
  • Logs and Services page fetch paths now check response.ok and extract JSON error messages.
  • Added regression tests for the new WriteHTTPError JSON envelope.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
gearbox/internal/framework/errors/errors.go Changes shared HTTP error serialization to JSON.
gearbox/internal/framework/errors/errors_writehttp_test.go Adds tests for JSON error response shape.
gearbox/internal/framework/templates/pages/logs.templ Hardens log fetch error handling and safer DOM update on failure.
gearbox/internal/framework/templates/pages/services.templ Hardens initial services fetch error handling.

Comment thread gearbox/internal/framework/errors/errors.go
Comment thread gearbox/internal/framework/templates/pages/services.templ Outdated
Comment thread gearbox/internal/framework/templates/pages/logs.templ Outdated
…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>
sarg3nt added a commit that referenced this pull request May 17, 2026
* feat(logs): derive default log sources from agent capabilities (#112)

APILogSourcesHandler returned a hardcoded [haproxy, system] pair
whenever no explicit per-box settings existed. On a box whose agent
has no `logs` (journalctl) or `access-log` gear — e.g. mjolnir's
distroless container agent — the Logs page would still load both
defaults and then trip every fetch against the agent, which has
nothing to serve. Combined with the plain-text 5xx error responses
(see #112 / sibling PR #114) this produced the user-visible
"Error loading logs: Unexpected token 'F'" toast.

Drive the no-settings default off the agent's probe table:

  * `haproxy` is offered when either access-log OR logs gear is
    available (access-log streams directly from /var/log/haproxy; the
    logs gear can serve it via journalctl/file too).
  * `system` is offered only when the `logs` gear is available (it
    needs journalctl/tail).
  * If the agent has neither, return an empty source list — the JS
    renders an empty dropdown and skips the immediate fetch instead
    of 5xx-storming.
  * If capabilities aren't reachable at all (agent down, older agent
    that pre-dates probing) fall back to the legacy pair so existing
    deployments still work through transient outages.

Phase 2 slice of #112.

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

* fix(logs): address Copilot review on capability presence vs availability (#112)

Copilot pointed out that defaultLogSourcesForBox conflated two
distinct cases:
  1. The agent doesn't surface a gear name at all (older agent that
     pre-dates the gear).
  2. The agent reports the gear as unavailable.

Both used IsAvailable() and collapsed into "return empty list",
which violates the function comment's fail-open promise: a
forward-compatibility gap would silently strip the source list.

Use caps.Has() to distinguish presence from availability:
  - If the agent surfaces neither access-log nor logs by name → fail
    open to the legacy [haproxy, system] pair (older-agent path).
  - If the agent surfaces at least one and reports both unavailable →
    return empty (the only case where the JS shows an empty dropdown).

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sarg3nt
sarg3nt merged commit c4715a8 into main May 17, 2026
22 checks passed
@sarg3nt
sarg3nt deleted the fix/issue-112-json-error-envelope branch May 28, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants