Commit 79e80b52 authored by Kassio Borges's avatar Kassio Borges

Expose failed import error through API

Gitlab already have a `projects/:id/import` to expose the import status
of the given project. This endpoint exposes "hard_failures", which are
errors that caused the import to fail.

This commit, changes the Gitlab::Import::ImportFailureService to create
the failure record with the `retry_count: 0`, when it's failing the
import. The `retry_count: 0` is is the condition to expose the failure
as a "hard_failure" in the API.

Related to: https://gitlab.com/gitlab-org/gitlab/-/issues/346652

MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75440
Changelog: changed
parent 568cab5c
......@@ -15,11 +15,21 @@ module Gitlab
exception: exception,
import_state: import_state,
project_id: project_id,
error_source: error_source
).execute(fail_import: fail_import, metrics: metrics)
error_source: error_source,
fail_import: fail_import,
metrics: metrics
).execute
end
def initialize(exception:, import_state: nil, project_id: nil, error_source: nil)
def initialize(
exception:,
import_state: nil,
project_id: nil,
error_source: nil,
fail_import: false,
metrics: false
)
if import_state.blank? && project_id.blank?
raise ArgumentError, 'import_state OR project_id must be provided'
end
......@@ -34,9 +44,11 @@ module Gitlab
@exception = exception
@error_source = error_source
@fail_import = fail_import
@metrics = metrics
end
def execute(fail_import:, metrics:)
def execute
track_exception
persist_failure
......@@ -46,7 +58,7 @@ module Gitlab
private
attr_reader :exception, :import_state, :project, :error_source
attr_reader :exception, :import_state, :project, :error_source, :fail_import, :metrics
def track_exception
attributes = {
......@@ -65,12 +77,15 @@ module Gitlab
Gitlab::ErrorTracking.track_exception(exception, attributes)
end
# Failures with `retry_count: 0` are considered "hard_failures" and those
# are exposed on the REST API projects/:id/import
def persist_failure
project.import_failures.create(
source: error_source,
exception_class: exception.class.to_s,
exception_message: exception.message.truncate(255),
correlation_id_value: Labkit::Correlation::CorrelationId.current_or_new_id
correlation_id_value: Labkit::Correlation::CorrelationId.current_or_new_id,
retry_count: fail_import ? 0 : nil
)
end
......
......@@ -7,58 +7,48 @@ RSpec.describe Gitlab::Import::ImportFailureService, :aggregate_failures do
let_it_be(:project) { create(:project, :import_started, import_type: import_type) }
let(:exception) { StandardError.new('some error') }
let(:arguments) { { project_id: project.id } }
let(:base_arguments) { { error_source: 'SomeImporter', exception: exception }.merge(arguments) }
let(:exe_arguments) { { fail_import: false, metrics: false } }
let(:import_state) { nil }
let(:fail_import) { false }
let(:metrics) { false }
let(:arguments) do
{
project_id: project.id,
error_source: 'SomeImporter',
exception: exception,
fail_import: fail_import,
metrics: metrics,
import_state: import_state
}
end
describe '.track' do
let(:instance) { double(:failure_service) }
context 'with all arguments provided' do
let(:instance) { double(:failure_service) }
let(:instance_arguments) do
let(:arguments) do
{
exception: exception,
import_state: '_import_state_',
project_id: '_project_id_',
error_source: '_error_source_'
}
end
let(:exe_arguments) do
{
error_source: '_error_source_',
fail_import: '_fail_import_',
metrics: '_metrics_'
}
end
it 'invokes a new instance and executes' do
expect(described_class).to receive(:new).with(**instance_arguments).and_return(instance)
expect(instance).to receive(:execute).with(**exe_arguments)
expect(described_class).to receive(:new).with(**arguments).and_return(instance)
expect(instance).to receive(:execute)
described_class.track(**instance_arguments.merge(exe_arguments))
described_class.track(**arguments)
end
end
context 'with only necessary arguments utilizing defaults' do
let(:instance) { double(:failure_service) }
let(:instance_arguments) do
{
exception: exception,
import_state: nil,
project_id: nil,
error_source: nil
}
end
let(:exe_arguments) do
{
fail_import: false,
metrics: false
}
end
it 'invokes a new instance and executes' do
expect(described_class).to receive(:new).with(**instance_arguments).and_return(instance)
expect(instance).to receive(:execute).with(**exe_arguments)
expect(described_class).to receive(:new).with(a_hash_including(exception: exception)).and_return(instance)
expect(instance).to receive(:execute)
described_class.track(exception: exception)
end
......@@ -66,7 +56,7 @@ RSpec.describe Gitlab::Import::ImportFailureService, :aggregate_failures do
end
describe '#execute' do
subject(:service) { described_class.new(**base_arguments) }
subject(:service) { described_class.new(**arguments) }
shared_examples 'logs the exception and fails the import' do
it 'when the failure does not abort the import' do
......@@ -89,13 +79,14 @@ RSpec.describe Gitlab::Import::ImportFailureService, :aggregate_failures do
source: 'SomeImporter'
)
service.execute(**exe_arguments)
service.execute
expect(project.import_state.reload.status).to eq('failed')
expect(project.import_failures).not_to be_empty
expect(project.import_failures.last.exception_class).to eq('StandardError')
expect(project.import_failures.last.exception_message).to eq('some error')
expect(project.import_failures.last.retry_count).to eq(0)
end
end
......@@ -120,32 +111,36 @@ RSpec.describe Gitlab::Import::ImportFailureService, :aggregate_failures do
source: 'SomeImporter'
)
service.execute(**exe_arguments)
service.execute
expect(project.import_state.reload.status).to eq('started')
expect(project.import_failures).not_to be_empty
expect(project.import_failures.last.exception_class).to eq('StandardError')
expect(project.import_failures.last.exception_message).to eq('some error')
expect(project.import_failures.last.retry_count).to eq(nil)
end
end
context 'when tracking metrics' do
let(:exe_arguments) { { fail_import: false, metrics: true } }
let(:metrics) { true }
it 'tracks the failed import' do
metrics = double(:metrics)
metrics_double = double(:metrics)
expect(Gitlab::Import::Metrics).to receive(:new).with("#{project.import_type}_importer", project).and_return(metrics)
expect(metrics).to receive(:track_failed_import)
expect(Gitlab::Import::Metrics)
.to receive(:new)
.with("#{project.import_type}_importer", project)
.and_return(metrics_double)
expect(metrics_double).to receive(:track_failed_import)
service.execute(**exe_arguments)
service.execute
end
end
context 'when using the project as reference' do
context 'when it fails the import' do
let(:exe_arguments) { { fail_import: true, metrics: false } }
let(:fail_import) { true }
it_behaves_like 'logs the exception and fails the import'
end
......@@ -156,10 +151,10 @@ RSpec.describe Gitlab::Import::ImportFailureService, :aggregate_failures do
end
context 'when using the import_state as reference' do
let(:arguments) { { import_state: project.import_state } }
let(:import_state) { project.import_state }
context 'when it fails the import' do
let(:exe_arguments) { { fail_import: true, metrics: false } }
let(:fail_import) { true }
it_behaves_like 'logs the exception and fails the import'
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