Handle Zoekt connection errors in aggregations and blob search

What does this MR do and why?

Both commits fix the same root cause: a ::Search::Zoekt::Errors::ClientConnectionError escaping past the code that is supposed to turn it into a handled search failure. The first commit fixes this for the aggregations action; the second fixes a related gap on the blob search path itself.

1. Return a 503 from SearchController#aggregations

Adds a rescue clause for ::Search::Zoekt::Errors::ClientConnectionError in the aggregations action in ee/app/controllers/ee/search_controller.rb. On this error it calls ::Gitlab::ErrorTracking.log_exception, derives error_type from the class name the same way the existing rescue does (yielding client_connection_error), and renders { error:, error_type: } with status :service_unavailable.

Why:

  • The action already treats "search backend unreachable" as a 503 with a structured body, but the existing rescue only lists ::Gitlab::Search::Client::ConnectionError and ::Gitlab::Search::Client::AuthorizationError, which are Elasticsearch classes.
  • Search::Zoekt::Errors::ClientConnectionError descends from a plain StandardError with unrelated ancestry, so it escaped the action. Gitlab::Search::Zoekt::Client raises it on network errors and on an unparseable JSON body (ee/lib/gitlab/search/zoekt/client.rb, around lines 128 and 158).
  • Today, an unreachable or slow Zoekt node produces an unhandled exception reported to Sentry as a 500 from SearchController#aggregations, which reads like an application bug rather than a downstream dependency being down. One event per aggregations request, so it scales with search traffic during an outage.
  • The response body uses a fixed message, Search is currently unavailable. Please try again later., rather than e.message. The Zoekt error carries the raw transport failure including the node URL, which must not be echoed to the client. The Elasticsearch classes hardcode a safe message on the class itself (ee/lib/gitlab/search/client.rb, around lines 62-72); the Zoekt one does not, and giving it one would hide the real cause from logs on the search path.

Search::Zoekt::SearchResults#known_languages in ee/lib/search/zoekt/search_results.rb now rescues ::Search::Zoekt::Errors::ClientConnectionError and returns an empty array.

Why:

  • #zoekt_search already rescues ClientConnectionError for the results query and converts it into @error_message / @is_server_error, surfaced through failed? / server_error?. But #zoekt_search_and_wrap calls #known_languages to build the Search::Zoekt::Cache key, and that call happens before the block that #zoekt_search rescues.
  • With the Unknown language filter applied and a cold aggregation cache, #known_languages issues the language-tally request itself, so the error escapes past the rescue to the caller.
  • Who is exposed: SearchController#count (chain: formatted_count('blobs') then blobs_and_file_count then search_as_found_blob then zoekt_search_and_wrap), and the GraphQL blobSearch resolver (Resolvers::Search::Blob::BlobSearchResolver), which is what the blob results UI actually calls. Both expect a failed result object, not a raised exception.
  • Not exposed: SearchController#show, because multi_match? is true for scope blobs with search type zoekt, so haml_search_results is skipped and the action runs no Zoekt query. Also not exposed: SearchController#autocomplete, because search_autocomplete_opts never calls Gitlab::Search::Zoekt::Client.
  • Also not exposed: the REST Search API (lib/api/search.rb) reaches the same SearchResults#objects chain, so it has the same shape of exposure in principle, but it doesn't take a language parameter, so known_languages is never invoked and the tally is never attempted.
  • An empty array is the right fallback because it is the case CodeQueryBuilder#unknown_atom already handles: without a known-language list the complement cannot be expressed, so the Unknown filter is dropped rather than returning a page the user cannot recover from.
  • The fallback's empty array also flows into Search::Zoekt::Cache#search_fingerprint (ee/lib/search/zoekt/cache.rb:78-83), where known_languages.sort is folded into the SHA256 digest alongside the query, scope, search mode, chunk size, and filters. A degraded search therefore hashes to a different cache key than a healthy one, so it cannot serve its filter-dropped results out of the cache to a later request that successfully built the language complement. No cache poisoning.
  • This is deliberately not the approach rejected in the closed !251074 (closed). That one rescued inside #fetch_language_tally, which also swallowed the failure on the aggregations path and turned it into a 200 with empty buckets. This rescue sits in #known_languages, which is only on the search path. The aggregations action reaches the tally through #language_aggregation_buckets, not #known_languages, so it still returns 503 and the SLI still counts the failure.
  • Raised by a GitLab Duo review comment on this MR: !254462 (comment 3808884067)

