Github Importer: Ensure to fail and log imports on exceptions

Currently, the Github Importer might end up stuck even after failing to
import the git repository. This might also happen when importing other
types of objects from Github.

To provide better feedback to the end user this kind of exception will
now mark the import (import_state) as failed. Also, to provide
information to developers and testers the exceptions errors will be
logged (importer.log) and saved in the import_failures table.

== What was done

* Created Gitlab::Import::ImportFailureService: This way we now have a
  standard way to log exceptions/errors that happens while importing a
  project;
  * All the exceptions/errors are logged to the importer.log with a
    standard format;
  * All the exceptions/errors are tracked in Gitlab::ErrorTracking;
  * All the exceptions/errors are recorded in the import_failures table;
  * When required, the exception/error might mark the whole import as
    failed;
  * This was created in a more generic way because it can later be used
    in the other importers (bitbucket, gitlab, etc);

* Updated Github Importer workers/importers to leverage of the
  Gitlab::Import::ImportFailureService;

* Updated Gitlab::GithubImport::Queue, to use
  Gitlab::Import::ImportFailureService when jobs exhausted retries;

* Updated Gitlab::Import::StuckImportJob to use
  Gitlab::Import::ImportFailureService;
  * This is also used in the Jira importer;

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

Changelog: fixed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67454

app/workers/concerns/gitlab/github_import/object_importer.rb

@@ -35,8 +35,24 @@ module Gitlab
         Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :imported)
 
         info(project.id, message: 'importer finished')
+      rescue KeyError => e
+        # This exception will be more useful in development when a new
+        # Representation is created but the developer forgot to add a
+        # `:github_id` field.
+        Gitlab::Import::ImportFailureService.track(
+          project_id: project.id,
+          error_source: importer_class.name,
+          exception: e,
+          fail_import: true
+        )
+
+        raise(e)
       rescue StandardError => e
-        error(project.id, e, hash)
+        Gitlab::Import::ImportFailureService.track(
+          project_id: project.id,
+          error_source: importer_class.name,
+          exception: e
+        )
       end
 
       def object_type
@@ -62,22 +78,6 @@ module Gitlab
         Logger.info(log_attributes(project_id, extra))
       end
 
-      def error(project_id, exception, data = {})
-        Logger.error(
-          log_attributes(
-            project_id,
-            message: 'importer failed',
-            'error.message': exception.message,
-            'github.data': data
-          )
-        )
-
-        Gitlab::ErrorTracking.track_and_raise_exception(
-          exception,
-          log_attributes(project_id, import_source: :github)
-        )
-      end
-
       def log_attributes(project_id, extra = {})
         extra.merge(
           project_id: project_id,

app/workers/concerns/gitlab/github_import/queue.rb

@@ -17,13 +17,10 @@ module Gitlab
         sidekiq_options dead: false, retry: 5
 
         sidekiq_retries_exhausted do |msg, e|
-          Logger.error(
-            event: :github_importer_exhausted,
-            message: msg['error_message'],
-            class: msg['class'],
-            args: msg['args'],
-            exception_message: e.message,
-            exception_backtrace: e.backtrace
+          Gitlab::Import::ImportFailureService.track(
+            project_id: msg['args'][0],
+            exception: e,
+            fail_import: true
           )
         end
       end

app/workers/concerns/gitlab/github_import/stage_methods.rb

@@ -15,7 +15,14 @@ module Gitlab
 
         info(project_id, message: 'stage finished')
       rescue StandardError => e
-        error(project_id, e)
+        Gitlab::Import::ImportFailureService.track(
+          project_id: project_id,
+          exception: e,
+          error_source: self.class.name,
+          fail_import: abort_on_failure
+        )
+
+        raise(e)
       end
 
       # client - An instance of Gitlab::GithubImport::Client.
@@ -34,25 +41,14 @@ module Gitlab
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      private
-
-      def info(project_id, extra = {})
-        Logger.info(log_attributes(project_id, extra))
+      def abort_on_failure
+        false
       end
 
-      def error(project_id, exception)
-        Logger.error(
-          log_attributes(
-            project_id,
-            message: 'stage failed',
-            'error.message': exception.message
-          )
-        )
+      private
 
-        Gitlab::ErrorTracking.track_and_raise_exception(
-          exception,
-          log_attributes(project_id, import_source: :github)
-        )
+      def info(project_id, extra = {})
+        Gitlab::GithubImport::Logger.info(log_attributes(project_id, extra))
       end
 
       def log_attributes(project_id, extra = {})

app/workers/gitlab/github_import/stage/import_repository_worker.rb

@@ -28,7 +28,7 @@ module Gitlab
           info(project.id, message: "starting importer", importer: 'Importer::RepositoryImporter')
           importer = Importer::RepositoryImporter.new(project, client)
 
-          return unless importer.execute
+          importer.execute
 
           counter.increment
 
@@ -41,6 +41,10 @@ module Gitlab
             'The number of imported GitHub repositories'
           )
         end
+
+        def abort_on_failure
+          true
+        end
       end
     end
   end

app/workers/gitlab/import/stuck_import_job.rb

@@ -5,6 +5,8 @@ module Gitlab
     module StuckImportJob
       extend ActiveSupport::Concern
 
+      StuckImportJobError = Class.new(StandardError)
+
       IMPORT_JOBS_EXPIRATION = 24.hours.seconds.to_i
 
       included do
@@ -34,9 +36,9 @@ module Gitlab
       end
 
       def mark_imports_without_jid_as_failed!
-        enqueued_import_states_without_jid.each do |import_state|
-          import_state.mark_as_failed(error_message)
-        end.size
+        enqueued_import_states_without_jid
+          .each(&method(:mark_as_failed))
+          .size
       end
 
       def mark_imports_with_jid_as_failed!
@@ -58,9 +60,20 @@ module Gitlab
           job_ids: completed_import_state_jids
         )
 
