Gitlab::Git::KeepAround silently rescues CommandError, risking undetected loss of keep-around refs

Summary

Gitlab::Git::KeepAround#execute rescues Gitlab::Git::CommandError and only tracks the exception via Gitlab::ErrorTracking.track_exception. This means when a Gitaly call fails (e.g. timeout), the keep-around ref is not created and no error is surfaced to the caller. The MR diff or merge request is created successfully, but the commit SHAs are left unprotected from garbage collection.

https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/git/keep_around.rb#L48-L49

rescue Gitlab::Git::CommandError => ex
  Gitlab::ErrorTracking.track_exception(ex, object_id: sha)

Problem

  1. CommandError (e.g. Gitaly timeout) is silently rescued — the caller has no idea the keep-around ref was not written
  2. If Git GC runs after this silent failure, the commit objects may be pruned
  3. This can lead to silent data loss — merge request diffs referencing commits that no longer exist
  4. The failure is only visible in error tracking (Sentry), not in application logs or user-facing errors
  5. As noted in the MR discussion, CommandError is only one of many Gitaly errors (see wraps_gitaly_errors.rb), but it is the one most likely to occur (e.g. timeouts) and the one being silently swallowed

Expected behavior

When keep_around_commits fails, the failure should either:

  • Fail loudly so the problem can be detected and addressed, or
  • Be logged with sufficient severity to trigger alerts

Proposal

Determine the appropriate error handling strategy for this code path. Options discussed in the related MR:

  1. Remove the rescue and let the error propagate — ensures failures are visible but may impact MR diff creation
  2. Add structured logging / alerting — keep the rescue but ensure failures are surfaced more visibly

Related

Edited by Marc Shaw