What does not change:

  • No user-visible change for the aggregations fix. fetchAllAggregation in app/assets/javascripts/search/store/actions.js (around lines 289-297) rejects on any non-2xx response and commits RECEIVE_AGGREGATIONS_ERROR, and app/assets/javascripts/search/sidebar/components/language_filter/index.vue renders the "Aggregations load error." alert. That was already true for the 500 and stays true for the 503.
  • No new user-visible failure mode for the blob search fix either. Before it, an Unknown-filtered blob search with Zoekt down raised an unhandled exception; after it, the request returns the normal error result that #zoekt_search already produces for a downed backend, just without the Unknown filter applied.
  • No change to SLI accounting on either path. record_search_error in app/controllers/search_controller.rb (around lines 172-181) flags an error when the status is 500 or above, so the aggregations failure is still counted, and it is reached through #language_aggregation_buckets, not through #known_languages.
  • No changelog trailer for either commit: the language sidebar and its filters are behind the zoekt_language_aggregations feature flag (ee/config/feature_flags/gitlab_com_derisk/zoekt_language_aggregations.yml), which is default_enabled: false, and neither change is user-facing.

Interaction with MR 254635

MR !254635 (merged) moves the Gitlab::Metrics::GlobalSearchSlis.record_error_rate call in BlobSearchResolver#results out of the if @search_results.failed? branch into an ensure, so it records error: @search_results.nil? || @search_results.server_error? on every exit path. Combined with this MR, the GraphQL blob path with the Unknown language filter applied and a cold cache gets an outcome neither MR describes on its own:

Tally request Results request Recorded by 254635
fails fails error: true (unchanged in spirit)
fails succeeds error: false -- known_languages swallows the tally failure, results render with the Unknown filter silently dropped

The second row is defensible: the user did get results. It's called out here so the outcome is a decision on record rather than an emergent property of merge order between the two MRs. There is no file-level conflict; the two MRs touch different files.

References

  • Earlier, closed attempt at the first fix: !251074 (closed). It rescued inside Search::Zoekt::SearchResults#fetch_language_tally and returned an empty hash, turning the aggregations request into a 200 with empty buckets. That silently hid the Language section instead of showing the alert, and stopped the SLI from counting the failure. Closed in favour of the controller-level approach in this MR. The second commit here also touches search_results.rb but rescues in #known_languages instead, which does not affect the aggregations path; see the note under "Stop the language tally from failing the whole blob search" above.
  • GitLab Duo review comment that raised the second issue: !254462 (comment 3808884067)
  • Feature flag rollout issue: #605457 (closed)
  • Flag definition: ee/config/feature_flags/gitlab_com_derisk/zoekt_language_aggregations.yml

Screenshots or screen recordings

No UI change. The "Aggregations load error." alert already rendered when the aggregations request failed with a 500; it renders the same way now that the request fails with a 503. The blob search page already has an error state for a downed Zoekt backend; the second commit only makes the Unknown-filtered case reach that same state instead of raising. No screenshots apply.

How to set up and validate locally

  1. Enable the flag in the rails console: Feature.enable(:zoekt_language_aggregations)
  2. Make the Zoekt search endpoint unreachable, e.g. gdk stop gitlab-zoekt-webserver-development-1 gitlab-zoekt-webserver-development-2
  3. Run an exact-code (blob) search in a Zoekt-indexed namespace.
  4. Expect: results render, the sidebar shows "Aggregations load error.", and the request to /search/aggregations returns 503 with {"error":"Search is currently unavailable. Please try again later.","error_type":"client_connection_error"} instead of a 500.
  5. Specs for the first commit: bundle exec rspec ee/spec/controllers/ee/search_controller_spec.rb -e aggregations, 24 examples, 0 failures. Two new examples cover the 503 body and that record_error_rate is still called with error: true; both fail if the rescue clause is removed.
  6. With the flag still enabled and Zoekt still stopped, apply the Unknown language filter on a blob search. Before the second fix this raised an unhandled exception; after it, the blob search returns its normal error state and the Unknown filter is dropped instead.
  7. Specs for the second commit: bundle exec rspec ee/spec/lib/search/zoekt/search_results_spec.rb, 137 examples, 0 failures. The new example was verified to fail without the rescue, with backtrace fetch_language_tally then known_languages then zoekt_search_and_wrap then search_as_found_blob then blobs_and_file_count then objects.

MR acceptance checklist

Check this MR acceptance checklist before merge.

🤖 Generated with Claude Code

Edited by Ravi Kumar

Merge request reports

Loading
Loading