-        completed_import_states.each do |import_state|
-          import_state.mark_as_failed(error_message)
-        end.size
+        completed_import_states
+          .each(&method(:mark_as_failed))
+          .size
+      end
+
+      def mark_as_failed(import_state)
+        raise StuckImportJobError, error_message
+      rescue StuckImportJobError => e
+        Gitlab::Import::ImportFailureService.track(
+          import_state: import_state,
+          exception: e,
+          error_source: self.class.name,
+          fail_import: true
+        )
       end
 
       def enqueued_import_states

lib/gitlab/github_import/importer/lfs_objects_importer.rb

@@ -35,7 +35,11 @@ module Gitlab
             yield object
           end
         rescue StandardError => e
-          error(project.id, e)
+          Gitlab::Import::ImportFailureService.track(
+            project_id: project.id,
+            error_source: importer_class.name,
+            exception: e
+          )
         end
       end
     end

lib/gitlab/github_import/importer/repository_importer.rb

@@ -59,8 +59,6 @@ module Gitlab
           Repositories::HousekeepingService.new(project, :gc).execute
 
           true
-        rescue Gitlab::Git::Repository::NoRepository, Gitlab::Shell::Error => e
-          fail_import("Failed to import the repository: #{e.message}")
         end
 
         def import_wiki_repository
@@ -70,7 +68,8 @@ module Gitlab
         rescue ::Gitlab::Git::CommandError => e
           if e.message !~ /repository not exported/
             project.create_wiki
-            fail_import("Failed to import the wiki: #{e.message}")
+
+            raise e
           else
             true
           end
@@ -84,11 +83,6 @@ module Gitlab
           project.update_column(:last_repository_updated_at, Time.zone.now)
         end
 
-        def fail_import(message)
-          project.import_state.mark_as_failed(message)
-          false
-        end
-
         private
 
         def default_branch

lib/gitlab/github_import/parallel_scheduling.rb

@@ -49,9 +49,14 @@ module Gitlab
 
         retval
       rescue StandardError => e
-        error(project.id, e)
+        Gitlab::Import::ImportFailureService.track(
+          project_id: project.id,
+          error_source: self.class.name,
+          exception: e,
+          fail_import: abort_on_failure
+        )
 
-        raise e
+        raise(e)
       end
 
       # Imports all the objects in sequence in the current thread.
@@ -165,6 +170,10 @@ module Gitlab
         raise NotImplementedError
       end
 
+      def abort_on_failure
+        false
+      end
+
       # Any options to be passed to the method used for retrieving the data to
       # import.
       def collection_options
