Commit 84adfb20 authored by David Fernandez's avatar David Fernandez Committed by Lin Jen-Shin

Add error handling in Gitlab DeleteTagsService

Properly translate any error into a service error response
parent 8467c199
...@@ -24,9 +24,9 @@ module Projects ...@@ -24,9 +24,9 @@ module Projects
return success(deleted: []) if @tag_names.empty? return success(deleted: []) if @tag_names.empty?
delete_tags delete_tags
rescue TimeoutError => e rescue => e
::Gitlab::ErrorTracking.track_exception(e, tags_count: @tag_names&.size, container_repository_id: @container_repository&.id) ::Gitlab::ErrorTracking.track_exception(e, tags_count: @tag_names&.size, container_repository_id: @container_repository&.id)
error('timeout while deleting tags', nil, pass_back: { deleted: @deleted_tags }) error('error while deleting tags', nil, pass_back: { deleted: @deleted_tags, exception_class_name: e.class.name })
end end
private private
......
---
title: Add error handling in the container registry delete tags service
merge_request: 50763
author:
type: changed
...@@ -119,9 +119,9 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do ...@@ -119,9 +119,9 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
end end
end end
it { is_expected.to include(status: :error, message: 'timeout while deleting tags') } it { is_expected.to include(status: :error, message: 'error while deleting tags') }
it_behaves_like 'logging an error response', message: 'timeout while deleting tags', extra_log: { deleted_tags_count: 0 } it_behaves_like 'logging an error response', message: 'error while deleting tags', extra_log: { deleted_tags_count: 0 }
end end
end end
end end
......
...@@ -67,7 +67,7 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do ...@@ -67,7 +67,7 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
stub_delete_reference_requests('A' => 200) stub_delete_reference_requests('A' => 200)
end end
it { is_expected.to eq(status: :error, message: 'timeout while deleting tags', deleted: ['A']) } it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: ['A'], exception_class_name: Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError.name) }
it 'tracks the exception' do it 'tracks the exception' do
expect(::Gitlab::ErrorTracking) expect(::Gitlab::ErrorTracking)
...@@ -89,6 +89,21 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do ...@@ -89,6 +89,21 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
it_behaves_like 'deleting tags' it_behaves_like 'deleting tags'
end end
end end
context 'with a network error' do
before do
expect(service).to receive(:delete_tags).and_raise(::Faraday::TimeoutError)
end
it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: [], exception_class_name: ::Faraday::TimeoutError.name) }
it 'tracks the exception' do
expect(::Gitlab::ErrorTracking)
.to receive(:track_exception).with(::Faraday::TimeoutError, tags_count: tags.size, container_repository_id: repository.id)
subject
end
end
end end
end end
......
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