Commit ea376523 authored by Alexandru Croitor's avatar Alexandru Croitor

Gracefully handle exception in importing a specific Jira issue

Avoid failing to import an entire batch of issues when
importing one issue raises exception, instead catch the
exception and continue to import other issues in the batch
parent 8f56f4b6
---
title: Skip the individual JIRA issues if failed to import vs failing the whole batch
merge_request: 32673
author:
type: fixed
...@@ -57,17 +57,27 @@ module Gitlab ...@@ -57,17 +57,27 @@ module Gitlab
# For such cases we exit early if issue was already imported. # For such cases we exit early if issue was already imported.
next if already_imported?(jira_issue.id) next if already_imported?(jira_issue.id)
issue_attrs = IssueSerializer.new(project, jira_issue, running_import.user_id, { iid: next_iid }).execute begin
Gitlab::JiraImport::ImportIssueWorker.perform_async(project.id, jira_issue.id, issue_attrs, job_waiter.key) issue_attrs = IssueSerializer.new(project, jira_issue, running_import.user_id, { iid: next_iid }).execute
job_waiter.jobs_remaining += 1 Gitlab::JiraImport::ImportIssueWorker.perform_async(project.id, jira_issue.id, issue_attrs, job_waiter.key)
next_iid += 1
job_waiter.jobs_remaining += 1
# Mark the issue as imported immediately so we don't end up next_iid += 1
# importing it multiple times within same import.
# These ids are cleaned-up when import finishes. # Mark the issue as imported immediately so we don't end up
# see Gitlab::JiraImport::Stage::FinishImportWorker # importing it multiple times within same import.
mark_as_imported(jira_issue.id) # These ids are cleaned-up when import finishes.
# see Gitlab::JiraImport::Stage::FinishImportWorker
mark_as_imported(jira_issue.id)
rescue => ex
# handle exceptionn here and skip the failed to import issue, instead of
# failing to import the entire batch of issues
# track the failed to import issue.
Gitlab::ErrorTracking.track_exception(ex, project_id: project.id)
JiraImport.increment_issue_failures(project.id)
end
end end
job_waiter job_waiter
......
...@@ -42,12 +42,19 @@ describe Gitlab::JiraImport::IssuesImporter do ...@@ -42,12 +42,19 @@ describe Gitlab::JiraImport::IssuesImporter do
jira_issue = Struct.new(:id) jira_issue = Struct.new(:id)
let_it_be(:jira_issues) { [jira_issue.new(1), jira_issue.new(2)] } let_it_be(:jira_issues) { [jira_issue.new(1), jira_issue.new(2)] }
def mock_issue_serializer(count) def mock_issue_serializer(count, raise_exception_on_even_mocks: false)
serializer = instance_double(Gitlab::JiraImport::IssueSerializer, execute: { key: 'data' }) serializer = instance_double(Gitlab::JiraImport::IssueSerializer, execute: { key: 'data' })
next_iid = project.issues.maximum(:iid).to_i
count.times do |i| count.times do |i|
expect(Gitlab::JiraImport::IssueSerializer).to receive(:new) if raise_exception_on_even_mocks && i.even?
.with(project, jira_issues[i], current_user.id, { iid: i + 1 }).and_return(serializer) expect(Gitlab::JiraImport::IssueSerializer).to receive(:new)
.with(project, jira_issues[i], current_user.id, { iid: next_iid + 1 }).and_raise('Some error')
else
next_iid += 1
expect(Gitlab::JiraImport::IssueSerializer).to receive(:new)
.with(project, jira_issues[i], current_user.id, { iid: next_iid }).and_return(serializer)
end
end end
end end
...@@ -70,21 +77,22 @@ describe Gitlab::JiraImport::IssuesImporter do ...@@ -70,21 +77,22 @@ describe Gitlab::JiraImport::IssuesImporter do
end end
end end
context 'when there is more than one page of results' do context 'when importing some issue raises an exception' do
before do before do
stub_const("#{described_class.name}::BATCH_SIZE", 2) stub_const("#{described_class.name}::BATCH_SIZE", 3)
end end
it 'schedules 3 import jobs' do it 'schedules 2 import jobs' do
expect(subject).to receive(:fetch_issues).with(0).and_return([jira_issues[0], jira_issues[1]]) expect(subject).to receive(:fetch_issues).with(0).and_return([jira_issues[0], jira_issues[1]])
expect(Gitlab::JiraImport::ImportIssueWorker).to receive(:perform_async).twice.times expect(Gitlab::JiraImport::ImportIssueWorker).to receive(:perform_async).once
expect(Gitlab::Cache::Import::Caching).to receive(:set_add).twice.times.and_call_original expect(Gitlab::Cache::Import::Caching).to receive(:set_add).once.and_call_original
expect(Gitlab::Cache::Import::Caching).to receive(:set_includes?).twice.times.and_call_original expect(Gitlab::Cache::Import::Caching).to receive(:set_includes?).twice.and_call_original
mock_issue_serializer(2) expect(Gitlab::ErrorTracking).to receive(:track_exception).once
mock_issue_serializer(2, raise_exception_on_even_mocks: true)
job_waiter = subject.execute job_waiter = subject.execute
expect(job_waiter.jobs_remaining).to eq(2) expect(job_waiter.jobs_remaining).to eq(1)
expect(Gitlab::JiraImport.get_issues_next_start_at(project.id)).to eq(2) expect(Gitlab::JiraImport.get_issues_next_start_at(project.id)).to eq(2)
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