Commit 85ac2990 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Remove error swallowing from pages config update

Updating the pages config would swallow all errors before. When we
moved this to Sidekiq, we re-raised the error from the worker, so we
could make use of the Sidekiq retries to try and recover from the
failed update.

Since we've moved this to Sidekiq, and are not performing config
updates if pages aren't deployed. We can now get rid of this weird
error handling that would cause the problem being reported to Sentry
twice.
parent 8becb68e
...@@ -22,9 +22,6 @@ module Projects ...@@ -22,9 +22,6 @@ module Projects
end end
success success
rescue => e
Gitlab::ErrorTracking.track_exception(e)
error(e.message, pass_back: { exception: e })
end end
private private
......
...@@ -10,14 +10,6 @@ class PagesUpdateConfigurationWorker ...@@ -10,14 +10,6 @@ class PagesUpdateConfigurationWorker
project = Project.find_by_id(project_id) project = Project.find_by_id(project_id)
return unless project return unless project
result = Projects::UpdatePagesConfigurationService.new(project).execute Projects::UpdatePagesConfigurationService.new(project).execute
# The ConfigurationService swallows all exceptions and wraps them in a status
# we need to keep this while the feature flag still allows running this
# service within a request.
# But we might as well take advantage of sidekiq retries here.
# We should let the service raise after we remove the feature flag
# https://gitlab.com/gitlab-org/gitlab/-/issues/230695
raise result[:exception] if result[:exception]
end end
end end
...@@ -48,15 +48,6 @@ RSpec.describe Projects::UpdatePagesConfigurationService do ...@@ -48,15 +48,6 @@ RSpec.describe Projects::UpdatePagesConfigurationService do
expect(subject).to include(status: :success) expect(subject).to include(status: :success)
end end
end end
context 'when an error occurs' do
it 'returns an error object' do
e = StandardError.new("Failure")
allow(service).to receive(:reload_daemon).and_raise(e)
expect(subject).to eq(status: :error, message: "Failure", exception: e)
end
end
end end
context 'when pages are not deployed' do context 'when pages are not deployed' do
......
...@@ -17,14 +17,6 @@ RSpec.describe PagesUpdateConfigurationWorker do ...@@ -17,14 +17,6 @@ RSpec.describe PagesUpdateConfigurationWorker do
subject.perform(project.id) subject.perform(project.id)
end end
it "raises an exception if the service returned an error" do
allow_next_instance_of(Projects::UpdatePagesConfigurationService) do |service|
allow(service).to receive(:execute).and_return({ exception: ":boom:" })
end
expect { subject.perform(project.id) }.to raise_error(":boom:")
end
it_behaves_like "an idempotent worker" do it_behaves_like "an idempotent worker" do
let(:job_args) { [project.id] } let(:job_args) { [project.id] }
let(:pages_dir) { Dir.mktmpdir } let(:pages_dir) { Dir.mktmpdir }
......
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