@@ -177,21 +186,6 @@ module Gitlab
         Logger.info(log_attributes(project_id, extra))
       end
 
-      def error(project_id, exception)
-        Logger.error(
-          log_attributes(
-            project_id,
-            message: 'importer failed',
-            'error.message': exception.message
-          )
-        )
-
-        Gitlab::ErrorTracking.track_exception(
-          exception,
-          log_attributes(project_id, import_source: :github)
-        )
-      end
-
       def log_attributes(project_id, extra = {})
         extra.merge(
           project_id: project_id,

lib/gitlab/import/import_failure_service.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Import
+    class ImportFailureService
+      def self.track(
+        exception:,
+        import_state: nil,
+        project_id: nil,
+        error_source: nil,
+        fail_import: false
+      )
+        new(
+          exception: exception,
+          import_state: import_state,
+          project_id: project_id,
+          error_source: error_source
+        ).execute(fail_import: fail_import)
+      end
+
+      def initialize(exception:, import_state: nil, project_id: nil, error_source: nil)
+        if import_state.blank? && project_id.blank?
+          raise ArgumentError, 'import_state OR project_id must be provided'
+        end
+
+        if project_id.blank?
+          @import_state = import_state
+          @project = import_state.project
+        else
+          @project = Project.find(project_id)
+          @import_state = @project.import_state
+        end
+
+        @exception = exception
+        @error_source = error_source
+      end
+
+      def execute(fail_import:)
+        track_exception
+        persist_failure
+
+        import_state.mark_as_failed(exception.message) if fail_import
+      end
+
+      private
+
+      attr_reader :exception, :import_state, :project, :error_source
+
+      def track_exception
+        attributes = {
+          import_type: project.import_type,
+          project_id: project.id,
+          source: error_source
+        }
+
+        Gitlab::Import::Logger.error(
+          attributes.merge(
+            message: 'importer failed',
+            'error.message': exception.message
+          )
+        )
+
+        Gitlab::ErrorTracking.track_exception(exception, attributes)
+      end
+
+      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
+        )
+      end
+    end
+  end
+end

spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb

@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do
-  let(:project) { double(:project, id: 4, import_source: 'foo/bar') }
+  let_it_be(:project) { create(:project, :import_started) }
+
   let(:client) { double(:client) }
   let(:download_link) { "http://www.gitlab.com/lfs_objects/oid" }
 
@@ -61,24 +62,12 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do
             .and_raise(exception)
         end
 
-        expect(Gitlab::GithubImport::Logger)
-          .to receive(:error)
-          .with(
-            message: 'importer failed',
-            project_id: project.id,
-            parallel: false,
-            importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter',
-            'error.message': 'Invalid Project URL'
-          )
-
-        expect(Gitlab::ErrorTracking)
-          .to receive(:track_exception)
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
           .with(
-            exception,
-            import_source: :github,
-            parallel: false,
             project_id: project.id,
-            importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter'
+            exception: exception,
+            error_source: 'Gitlab::GithubImport::Importer::LfsObjectImporter'
           ).and_call_original
 
         importer.execute

spec/lib/gitlab/github_import/importer/repository_importer_spec.rb

@@ -211,17 +211,6 @@ RSpec.describe Gitlab::GithubImport::Importer::RepositoryImporter do
 
       expect(importer.import_repository).to eq(true)
     end
-
-    it 'marks the import as failed when an error was raised' do
-      expect(project).to receive(:ensure_repository)
-        .and_raise(Gitlab::Git::Repository::NoRepository)
-
-      expect(importer)
-        .to receive(:fail_import)
-        .and_return(false)
-
-      expect(importer.import_repository).to eq(false)
-    end
   end
 
   describe '#import_wiki_repository' do
@@ -234,28 +223,40 @@ RSpec.describe Gitlab::GithubImport::Importer::RepositoryImporter do
       expect(importer.import_wiki_repository).to eq(true)
     end
 
-    it 'marks the import as failed and creates an empty repo if an error was raised' do
-      expect(wiki_repository)
-        .to receive(:import_repository)
-        .with(importer.wiki_url)
-        .and_raise(Gitlab::Git::CommandError)
+    context 'when it raises a Gitlab::Git::CommandError' do
+      context 'when the error is not a "repository not exported"' do
+        it 'creates the wiki and re-raise the exception' do
+          exception = Gitlab::Git::CommandError.new
 
