From c3df27acb4e0f2ec43fb5891f2c24c3d8ce5507c Mon Sep 17 00:00:00 2001
From: Vasilii Iakliushin <viakliushin@gitlab.com>
Date: Thu, 17 Mar 2022 11:10:29 +0100
Subject: [PATCH] Don't trigger a sentry error for Gitaly exceptions
**Problem**
We add a Sentry error for cases when Gitaly is not available. We
already handle this exception, so Sentry tracking is not necessary.
We return a 200 response when the Gitaly is not available. It is
confusing and makes it difficult to track unsuccessful requests.
**Solution**
* Respond with 503 error
* Don't log exceptions in Sentry
Changelog: changed
---
app/controllers/projects/branches_controller.rb | 6 ++----
.../projects/branches_controller_spec.rb | 14 ++++++++------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index dad73c37fea5d..6264f10ce2d93 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -34,11 +34,9 @@ def index
Gitlab::GitalyClient.allow_n_plus_1_calls do
render
end
- rescue Gitlab::Git::CommandError => e
- Gitlab::ErrorTracking.track_exception(e)
-
+ rescue Gitlab::Git::CommandError
@gitaly_unavailable = true
- render
+ render status: :service_unavailable
end
format.json do
branches = BranchesFinder.new(@repository, params).execute
diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb
index ea22e6b6f1037..1580ad9361dee 100644
--- a/spec/controllers/projects/branches_controller_spec.rb
+++ b/spec/controllers/projects/branches_controller_spec.rb
@@ -688,21 +688,23 @@ def destroy_all_merged
end
context 'when gitaly is not available' do
+ let(:request) { get :index, format: :html, params: { namespace_id: project.namespace, project_id: project } }
+
before do
allow_next_instance_of(Gitlab::GitalyClient::RefService) do |ref_service|
allow(ref_service).to receive(:local_branches).and_raise(GRPC::DeadlineExceeded)
end
-
- get :index, format: :html, params: {
- namespace_id: project.namespace, project_id: project
- }
end
- it 'returns with a status 200' do
- expect(response).to have_gitlab_http_status(:ok)
+ it 'returns with a status 503' do
+ request
+
+ expect(response).to have_gitlab_http_status(:service_unavailable)
end
it 'sets gitaly_unavailable variable' do
+ request
+
expect(assigns[:gitaly_unavailable]).to be_truthy
end
end
--
GitLab