Commit 2bd0b96c authored by Nick Thomas's avatar Nick Thomas

Correctly handle Gitaly being unavailable in more locations

If `GRPC::Unavailable` or `Gitlab::Git::CommandError` is raised in code
outside of a controller action, such as an `around_action`, it can be
executed after a render call inside the controller has completed. When
this happens, ActionController detects it and raises a very unhelpful
`AbstractController::DoubleRenderError`, swallowing the original error.

This code was introduced as part of this MR:
https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/114491

Circuit-breakers have since been removed, so it should be safe to
remove this code as well.
parent c46476c0
......@@ -103,14 +103,6 @@ class ApplicationController < ActionController::Base
head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window
end
rescue_from GRPC::Unavailable, Gitlab::Git::CommandError do |exception|
log_exception(exception)
headers['Retry-After'] = exception.retry_after if exception.respond_to?(:retry_after)
render_503
end
def redirect_back_or_default(default: root_path, options: {})
redirect_back(fallback_location: default, **options)
end
......@@ -246,19 +238,6 @@ class ApplicationController < ActionController::Base
head :unprocessable_entity
end
def render_503
respond_to do |format|
format.html do
render(
file: Rails.root.join("public", "503"),
layout: false,
status: :service_unavailable
)
end
format.any { head :service_unavailable }
end
end
def no_cache_headers
DEFAULT_GITLAB_NO_CACHE_HEADERS.each do |k, v|
headers[k] = v
......
---
title: Correctly handle Gitaly being unavailable in more locations
merge_request: 51222
author:
type: fixed
......@@ -211,21 +211,6 @@ RSpec.describe ProjectsController do
end
end
context 'when the storage is not available', :broken_storage do
let_it_be(:project) { create(:project, :broken_storage) }
before do
project.add_developer(user)
sign_in(user)
end
it 'renders a 503' do
get :show, params: { namespace_id: project.namespace, id: project }
expect(response).to have_gitlab_http_status(:service_unavailable)
end
end
context "project with empty repo" do
let_it_be(:empty_project) { create(:project_empty_repo, :public) }
......
......@@ -77,30 +77,6 @@ RSpec.shared_examples Repositories::GitHttpController do
end
end
end
context 'with exceptions' do
before do
allow(controller).to receive(:authenticate_user).and_return(true)
allow(controller).to receive(:verify_workhorse_api!).and_return(true)
end
it 'returns 503 with GRPC Unavailable' do
allow(controller).to receive(:access_check).and_raise(GRPC::Unavailable)
get :info_refs, params: params
expect(response).to have_gitlab_http_status(:service_unavailable)
end
it 'returns 503 with timeout error' do
allow(controller).to receive(:access_check).and_raise(Gitlab::GitAccess::TimeoutError)
get :info_refs, params: params
expect(response).to have_gitlab_http_status(:service_unavailable)
expect(response.body).to eq 'Gitlab::GitAccess::TimeoutError'
end
end
end
describe 'POST #git_upload_pack' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment