fix: map prometheus /metrics upstream timeouts to 504 instead of 502

[detail-app]

Detail bug report: View on Detail

Fixes ENG-715

Bug

On the httpz Prometheus scrape path (GET /metrics and GET /metrics/...), a stalled upstream that trips the 30-second upstream watchdog returned HTTP 502 Bad Gateway instead of the intended 504 Gateway Timeout.

Root cause: commit e75cfca (PR #263) registered execFetchFiltered with the watchdog via trackUpstream but — unlike its sibling exchange helper — never added the bufs.timed_out check at the phase boundaries. When the watchdog fires it shuts down the upstream socket (both directions), so the in-flight sendBodiless()/receiveHead()/streamReaderToWriter() fails with a generic transport error (HttpConnectionClosing, HttpRequestTruncated, ReadFailed). That propagates to handle, where errorStatus maps it via the else => 502 arm. errorStatus already maps error.UpstreamTimeout => 504, but this path never returned that error. The net effect: a timeout was indistinguishable from a broken/malformed upstream in both the status line and the RequestFailed/RequestCompleted debug telemetry.

Fix

src/frontend/httpz/server.zig, Handler.execFetchFiltered — add bufs.timed_out checks mirroring exchange at both phase boundaries, converting watchdog-induced transport errors to error.UpstreamTimeout (→ 504):

• Head phase (sendBodiless + receiveHead): restructured the block so the block produces an error union the outer catch can handle (Zig 0.16 try inside a labeled block propagates to the enclosing function, so the literal pattern from the report doesn't compile). The block-scoped errdefer eviction was moved explicitly into the catch to preserve the prior unconditional eviction (a break :blk err normal break does not fire errdefer), matching exchange's evict-then-check ordering.
• Body phase (streamReaderToWriter): catch and return error.UpstreamTimeout after evicting when timed_out is set; otherwise re-propagate the original error so non-timeout body errors keep their status (e.g. BodyTooLarge → 413).

The upstream send mechanics (sendBodiless() + receiveHead(&.{})) are byte-for-byte unchanged; only the error handling around them changed. errorStatus and exchange are untouched. The stdio frontend's parallel execFetchFiltered (src/frontend/stdio/conn.zig) has no watchdog wiring and is intentionally unchanged.

A unit test errorStatus maps upstream timeout to 504 and watchdog errors to 502 pins the status-code contract the bug hinged on: UpstreamTimeout → 504, the watchdog-induced transport errors (ReadFailed, HttpRequestTruncated) → 502, and the other arms (413/400/503) stay stable.

Testing

• Routine checks: zig build, zig build -Doptimize=ReleaseSafe, zig build test (520 pass / 1 pre-existing skip), zig fmt --check, ziglint (v0.5.2), and task do all green. The new unit test was confirmed to actually execute by temporarily flipping its assertion (the suite went 520 pass → 519 pass + 1 fail on the exact test), then restoring.
• End-to-end reproducer: built edge-prometheus (ReleaseSafe) and a Python mock upstream that accepts the connection and then stalls (never responds, never closes). Hit GET /metrics through the real proxy.
  • Before the fix (stashed the fix, rebuilt): returned HTTP 502 after ~30s, with request.failed err="HttpConnectionClosing" and request.completed status=502 — the reported bug.
  • After the fix: the same request returned HTTP 504, with request.failed err="UpstreamTimeout", an upstream.connection.evicted event, and request.completed status=504.
  • A partial-head variant (mock sends a few head bytes then stalls, exercising the HttpRequestTruncated path named in the report) also returned 504 after the fix.
• Regression checks on the same harness:
  • Happy path (ok mock) returns 200 with the policy-filtered body; no timeout/eviction events.
  • Non-timeout head failure (mock accepts then closes immediately — a stale keep-alive, not a watchdog) still returns 502 + eviction, fast (no 30s wait) — confirms the translation is strictly conditional on timed_out.
  • Non-timeout body error (max_input_bytes_per_scrape set small so the body exceeds the budget) returns 413, not 504 — confirms the body catch doesn't mistranslate non-timeout errors.
• Parity with the unchanged exchange path: built edge-datadog and stalled POST /api/v2/logs; it returned 504 with err="UpstreamTimeout" (no regression to the Datadog/OTLP intake paths).
• Stale-keepalive recovery: zig build upstream-pool-harness PASSes (PR fix: retry replayable log intake on stale upstream connections #263's eviction/replay behavior intact).
• Shutdown: SIGTERM during an in-flight stalled /metrics interrupted the I/O via the watchdog's force-expiry path, reported 504, and the server drained and stopped cleanly with no hang.
• Out-of-scope finding: a body-phase watchdog stall (valid head + Content-Length, few bytes, then stall) does not surface as 502/504 — it returns 200 with a truncated body, because the socket shutdown produces a clean EOF that pipeline.streamReaderToWriter treats as a successful end-of-stream. The body-phase catch is therefore defensive/parity-only and doesn't fire today. This is a separate latent issue (truncated upstream body reported as a successful 200), independent of the 502-vs-504 defect, and is left for a follow-up.

---

Automatic Fixes PRs can be configured here.

[macroscopeapp]

Approvability

Verdict: Approved at ca39069

Macroscope's review found this PR approvable — This is a focused one-file bug fix that maps existing Prometheus upstream watchdog timeouts to the already-defined 504 response while preserving non-timeout error handling. It introduces no schema, infrastructure, security, billing, or authentication changes and includes targeted status-mapping coverage.

^{You can add or adjust custom eligibility rules. Learn more.}