fix: report a failed firehose stream to the caller of the handler

[Peeja]

[From Claude:]

One line in firehoseHandler, plus tests. Independent of #18 — that one is client-side, this is the server handler.

The problem

firehoseHandler writes an error event and then return nil (app.go:321), so a stream that died on a store failure looks to echo exactly like one that reached the end of the data.

There is no logging or metrics machinery in the repo today to notice, so nothing is broken right now. The reason to change it now is that adding that machinery later would not pick these up. Echo's request logger derives what it reports from exactly two things (request_logger.go:353 onward, v4.15.0):

err := next(c)
...
v.Status = res.Status
...
if config.LogError && err != nil { v.Error = err }

This handler defeats both. It returns nil, and response.WriteHeader(http.StatusOK) (app.go:313) commits the status before the first record, so the status is fixed before anything can fail.

I measured what standard middleware sees today, with RequestLoggerWithConfig{LogStatus, LogError, HandleError} in front of the real handler and a store that fails mid-stream:

HTTP status on the wire : 200
request logger status   : 200
request logger error    : <nil>
response body           : "event: error\ndata: {\"error\":\"getting revocations: connection refused\"}\n\n"

The database is down, the consumer is told, and the request logs as a clean 200. Same two inputs feed RED metrics, so it would count as a success. The signal is erased in the handler, below the layer that would go looking for it — so this is not something the observability work can fix on its own later.

The change

Return the error instead of nil. Same setup, after:

request logger status   : 200
request logger error    : streaming revocations: getting revocations: connection refused
body still intact       : true

It costs nothing today. With no middleware registered, echo routes the error to DefaultHTTPErrorHandler, which returns immediately on a committed response (echo.go:428) — nothing tries to rewrite the response, the client still gets its error event, the body is untouched. What changes is that the failure is there to be found when something goes looking.

A canceled request is the consumer hanging up, not a failure, and still returns nil.

What this does not fix

The status stays 200 even with the error returned, because it genuinely already went out on the wire. Log-based alerting can catch this; status-code metrics cannot, and would need a counter emitted from the handler itself. Worth knowing when the dashboards get built — "2xx rate is fine" will not tell you the firehose is failing.

Tests

Both go through request-logging middleware rather than asserting on the handler's return value, since being visible to that machinery is the property the change exists for:

test	with the change	with return nil
TestFirehoseRouteReportsAStoreFailure	PASS	FAIL — "a store failure was invisible to the request logger"
TestFirehoseRouteDoesNotReportACanceledRequest	PASS	PASS (unchanged behaviour)

The failure test also asserts the status is 200, documenting that the status cannot carry this, and that event: error still reaches the client.

Clean: go build ./..., GOWORK=off go vet ./..., gofmt -l ., go mod tidy (no diff), GOWORK=off go test ./....

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01AW65SG2vbQZ8X4wyzLkmVK

---

Generated by Claude Code