-      expect(importer)
-        .to receive(:fail_import)
-        .and_return(false)
+          expect(wiki_repository)
+            .to receive(:import_repository)
+            .with(importer.wiki_url)
+            .and_raise(exception)
 
-      expect(project)
-        .to receive(:create_wiki)
+          expect(project)
+            .to receive(:create_wiki)
 
-      expect(importer.import_wiki_repository).to eq(false)
-    end
-  end
+          expect { importer.import_wiki_repository }
+            .to raise_error(exception)
+        end
+      end
+
+      context 'when the error is a "repository not exported"' do
+        it 'returns true' do
+          exception = Gitlab::Git::CommandError.new('repository not exported')
 
-  describe '#fail_import' do
-    it 'marks the import as failed' do
-      expect(project.import_state).to receive(:mark_as_failed).with('foo')
+          expect(wiki_repository)
+            .to receive(:import_repository)
+            .with(importer.wiki_url)
+            .and_raise(exception)
 
-      expect(importer.fail_import('foo')).to eq(false)
+          expect(project)
+            .not_to receive(:create_wiki)
+
+          expect(importer.import_wiki_repository)
+            .to eq(true)
+        end
+      end
     end
   end
 

spec/lib/gitlab/github_import/parallel_scheduling_spec.rb

@@ -5,6 +5,10 @@ require 'spec_helper'
 RSpec.describe Gitlab::GithubImport::ParallelScheduling do
   let(:importer_class) do
     Class.new do
+      def self.name
+        'MyImporter'
+      end
+
       include(Gitlab::GithubImport::ParallelScheduling)
 
       def importer_class
@@ -21,7 +25,8 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
     end
   end
 
-  let(:project) { double(:project, id: 4, import_source: 'foo/bar') }
+  let_it_be(:project) { create(:project, :import_started, import_source: 'foo/bar') }
+
   let(:client) { double(:client) }
 
   describe '#parallel?' do
@@ -100,46 +105,109 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
       importer.execute
     end
 
-    it 'logs the error when it fails' do
-      exception = StandardError.new('some error')
+    context 'when abort_on_failure is false' do
+      it 'logs the error when it fails' do
+        exception = StandardError.new('some error')
+
+        importer = importer_class.new(project, client, parallel: false)
+
+        expect(importer)
+          .to receive(:sequential_import)
+          .and_raise(exception)
+
+        expect(Gitlab::GithubImport::Logger)
+          .to receive(:info)
+          .with(
+            message: 'starting importer',
+            parallel: false,
+            project_id: project.id,
+            importer: 'Class'
+          )
+
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
+          .with(
+            project_id: project.id,
+            exception: exception,
+            error_source: 'MyImporter',
+            fail_import: false
+          ).and_call_original
+
+        expect { importer.execute }
+          .to raise_error(exception)
+
+        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')
+      end
+    end
 
-      importer = importer_class.new(project, client, parallel: false)
+    context 'when abort_on_failure is true' do
+      let(:importer_class) do
+        Class.new do
+          def self.name
+            'MyImporter'
+          end
 
-      expect(importer)
-        .to receive(:sequential_import)
-        .and_raise(exception)
+          include(Gitlab::GithubImport::ParallelScheduling)
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:info)
-        .with(
-          message: 'starting importer',
-          parallel: false,
-          project_id: project.id,
-          importer: 'Class'
-        )
+          def importer_class
+            Class
+          end
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:error)
-        .with(
-          message: 'importer failed',
-          project_id: project.id,
-          parallel: false,
-          importer: 'Class',
-          'error.message': 'some error'
-        )
+          def object_type
+            :dummy
+          end
 
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_exception)
-        .with(
-          exception,
-          parallel: false,
-          project_id: project.id,
-          import_source: :github,
-          importer: 'Class'
-        )
-        .and_call_original
+          def collection_method
+            :issues
+          end
 
