Return 408 for Elasticsearch timeouts on the search API

🤖 AI-authored change.

What does this MR do and why?

GET /api/v4/search and the group and project variants now return 408 Request timed out on an Elasticsearch timeout instead of 500, matching the web search page. No other status code changes.

A 500 counts against the error budget where a 408 does not, and the status a caller sees currently depends only on which surface the query arrived through.

References

Screenshots or screen recordings

Not applicable — no user interface change.

How to set up and validate locally

  1. Enable advanced search in a GDK with Elasticsearch running.
  2. In rails console, force the timeout:
    Elastic::Latest::ApplicationClassProxy.prepend(Module.new do
      def search(*) = raise(::Elastic::TimeoutError)
    end)
  3. curl -i --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/search?scope=issues&search=x" returns 408 with {"message":"Request timed out"}.

MR acceptance checklist

Evaluated against the MR acceptance checklist.

Mechanism, evidence and gaps

Why the two surfaces now agree. The web page rescues the same class at ee/app/controllers/ee/search_controller.rb#L46 and renders :request_timeout. The Grape API had no handler, so the exception fell through to rescue_from :all and became a 500.

Where the handler goes. On the EE::API::Search prepend, not in lib/api/api.rb: Elastic::TimeoutError is defined under ee/lib, so the CE file cannot name it.

Why the explicit report is needed. The handler reports the exception itself, because report_exception? reports only status-500 exceptions — without that line the change would narrow the status and silence the report. Of the two raise sites, only the transport reports upstream; elastic_client_setup.rb — the one behind the production timeouts this was filed for — does not. The web path also keeps reporting, via render_timeout → log_exception.

Verification in a GDK worktree at master@65ae591176f9: ee/spec/requests/api/search_spec.rb -e 'support for elasticsearch timeouts' — 3 examples, 0 failures, no infrastructure noise. rubocop on both changed files — no offenses.

The specs can fail, and they fail on the property under test. Dropping the track_exception call alone turns the three new examples red on the reporting assertion; dropping the whole rescue_from block turns them red on the status.

What I did not verify. I did not re-measure the production status split after deploy — that is only possible once this ships. I found no contract test or documented behaviour asserting 500 on this path, so I have not established that a caller depends on the current status; this narrows an error status and changes no success path.


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

Edited by John Mason

Merge request reports

Loading
Loading