Merge branch 'kassio/github-importer-log-and-fail-on-exceptions' into 'master'

GithubImporter: Ensure to fail and log imports on exceptions

See merge request gitlab-org/gitlab!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