diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/models/project.rb | 12 | ||||
| -rw-r--r-- | app/services/projects/create_service.rb | 26 | ||||
| -rw-r--r-- | app/services/projects/import_service.rb | 2 | ||||
| -rw-r--r-- | app/workers/repository_fork_worker.rb | 6 | ||||
| -rw-r--r-- | app/workers/repository_import_worker.rb | 3 | ||||
| -rw-r--r-- | spec/services/projects/import_service_spec.rb | 2 |
7 files changed, 38 insertions, 14 deletions
diff --git a/CHANGELOG b/CHANGELOG index 68ea866968d..62870aee083 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -31,6 +31,7 @@ v 8.9.0 (unreleased) - Cache assigned issue and merge request counts in sidebar nav - Cache project build count in sidebar nav - Reduce number of queries needed to render issue labels in the sidebar + - Improve error handling importing projects v 8.8.3 - Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312 diff --git a/app/models/project.rb b/app/models/project.rb index 9ccf6a97df6..e4a9d17a20c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1017,4 +1017,16 @@ class Project < ActiveRecord::Base builds.running_or_pending.count(:all) end end + + def mark_import_as_failed(error_message) + original_errors = errors.dup + sanitized_message = Gitlab::UrlSanitizer.sanitize(error_message) + + import_fail + update_column(:import_error, sanitized_message) + rescue ActiveRecord::ActiveRecordError => e + Rails.logger.error("Error setting import status to failed: #{e.message}. Original error: #{sanitized_message}") + ensure + @errors = original_errors + end end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 6728fabea1e..61cac5419ad 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -56,14 +56,14 @@ module Projects after_create_actions if @project.persisted? - @project.add_import_job if @project.import? - + if @project.errors.empty? + @project.add_import_job if @project.import? + else + fail(error: @project.errors.full_messages.join(', ')) + end @project rescue => e - message = "Unable to save project: #{e.message}" - Rails.logger.error(message) - @project.errors.add(:base, message) if @project - @project + fail(error: e.message) end protected @@ -103,5 +103,19 @@ module Projects end end end + + def fail(error:) + message = "Unable to save project. Error: #{error}" + message << "Project ID: #{@project.id}" if @project && @project.id + + Rails.logger.error(message) + + if @project && @project.import? + @project.errors.add(:base, message) + @project.mark_import_as_failed(message) + end + + @project + end end end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index ef15ef6a473..c4838d31f2f 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -39,7 +39,7 @@ module Projects begin gitlab_shell.import_repository(project.path_with_namespace, project.import_url) rescue Gitlab::Shell::Error => e - raise Error, e.message + raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}" end end diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index f9e32337983..d947f105516 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -15,8 +15,7 @@ class RepositoryForkWorker result = gitlab_shell.fork_repository(source_path, target_path) unless result logger.error("Unable to fork project #{project_id} for repository #{source_path} -> #{target_path}") - project.update(import_error: "The project could not be forked.") - project.import_fail + project.mark_import_as_failed('The project could not be forked.') return end @@ -24,8 +23,7 @@ class RepositoryForkWorker unless project.valid_repo? logger.error("Project #{project_id} had an invalid repository after fork") - project.update(import_error: "The forked repository is invalid.") - project.import_fail + project.mark_import_as_failed('The forked repository is invalid.') return end diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index fbc7ed63c6a..7d819fe78f8 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -13,8 +13,7 @@ class RepositoryImportWorker result = Projects::ImportService.new(project, current_user).execute if result[:status] == :error - project.update(import_error: Gitlab::UrlSanitizer.sanitize(result[:message])) - project.import_fail + project.mark_import_as_failed(result[:message]) return end diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index 7f2dcdab960..9d90bfceb73 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -49,7 +49,7 @@ describe Projects::ImportService, services: true do result = subject.execute expect(result[:status]).to eq :error - expect(result[:message]).to eq 'Failed to import the repository' + expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.path_with_namespace} - Failed to import the repository" end end |