Return 503 when no Zoekt node is available
What does this MR do and why?
Search degrades to 503 when no Zoekt node is available, instead of 500. A reachable-but-failing node already yields 503 after !254462 (merged). The no-reachable-node case raises a different class before any HTTP call, escapes both rescues, and still 500s — reaching Sentry as an application bug, and the UI as a bad query.
- A new
Search::Zoekt::Errors::NodeUnavailableErrorreplaces the three raises that mean the backend is genuinely unavailable: an empty searchable set inGitlab::Search::Zoekt::Client#load_balancer(anArgumentError), no node picked in#with_load_balanced_node(a bareRuntimeError), and a ready replica with no online node inNodeSelector#validate_result!. - Both rescues widen from
ClientConnectionErrortoBaseError, inSearchController#aggregationsandSearchResults. - The 503 message carries ids only. GraphQL
blobSearchechoesSearchResults#error, sovalidate_result!names ids and logs theEnabledNamespacerow rather than inspecting it into the message. - The frontend treats it as an outage.
ZOEKT_BACKEND_ERROR_IDENTIFIERSlists both identifiers, so the results error component shows the connection copy, not the query-syntax help link. - The other two
NodeSelectorArgumentErrorsites — no enabled namespace, no ready replica — stay as they are: they mean the namespace is misconfigured, not that the backend is down.
Reviewer focus: is ids-only the right redaction boundary for a
client-visible 503 message, and is rescuing BaseError too wide for future
subclasses?
References
- Closes #628430
- Spec:
exact-code-search-routing-and-degradation/SPEC.mdin gitlab-org/search-team/artifacts (f8aa8349baf6) - Reachable-node half: !254462 (merged)
- Rejected wider variant: !251074 (closed)
Screenshots or screen recordings
No visual change — the new error type routes to the existing unavailable-backend empty state.
How to set up and validate locally
bundle exec rspec ee/spec/lib/gitlab/search/zoekt ee/spec/lib/search/zoektyarn jest spec/frontend/search/results/components/result_error_spec.js
Mechanism, citations and verification (for agents / deep readers)
Mechanism
The ArgumentError translation lives in Client#load_balancer rather than in
LoadBalancer itself, because ReplicaSelector is its other caller and relies
on the ArgumentError contract. #load_balancer also loads the searchable node
relation once, so the new emptiness guard does not re-check an unloaded relation
that LoadBalancer checks again and then iterates.
Resolvers::Search::Blob::BlobSearchResolver echoes SearchResults#error with
extensions: { error_type: ... }, so anything in the raised message is
client-visible. That is why NodeSelector#validate_result! includes
Gitlab::Loggable, writes the EnabledNamespace attributes through
Search::Zoekt::Logger with build_structured_payload_labkit, and raises a
message naming only ids. The controller path never echoes the message at all —
it renders a fixed string plus the demodulised error_type, which is what the
Vue component matches on.
Verification
Green on the merged-result pipeline
https://gitlab.com/gitlab-org/gitlab/-/pipelines/2854330220 at a7fc8254. Every
changed spec was confirmed to have executed, by grepping the traces of the ee
unit, ee integration and jest shards that ran it; rubocop reported no offenses.
The one shard failure was an unrelated Types::ProjectType example, which
passed on retry. Per-job breakdown and the example names observed:
$6056814
Each new example was shown able to fail before the last two commits: reverting
the behaviour files while keeping errors.rb — so a failure could not be a
missing constant — failed each one naming the exact wrong class, and reverting
the relation load alone fails the query-count example.
What I did not verify
- End-to-end behaviour against a live all-nodes-offline instance. The evidence here is unit, integration and jest level only.
- That the revert run above was repeated at the current head; it was run at
origin/master688aef73a8a8against the first two commits. - The shard failure is asserted unrelated from its spec path and its passing retry, not from a bisect of it.