-      expect { importer.execute }.to raise_error(exception)
+          def abort_on_failure
+            true
+          end
+        end
+      end
+
+      it 'logs the error when it fails and marks import as failed' do
+        exception = StandardError.new('some error')
+
+        importer = importer_class.new(project, client, parallel: false)
+
+        expect(importer)
+          .to receive(:sequential_import)
+          .and_raise(exception)
+
+        expect(Gitlab::GithubImport::Logger)
+          .to receive(:info)
+          .with(
+            message: 'starting importer',
+            parallel: false,
+            project_id: project.id,
+            importer: 'Class'
+          )
+
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
+          .with(
+            project_id: project.id,
+            exception: exception,
+            error_source: 'MyImporter',
+            fail_import: true
+          ).and_call_original
+
+        expect { importer.execute }
+          .to raise_error(exception)
+
+        expect(project.import_state.reload.status).to eq('failed')
+        expect(project.import_state.last_error).to eq('some error')
+
+        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')
+      end
     end
   end
 

spec/lib/gitlab/import/import_failure_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Import::ImportFailureService do
+  let_it_be(:import_type) { 'import_type' }
+
+  let_it_be(:project) do
+    create(
+      :project,
+      :import_started,
+      import_type: import_type
+    )
+  end
+
+  let(:import_state) { project.import_state }
+  let(:exception) { StandardError.new('some error') }
+
+  shared_examples 'logs the exception and fails the import' do
+    it 'when the failure does not abort the import' do
+      expect(Gitlab::ErrorTracking)
+        .to receive(:track_exception)
+        .with(
+          exception,
+          project_id: project.id,
+          import_type: import_type,
+          source: 'SomeImporter'
+        )
+
+      expect(Gitlab::Import::Logger)
+        .to receive(:error)
+        .with(
+          message: 'importer failed',
+          'error.message': 'some error',
+          project_id: project.id,
+          import_type: import_type,
+          source: 'SomeImporter'
+        )
+
+      described_class.track(**arguments)
+
+      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')
+    end
+  end
+
+  shared_examples 'logs the exception and does not fail the import' do
+    it 'when the failure does not abort the import' do
+      expect(Gitlab::ErrorTracking)
+        .to receive(:track_exception)
+        .with(
+          exception,
+          project_id: project.id,
+          import_type: import_type,
+          source: 'SomeImporter'
+        )
+
+      expect(Gitlab::Import::Logger)
+        .to receive(:error)
+        .with(
+          message: 'importer failed',
+          'error.message': 'some error',
+          project_id: project.id,
+          import_type: import_type,
+          source: 'SomeImporter'
+        )
+
+      described_class.track(**arguments)
+
+      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')
+    end
+  end
+
+  context 'when using the project as reference' do
+    context 'when it fails the import' do
+      let(:arguments) do
+        {
+          project_id: project.id,
+          exception: exception,
+          error_source: 'SomeImporter',
+          fail_import: true
+        }
+      end
+
+      it_behaves_like 'logs the exception and fails the import'
+    end
+
+    context 'when it does not fail the import' do
+      let(:arguments) do
+        {
+          project_id: project.id,
+          exception: exception,
+          error_source: 'SomeImporter',
+          fail_import: false
+        }
+      end
+
+      it_behaves_like 'logs the exception and does not fail the import'
+    end
+  end
+
+  context 'when using the import_state as reference' do
+    context 'when it fails the import' do
+      let(:arguments) do
+        {
+          import_state: import_state,
+          exception: exception,
+          error_source: 'SomeImporter',
+          fail_import: true
+        }
+      end
+
+      it_behaves_like 'logs the exception and fails the import'
+    end
+
+    context 'when it does not fail the import' do
+      let(:arguments) do
+        {
+          import_state: import_state,
+          exception: exception,
+          error_source: 'SomeImporter',
+          fail_import: false
+        }
+      end
+
+      it_behaves_like 'logs the exception and does not fail the import'
+    end
+  end
+end

spec/workers/concerns/gitlab/github_import/object_importer_spec.rb

@@ -21,6 +21,12 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
     end.new
   end
 
