Handle Zoekt connection errors in language aggregation

What

Search::Zoekt::SearchResults#fetch_language_tally in ee/lib/search/zoekt/search_results.rb does not rescue anything. When Gitlab::Search::Zoekt::Client.search raises, the exception propagates all the way to the aggregations controller action and the request returns a 500 instead of a degraded response.

Why

Gitlab::Search::Zoekt::Client.search raises Search::Zoekt::Errors::ClientConnectionError on network errors (ee/lib/gitlab/search/zoekt/client.rb:128) and on an invalid JSON response body (ee/lib/gitlab/search/zoekt/client.rb:158).

That exception propagates: fetch_language_tally → language_aggregation_buckets → SearchResults#aggregations → SearchService#search_aggregations (app/services/search_service.rb:75) → the aggregations action in ee/app/controllers/ee/search_controller.rb:68. That action only rescues Gitlab::Search::Client::ConnectionError and Gitlab::Search::Client::AuthorizationError, which are Elasticsearch client error classes. Search::Zoekt::Errors::ClientConnectionError descends from Search::Zoekt::Errors::BaseError, itself a plain StandardError, and is unrelated to those, so nothing catches it.

The asymmetry that gives this away: zoekt_search, in the same file, already rescues this exact error class and degrades to empty results. So when a Zoekt node is unreachable or slow, search results render fine while the language sidebar's separate XHR call to /search/aggregations returns 500.

The sidebar is behind the default-off zoekt_language_aggregations feature flag, so today this only affects environments where that flag is enabled.

Found while reviewing !248990 (merged). It is pre-existing on master, not a regression from that MR.

How

Rescue Search::Zoekt::Errors::ClientConnectionError in fetch_language_tally, report it via Gitlab::ErrorTracking.track_exception, and return an empty hash. An empty tally produces empty buckets, and language_filter/index.vue wraps the whole section in v-if="hasBuckets", so the Language section disappears. The Archived and Forks filters are siblings that do not read aggregation data, so they are unaffected — the same outcome as a cache miss.

Left alone: the bare RuntimeError ("Node can not be found") that Gitlab::Search::Zoekt::Client also raises (client.rb:69). zoekt_search does not rescue it either, so keeping the two paths symmetric seemed better than broadening the rescue here alone.

Testing

Added three examples to ee/spec/lib/search/zoekt/search_results_spec.rb:

  • the aggregation returns [] instead of propagating the exception
  • the exception is reported via Gitlab::ErrorTracking.track_exception
  • nothing is written to the aggregation cache

All three were confirmed to fail when the rescue is removed.

Edited by Ravi Kumar

Merge request reports

Loading
Loading