Return 503 when no Zoekt node is available

🤖 AI-authored change.

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::NodeUnavailableError replaces the three raises that mean the backend is genuinely unavailable: an empty searchable set in Gitlab::Search::Zoekt::Client#load_balancer (an ArgumentError), no node picked in #with_load_balanced_node (a bare RuntimeError), and a ready replica with no online node in NodeSelector#validate_result!.
  • Both rescues widen from ClientConnectionError to BaseError, in SearchController#aggregations and SearchResults.
  • The 503 message carries ids only. GraphQL blobSearch echoes SearchResults#error, so validate_result! names ids and logs the EnabledNamespace row rather than inspecting it into the message.
  • The frontend treats it as an outage. ZOEKT_BACKEND_ERROR_IDENTIFIERS lists both identifiers, so the results error component shows the connection copy, not the query-syntax help link.
  • The other two NodeSelector ArgumentError sites — 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.md in 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

  1. bundle exec rspec ee/spec/lib/gitlab/search/zoekt ee/spec/lib/search/zoekt
  2. yarn jest spec/frontend/search/results/components/result_error_spec.js

🤖 Automated change — mention me to get a response. Feedback welcome. Ask to speak to John directly anytime.

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/master 688aef73a8a8 against the first two commits.
  • The shard failure is asserted unrelated from its spec path and its passing retry, not from a bisect of it.
Edited by John Mason

Merge request reports

Loading
Loading