+  let_it_be(:project) { create(:project, :import_started) }
+
+  let(:importer_class) { double(:importer_class, name: 'klass_name') }
+  let(:importer_instance) { double(:importer_instance) }
+  let(:client) { double(:client) }
+
   before do
     stub_const('MockRepresantation', Class.new do
       include Gitlab::GithubImport::Representation::ToHash
@@ -39,11 +45,6 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
   end
 
   describe '#import', :clean_gitlab_redis_cache do
-    let(:importer_class) { double(:importer_class, name: 'klass_name') }
-    let(:importer_instance) { double(:importer_instance) }
-    let(:project) { double(:project, full_path: 'foo/bar', id: 1) }
-    let(:client) { double(:client) }
-
     before do
       expect(worker)
         .to receive(:importer_class)
@@ -65,7 +66,7 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
         .with(
           github_id: 1,
           message: 'starting importer',
-          project_id: 1,
+          project_id: project.id,
           importer: 'klass_name'
         )
 
@@ -74,7 +75,7 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
         .with(
           github_id: 1,
           message: 'importer finished',
-          project_id: 1,
+          project_id: project.id,
           importer: 'klass_name'
         )
 
@@ -106,59 +107,36 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
           importer: 'klass_name'
         )
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:error)
+      expect(Gitlab::Import::ImportFailureService)
+        .to receive(:track)
         .with(
-          github_id:  1,
-          message: 'importer failed',
           project_id: project.id,
-          importer: 'klass_name',
-          'error.message': 'some error',
-          'github.data': {
-            'github_id' => 1,
-            'number' => 10
-          }
+          exception: exception,
+          error_source: 'klass_name'
         )
+        .and_call_original
 
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
-        .with(
-          exception,
-          import_source: :github,
-          github_id: 1,
-          project_id: 1,
-          importer: 'klass_name'
-        ).and_call_original
+      worker.import(project, client, { 'number' => 10, 'github_id' => 1 })
 
-      expect { worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) }
-        .to raise_error(exception)
+      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')
     end
 
     it 'logs error when representation does not have a github_id' do
       expect(importer_class).not_to receive(:new)
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:error)
+      expect(Gitlab::Import::ImportFailureService)
+        .to receive(:track)
         .with(
-          github_id:  nil,
-          message: 'importer failed',
           project_id: project.id,
-          importer: 'klass_name',
-          'error.message': 'key not found: :github_id',
-          'github.data': {
-            'number' => 10
-          }
+          exception: a_kind_of(KeyError),
+          error_source: 'klass_name',
+          fail_import: true
         )
-
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
-        .with(
-          an_instance_of(KeyError),
-          import_source: :github,
-          github_id: nil,
-          project_id: 1,
-          importer: 'klass_name'
-        ).and_call_original
+        .and_call_original
 
       expect { worker.import(project, client, { 'number' => 10 }) }
         .to raise_error(KeyError, 'key not found: :github_id')

spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb

@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::StageMethods do
-  let(:project) { create(:project) }
+  let_it_be(:project) { create(:project, :import_started, import_url: 'https://t0ken@github.com/repo/repo.git') }
+
   let(:worker) do
     Class.new do
       def self.name
@@ -15,8 +16,6 @@ RSpec.describe Gitlab::GithubImport::StageMethods do
   end
 
   describe '#perform' do
-    let(:project) { create(:project, import_url: 'https://t0ken@github.com/repo/repo.git') }
-
     it 'returns if no project could be found' do
       expect(worker).not_to receive(:try_import)
 
@@ -55,46 +54,100 @@ RSpec.describe Gitlab::GithubImport::StageMethods do
       worker.perform(project.id)
     end
 
-    it 'logs error when import fails' do
-      exception = StandardError.new('some error')
-
-      allow(worker)
-        .to receive(:find_project)
-        .with(project.id)
-        .and_return(project)
-
-      expect(worker)
-        .to receive(:try_import)
-        .and_raise(exception)
+    context 'when abort_on_failure is false' do
+      it 'logs error when import fails' do
+        exception = StandardError.new('some error')
+
+        allow(worker)
+          .to receive(:find_project)
+          .with(project.id)
+          .and_return(project)
+
+        expect(worker)
+          .to receive(:try_import)
+          .and_raise(exception)
+
+        expect(Gitlab::GithubImport::Logger)
+          .to receive(:info)
+          .with(
+            message: 'starting stage',
+            project_id: project.id,
+            import_stage: 'DummyStage'
+          )
+
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
+          .with(
+            project_id: project.id,
+            exception: exception,
+            error_source: 'DummyStage',
+            fail_import: false
+          ).and_call_original
+
+        expect { worker.perform(project.id) }
+          .to raise_error(exception)
+
+        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')
+      end
+    end
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:info)
-        .with(
-          message: 'starting stage',
-          project_id: project.id,
-          import_stage: 'DummyStage'
-        )
+    context 'when abort_on_failure is true' do
+      let(:worker) do
+        Class.new do
+          def self.name
+            'DummyStage'
+          end
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:error)
-        .with(
-          message: 'stage failed',
-          project_id: project.id,
-          import_stage: 'DummyStage',
-          'error.message': 'some error'
-        )
+          def abort_on_failure
+            true
+          end
 
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
-        .with(
-          exception,
-          import_source: :github,
-          project_id: project.id,
-          import_stage: 'DummyStage'
-        )
-        .and_call_original
+          include(Gitlab::GithubImport::StageMethods)
+        end.new
+      end
 
