fix: guard lookupField against Null body in ensureBackendReachable#5310
fix: guard lookupField against Null body in ensureBackendReachable#5310blackheaven wants to merge 2 commits into
Conversation
When the federator returns a response with an empty/Null JSON body during backend warmup, lookupField res.json "inner" threw an uncaught assertion failure that aborted the retry loop (the surrounding E.try only catches HttpException). Wrap the call with catchAll so a Null body yields Nothing and the retry continues, instead of crashing testNotificationsForOfflineBackends.
zebot
added
the
ok-to-test
| -- but ingress is working, since we're processing the request. | ||
| let is200 = res.status == 200 | ||
| mInner <- lookupField res.json "inner" | ||
| mInner <- lookupField res.json "inner" `catchAll` const (pure Nothing) |
There was a problem hiding this comment.
I think this is not the problem, I see that sometimes the body contains metrics instead of the API version: https://grafana.zinfra.io/d/-jrKJOVIz/flake-news-failures?orgId=1&var-wire_server_version=5.33.30&var-test_suite_run=20260706170306721592739&var-test_suite=integration&var-test_case_name=wire-server.integration.Federation.testNotificationsForOfflineBackends&from=now-6h&to=now&timezone=browser
I think something in the integration tests machinery is not working right.
…rors The previous approach wrapped lookupField with catchAll to swallow the AssertionFailure thrown when the federator returns a non-object body during backend warmup (empty, JSON null, or Prometheus metrics text). That had two problems: 1. catchAll masks all exceptions, not just the AssertionFailure we care about. 2. The status check (res.status == 200) alone can false-positive when the metrics middleware returns HTTP 200, declaring the backend reachable before brig is actually serving api-version. Replace it with a body-shape guard: only consider the backend reachable when res.json is a proper JSON Object. For 200 responses, additionally require the VersionInfo "supported" field. The retry loop now genuinely waits for warmup to complete. See review feedback on #5310.
|
Good catch — you're right that it's not just a Null body. I looked into where the metrics text comes from: the federator (and every wire-server service) serves Prometheus metrics at Rather than catching the
This way The deeper question of why the federator occasionally routes the probe to a metrics-shaped response during warmup is still worth investigating separately, but this fix makes the poll robust regardless of the exact transient shape. |
3 participants
When the federator returns a response with an empty/Null JSON body during backend warmup, lookupField res.json "inner" threw an uncaught assertion failure that aborted the retry loop (the surrounding E.try only catches HttpException). Wrap the call with catchAll so a Null body yields Nothing and the retry continues, instead of crashing testNotificationsForOfflineBackends.
Checklist
changelog.d