Return 503 when Gitaly is unavailable in Files API

Kerri Millerrequested to merge
629246-gitaly-transport-errors-return-503 into master

What does this MR do and why?

Fixes the Files API returning misleading 404 Commit Not Found for transient Gitaly transport errors (GRPC::Unavailable, GRPC::Internal, GRPC::Unknown, GRPC::DeadlineExceeded). These errors are now surfaced as retryable 503 Service Unavailable, while genuinely missing refs still return 404.

Closes #629246 (closed)

Related follow-up: #629309

Problem

Gitlab::Git::Commit.find swallows all CommandError exceptions (which include transport errors like UNAVAILABLE, INTERNAL, UNKNOWN, and DEADLINE_EXCEEDED) and returns nil, making transient Gitaly failures indistinguishable from genuinely missing commits. The Files API then calls not_found!('Commit'), returning a misleading 404 with no logging or tracking.

This violates the principle of distinguishing transient infrastructure failures (503, retryable) from permanent resource-not-found errors (404, not retryable).

Solution

When user_project.commit(params[:ref]) returns nil (ambiguous: could be a real missing ref OR a swallowed transport error), probe Gitaly health using Gitlab::GitalyClient::HealthCheckService. If the probe fails, raise Gitlab::Git::CommandError explicitly, which a method-level rescue maps to service_unavailable! (503). If the probe succeeds, it's a genuine missing ref, so return not_found!('Commit') (404).

This approach mirrors the existing graceful-degradation pattern used in CompareController#verify_gitaly_availability! and lib/extracts_path.rb.

Why not narrow the rescue in Commit.find?

Commit.find has 349 callers across the codebase. Letting CommandError propagate would break the nil-means-not-found contract for all of them, causing 500s in places that expect nil. The caller-side fix is lower risk and addresses the root cause for the specific endpoint without a large blast radius.

Scope

This fix applies to all 5 GET/HEAD Files API endpoints via the shared assign_file_vars! helper:

  • HEAD /repository/files/:file_path/blame
  • GET /repository/files/:file_path/blame
  • GET /repository/files/:file_path/raw
  • HEAD /repository/files/:file_path
  • GET /repository/files/:file_path

POST/PUT/DELETE endpoints go through service objects and were out of scope for this issue.

How to set up and validate locally

Prerequisites

  • Local GDK running
  • A project with at least one file in the repository

Test 1: Verify 503 when Gitaly is unavailable

  1. Stop Gitaly: gdk stop gitaly
  2. Call the Files API: 
    curl -H "PRIVATE-TOKEN: <your_token>" \
  "http://gdk.test:3000/api/v4/projects/<project_id>/repository/files/README.md?ref=main"
  3. Expected: 503 Service Unavailable
  4. Restart Gitaly: gdk start gitaly

Test 2: Verify 404 for genuinely missing ref

  1. Call the Files API with an invalid ref: 
    curl -H "PRIVATE-TOKEN: <your_token>" \
  "http://gdk.test:3000/api/v4/projects/<project_id>/repository/files/README.md?ref=nonexistent-branch-12345"
  2. Expected: 404 Not Found with message "404 Commit Not Found"

Test 3: Verify 200 for valid ref (no regression)

  1. Call the Files API with a valid ref: 
    curl -H "PRIVATE-TOKEN: <your_token>" \
  "http://gdk.test:3000/api/v4/projects/<project_id>/repository/files/README.md?ref=main"
  2. Expected: 200 OK with file content

Test 4: Run the new specs

bundle exec rspec spec/requests/api/files_spec.rb

Expected: GRPC::Unavailable to 503, GRPC::DeadlineExceeded to 503, missing ref with healthy Gitaly to 404.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Not applicable (API-only change)

Related to

  • Related follow-up: #629309 (audit other repository.commit() callers for the same issue)

Merge request reports