-      expect { worker.perform(project.id) }.to raise_error(exception)
+      it 'logs, captures and re-raises the exception and also marks the import as failed' do
+        exception = StandardError.new('some error')
+
+        allow(worker)
+          .to receive(:find_project)
+          .with(project.id)
+          .and_return(project)
+
+        expect(worker)
+          .to receive(:try_import)
+          .and_raise(exception)
+
+        expect(Gitlab::GithubImport::Logger)
+          .to receive(:info)
+          .with(
+            message: 'starting stage',
+            project_id: project.id,
+            import_stage: 'DummyStage'
+          )
+
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
+          .with(
+            project_id: project.id,
+            exception: exception,
+            error_source: 'DummyStage',
+            fail_import: true
+          ).and_call_original
+
+        expect { worker.perform(project.id) }.to raise_error(exception)
+
+        expect(project.import_state.reload.status).to eq('failed')
+        expect(project.import_state.last_error).to eq('some error')
+
+        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')
+      end
     end
   end
 
@@ -126,16 +179,14 @@ RSpec.describe Gitlab::GithubImport::StageMethods do
   end
 
   describe '#find_project' do
-    let(:import_state) { create(:import_state, project: project) }
-
     it 'returns a Project for an existing ID' do
-      import_state.update_column(:status, 'started')
+      project.import_state.update_column(:status, 'started')
 
       expect(worker.find_project(project.id)).to eq(project)
     end
 
     it 'returns nil for a project that failed importing' do
-      import_state.update_column(:status, 'failed')
+      project.import_state.update_column(:status, 'failed')
 
       expect(worker.find_project(project.id)).to be_nil
     end

spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do
   let(:project) { double(:project, id: 4) }
+
   let(:worker) { described_class.new }
 
   describe '#import' do
@@ -36,15 +37,19 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do
     context 'when the import fails' do
       it 'does not schedule the importing of the base data' do
         client = double(:client)
+        exception_class = Gitlab::Git::Repository::NoRepository
 
         expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance|
-          expect(instance).to receive(:execute).and_return(false)
+          expect(instance).to receive(:execute).and_raise(exception_class)
         end
 
         expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker)
           .not_to receive(:perform_async)
 
-        worker.import(client, project)
+        expect(worker.abort_on_failure).to eq(true)
+
+        expect { worker.import(client, project) }
+          .to raise_error(exception_class)
       end
     end
   end

spec/workers/gitlab/import/stuck_import_job_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Import::StuckImportJob do
+  let_it_be(:project) { create(:project, :import_started, import_source: 'foo/bar') }
+
+  let(:worker) do
+    Class.new do
+      def self.name
+        'MyStuckProjectImportsWorker'
+      end
+
+      include(Gitlab::Import::StuckImportJob)
+
+      def track_metrics(...)
+        nil
+      end
+
+      def enqueued_import_states
+        ProjectImportState.with_status([:scheduled, :started])
+      end
+    end.new
+  end
+
+  it 'marks the stuck import project as failed and track the error on import_failures' do
+    worker.perform
+
+    expect(project.import_state.reload.status).to eq('failed')
+    expect(project.import_state.last_error).to eq('Import timed out. Import took longer than 86400 seconds')
+
+    expect(project.import_failures).not_to be_empty
+    expect(project.import_failures.last.exception_class).to eq('Gitlab::Import::StuckImportJob::StuckImportJobError')
+    expect(project.import_failures.last.exception_message).to eq('Import timed out. Import took longer than 86400 seconds')
+  end
+end