From 72a85ae9ac2468b099a565d3848bf8e0dcdf4499 Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Fri, 28 Apr 2017 06:46:15 +0000
Subject: [PATCH 1/5] Handle errors while a project is being deleted
 asynchronously.

1. Rescue all errors that `Projects::DestroyService` might throw, to prevent the
   worker from leaving things in an inconsistent state

2. Unmark the project as `pending_delete`

3. Add a `delete_error` text column to `projects`, and save the error message in
   there, to be shown to the project masters/owners.
---
 app/services/projects/destroy_service.rb      |  9 +--
 app/workers/project_destroy_worker.rb         |  3 +
 ...307_add_column_delete_error_to_projects.rb | 31 +++++++++
 db/schema.rb                                  |  1 +
 .../import_export/safe_model_attributes.yml   |  1 +
 .../services/projects/destroy_service_spec.rb | 63 +++++++++++++++++++
 6 files changed, 104 insertions(+), 4 deletions(-)
 create mode 100644 db/migrate/20170428064307_add_column_delete_error_to_projects.rb

diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index e2b2660ea71..682407ac896 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -26,9 +26,6 @@ module Projects
       Projects::UnlinkForkService.new(project, current_user).execute
 
       Project.transaction do
-        project.team.truncate
-        project.destroy!
-
         unless remove_legacy_registry_tags
           raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
         end
@@ -40,10 +37,14 @@ module Projects
         unless remove_repository(wiki_path)
           raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
         end
+
+        project.team.truncate
+        project.destroy!
       end
 
-      log_info("Project \"#{project.path_with_namespace}\" was removed")
       system_hook_service.execute_hooks_for(project, :destroy)
+
+      log_info("Project \"#{project.path_with_namespace}\" was removed")
       true
     end
 
diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb
index b462327490e..482e1e38cd1 100644
--- a/app/workers/project_destroy_worker.rb
+++ b/app/workers/project_destroy_worker.rb
@@ -12,5 +12,8 @@ class ProjectDestroyWorker
     user = User.find(user_id)
 
     ::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
+  rescue StandardError => error
+    project.assign_attributes(delete_error: error.message, pending_delete: false)
+    project.save!(validate: false)
   end
 end
diff --git a/db/migrate/20170428064307_add_column_delete_error_to_projects.rb b/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
new file mode 100644
index 00000000000..ef5fc2cdea5
--- /dev/null
+++ b/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
@@ -0,0 +1,31 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddColumnDeleteErrorToProjects < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+  # "add_column_with_default" you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    add_column :projects, :delete_error, :text
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 284b2068166..0ba2bd31517 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1134,6 +1134,7 @@ ActiveRecord::Schema.define(version: 20170717150329) do
     t.integer "cached_markdown_version"
     t.datetime "last_repository_updated_at"
     t.string "ci_config_path"
+    t.text "delete_error"
   end
 
   add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 4ef3db3721f..0f2db3380a7 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -396,6 +396,7 @@ Project:
 - build_allow_git_fetch
 - last_repository_updated_at
 - ci_config_path
+- delete_error
 Author:
 - name
 ProjectFeature:
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index b399d3402fd..a629afe723d 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -36,6 +36,27 @@ describe Projects::DestroyService, services: true do
     end
   end
 
+  shared_examples 'handles errors thrown during async destroy' do |error_message|
+    it 'does not allow the error to bubble up' do
+      expect do
+        Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+      end.not_to raise_error
+    end
+
+    it 'unmarks the project as "pending deletion"' do
+      Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+
+      expect(project.reload.pending_delete).to be(false)
+    end
+
+    it 'stores an error message in `projects.delete_error`' do
+      Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+
+      expect(project.reload.delete_error).to be_present
+      expect(project.delete_error).to include(error_message)
+    end
+  end
+
   context 'Sidekiq inline' do
     before do
       # Run sidekiq immediatly to check that renamed repository will be removed
@@ -89,10 +110,52 @@ describe Projects::DestroyService, services: true do
     end
 
     it_behaves_like 'deleting the project with pipeline and build'
+
+    context 'errors' do
+      context 'when `remove_legacy_registry_tags` fails' do
+        before do
+          expect_any_instance_of(Projects::DestroyService)
+            .to receive(:remove_legacy_registry_tags).and_return(false)
+        end
+
+        it_behaves_like 'handles errors thrown during async destroy', "Failed to remove some tags"
+      end
+
+      context 'when `remove_repository` fails' do
+        before do
+          expect_any_instance_of(Projects::DestroyService)
+            .to receive(:remove_repository).and_return(false)
+        end
+
+        it_behaves_like 'handles errors thrown during async destroy', "Failed to remove project repository"
+      end
+
+      context 'when `execute` raises any other error' do
+        before do
+          expect_any_instance_of(Projects::DestroyService)
+            .to receive(:execute).and_raise(ArgumentError.new("Other error message"))
+        end
+
+        it_behaves_like 'handles errors thrown during async destroy', "Other error message"
+      end
+    end
   end
 
   context 'with execute' do
     it_behaves_like 'deleting the project with pipeline and build'
+
+    context 'when `execute` raises an error' do
+      before do
+        expect_any_instance_of(Projects::DestroyService)
+          .to receive(:execute).and_raise(ArgumentError)
+      end
+
+      it 'allows the error to bubble up' do
+        expect do
+          Sidekiq::Testing.inline! { Projects::DestroyService.new(project, user, {}).execute }
+        end.to raise_error(ArgumentError)
+      end
+    end
   end
 
   describe 'container registry' do

From f0e4e3993b1f5a21ab61aaff95f73ac4e5b88ad3 Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Thu, 4 May 2017 10:50:05 +0000
Subject: [PATCH 2/5] WIP: Display a project's `delete_error` on the project
 homepage.

---
 app/views/projects/_deletion_failed.html.haml | 9 +++++++++
 app/views/projects/show.html.haml             | 1 +
 2 files changed, 10 insertions(+)
 create mode 100644 app/views/projects/_deletion_failed.html.haml

diff --git a/app/views/projects/_deletion_failed.html.haml b/app/views/projects/_deletion_failed.html.haml
new file mode 100644
index 00000000000..cd717760432
--- /dev/null
+++ b/app/views/projects/_deletion_failed.html.haml
@@ -0,0 +1,9 @@
+- if @project.delete_error.present?
+  .project-deletion-failed-message.alert.alert-warning
+    This project was scheduled for deletion, but failed with the message:
+    = @project.delete_error
+
+    .alert-link-group
+      = link_to "Don't show again", profile_path(user: {hide_no_ssh_key: true}), method: :put, class: 'alert-link'
+      |
+      = link_to 'Remind later', '#', class: 'hide-no-ssh-message alert-link'
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index 49d0a6828fe..336bc694ffc 100644
--- a/app/views/projects/show.html.haml
+++ b/app/views/projects/show.html.haml
@@ -7,6 +7,7 @@
   = auto_discovery_link_tag(:atom, project_path(@project, rss_url_options), title: "#{@project.name} activity")
 
 = content_for flash_message_container do
+  = render 'deletion_failed'
   - if current_user && can?(current_user, :download_code, @project)
     = render 'shared/no_ssh'
     = render 'shared/no_password'

From 3491b19a4e67a9f439c12afac45ef38f3fce0ef5 Mon Sep 17 00:00:00 2001
From: Tiago Botelho <tiagonbotelho@hotmail.com>
Date: Thu, 29 Jun 2017 12:43:01 +0100
Subject: [PATCH 3/5] Add specs for ProjectDestroyWorker

---
 app/services/projects/destroy_service.rb      |  2 +-
 app/views/projects/_deletion_failed.html.haml |  4 +--
 app/views/projects/_flash_messages.html.haml  |  5 ++++
 app/views/projects/empty.html.haml            |  5 +---
 app/views/projects/show.html.haml             |  6 +---
 app/workers/project_destroy_worker.rb         | 15 ++++------
 ...307_add_column_delete_error_to_projects.rb | 24 ---------------
 spec/features/projects/show_project_spec.rb   | 30 +++++++++++++++++++
 spec/workers/project_destroy_worker_spec.rb   | 21 ++++++++++---
 9 files changed, 63 insertions(+), 49 deletions(-)
 create mode 100644 app/views/projects/_flash_messages.html.haml
 create mode 100644 spec/features/projects/show_project_spec.rb

diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 682407ac896..7b0a08af290 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -44,7 +44,7 @@ module Projects
 
       system_hook_service.execute_hooks_for(project, :destroy)
 
-      log_info("Project \"#{project.path_with_namespace}\" was removed")
+      log_info("Project \"#{project.full_path}\" was removed")
       true
     end
 
diff --git a/app/views/projects/_deletion_failed.html.haml b/app/views/projects/_deletion_failed.html.haml
index cd717760432..028510b5671 100644
--- a/app/views/projects/_deletion_failed.html.haml
+++ b/app/views/projects/_deletion_failed.html.haml
@@ -1,9 +1,9 @@
 - if @project.delete_error.present?
   .project-deletion-failed-message.alert.alert-warning
-    This project was scheduled for deletion, but failed with the message:
+    This project was scheduled for deletion, but failed with the following message:
     = @project.delete_error
 
     .alert-link-group
-      = link_to "Don't show again", profile_path(user: {hide_no_ssh_key: true}), method: :put, class: 'alert-link'
+      = link_to "Don't show again", profile_path(user: { hide_no_ssh_key: true }), method: :put, class: 'alert-link'
       |
       = link_to 'Remind later', '#', class: 'hide-no-ssh-message alert-link'
diff --git a/app/views/projects/_flash_messages.html.haml b/app/views/projects/_flash_messages.html.haml
new file mode 100644
index 00000000000..6c9d466c761
--- /dev/null
+++ b/app/views/projects/_flash_messages.html.haml
@@ -0,0 +1,5 @@
+= content_for flash_message_container do
+  = render 'deletion_failed'
+  - if current_user && can?(current_user, :download_code, project)
+    = render 'shared/no_ssh'
+    = render 'shared/no_password'
diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml
index 0f132a68ce1..3d7c72ae61a 100644
--- a/app/views/projects/empty.html.haml
+++ b/app/views/projects/empty.html.haml
@@ -1,10 +1,7 @@
 - @no_container = true
 - flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
 
-= content_for flash_message_container do
-  - if current_user && can?(current_user, :download_code, @project)
-    = render 'shared/no_ssh'
-    = render 'shared/no_password'
+= render partial: 'flash_messages', locals: { project: @project }
 
 = render "projects/head"
 = render "home_panel"
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index 336bc694ffc..3926149e790 100644
--- a/app/views/projects/show.html.haml
+++ b/app/views/projects/show.html.haml
@@ -6,11 +6,7 @@
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, project_path(@project, rss_url_options), title: "#{@project.name} activity")
 
-= content_for flash_message_container do
-  = render 'deletion_failed'
-  - if current_user && can?(current_user, :download_code, @project)
-    = render 'shared/no_ssh'
-    = render 'shared/no_password'
+= render partial: 'flash_messages', locals: { project: @project }
 
 = render "projects/head"
 = render "projects/last_push"
diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb
index 482e1e38cd1..209cf11e893 100644
--- a/app/workers/project_destroy_worker.rb
+++ b/app/workers/project_destroy_worker.rb
@@ -3,17 +3,14 @@ class ProjectDestroyWorker
   include DedicatedSidekiqQueue
 
   def perform(project_id, user_id, params)
-    begin
-      project = Project.unscoped.find(project_id)
-    rescue ActiveRecord::RecordNotFound
-      return
-    end
-
+    project = Project.find(project_id)
     user = User.find(user_id)
 
     ::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
-  rescue StandardError => error
-    project.assign_attributes(delete_error: error.message, pending_delete: false)
-    project.save!(validate: false)
+  rescue Exception => error # rubocop:disable Lint/RescueException
+    project&.update_attributes(delete_error: error.message, pending_delete: false)
+    Rails.logger.error("Deletion failed on #{project&.full_path} with the following message: #{error.message}")
+
+    raise
   end
 end
diff --git a/db/migrate/20170428064307_add_column_delete_error_to_projects.rb b/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
index ef5fc2cdea5..09f9d9b5b7a 100644
--- a/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
+++ b/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
@@ -1,30 +1,6 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class AddColumnDeleteErrorToProjects < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
-  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
-  # "add_column_with_default" you must disable the use of transactions
-  # as these methods can not run in an existing transaction.
-  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
-  # that either of them is the _only_ method called in the migration,
-  # any other changes should go in a separate migration.
-  # This ensures that upon failure _only_ the index creation or removing fails
-  # and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  # disable_ddl_transaction!
-
   def change
     add_column :projects, :delete_error, :text
   end
diff --git a/spec/features/projects/show_project_spec.rb b/spec/features/projects/show_project_spec.rb
new file mode 100644
index 00000000000..5aa0d8f0026
--- /dev/null
+++ b/spec/features/projects/show_project_spec.rb
@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe 'Project show page', feature: true do
+  context 'when project pending delete' do
+    let(:project) { create(:project, :empty_repo, pending_delete: true) }
+    let(:worker) { ProjectDestroyWorker.new }
+
+    before do
+      sign_in(project.owner)
+    end
+
+    it 'shows flash error if deletion for project fails' do
+      error_message = "some error message"
+      project.update_attributes(delete_error: error_message, pending_delete: false)
+
+      visit namespace_project_path(project.namespace, project)
+
+      expect(page).to have_selector('.project-deletion-failed-message')
+      expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{error_message}")
+    end
+
+    it 'renders 404 if project was successfully deleted' do
+      worker.perform(project.id, project.owner.id, {})
+
+      visit namespace_project_path(project.namespace, project)
+
+      expect(page).to have_http_status(404)
+    end
+  end
+end
diff --git a/spec/workers/project_destroy_worker_spec.rb b/spec/workers/project_destroy_worker_spec.rb
index 3d135f40c1f..29f0295de42 100644
--- a/spec/workers/project_destroy_worker_spec.rb
+++ b/spec/workers/project_destroy_worker_spec.rb
@@ -1,24 +1,37 @@
 require 'spec_helper'
 
 describe ProjectDestroyWorker do
-  let(:project) { create(:project, :repository) }
+  let(:project) { create(:project, :repository, pending_delete: true) }
   let(:path) { project.repository.path_to_repo }
 
   subject { described_class.new }
 
-  describe "#perform" do
-    it "deletes the project" do
+  describe '#perform' do
+    it 'deletes the project' do
       subject.perform(project.id, project.owner.id, {})
 
       expect(Project.all).not_to include(project)
       expect(Dir.exist?(path)).to be_falsey
     end
 
-    it "deletes the project but skips repo deletion" do
+    it 'deletes the project but skips repo deletion' do
       subject.perform(project.id, project.owner.id, { "skip_repo" => true })
 
       expect(Project.all).not_to include(project)
       expect(Dir.exist?(path)).to be_truthy
     end
+
+    describe 'when StandardError is raised' do
+      it 'reverts pending_delete attribute with a error message' do
+        allow_any_instance_of(::Projects::DestroyService).to receive(:execute).and_raise(StandardError, "some error message")
+
+        expect do
+          subject.perform(project.id, project.owner.id, {})
+        end.to change { project.reload.pending_delete }.from(true).to(false)
+
+        expect(Project.all).to include(project)
+        expect(project.delete_error).to eq("some error message")
+      end
+    end
   end
 end

From 0aa8249e484ca97cfc28c7301d69077919032c08 Mon Sep 17 00:00:00 2001
From: Tiago Botelho <tiagonbotelho@hotmail.com>
Date: Thu, 6 Jul 2017 14:43:07 +0100
Subject: [PATCH 4/5] Refactors Project Destroy service and worker code

---
 app/services/projects/destroy_service.rb | 42 +++++++++++++++---------
 app/workers/project_destroy_worker.rb    |  7 ++--
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 7b0a08af290..7e38aacc91a 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -25,27 +25,15 @@ module Projects
 
       Projects::UnlinkForkService.new(project, current_user).execute
 
-      Project.transaction do
-        unless remove_legacy_registry_tags
-          raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
-        end
-
-        unless remove_repository(repo_path)
-          raise_error('Failed to remove project repository. Please try again or contact administrator.')
-        end
-
-        unless remove_repository(wiki_path)
-          raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
-        end
-
-        project.team.truncate
-        project.destroy!
-      end
+      attempt_destroy_transaction(project, repo_path, wiki_path)
 
       system_hook_service.execute_hooks_for(project, :destroy)
 
       log_info("Project \"#{project.full_path}\" was removed")
       true
+    rescue Projects::DestroyService::DestroyError => error
+      Rails.logger.error("Deletion failed on #{project.full_path} with the following message: #{error.message}")
+      false
     end
 
     private
@@ -71,6 +59,28 @@ module Projects
       end
     end
 
+    def attempt_destroy_transaction(project, repo_path, wiki_path)
+      Project.transaction do
+        unless remove_legacy_registry_tags
+          raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
+        end
+
+        unless remove_repository(repo_path)
+          raise_error('Failed to remove project repository. Please try again or contact administrator.')
+        end
+
+        unless remove_repository(wiki_path)
+          raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
+        end
+
+        project.team.truncate
+        project.destroy!
+      end
+    rescue Exception => error # rubocop:disable Lint/RescueException
+      project.update_attributes(delete_error: error.message, pending_delete: false)
+      raise
+    end
+
     ##
     # This method makes sure that we correctly remove registry tags
     # for legacy image repository (when repository path equals project path).
diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb
index 209cf11e893..e695ec060f0 100644
--- a/app/workers/project_destroy_worker.rb
+++ b/app/workers/project_destroy_worker.rb
@@ -7,10 +7,7 @@ class ProjectDestroyWorker
     user = User.find(user_id)
 
     ::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
-  rescue Exception => error # rubocop:disable Lint/RescueException
-    project&.update_attributes(delete_error: error.message, pending_delete: false)
-    Rails.logger.error("Deletion failed on #{project&.full_path} with the following message: #{error.message}")
-
-    raise
+  rescue ActiveRecord::RecordNotFound => error
+    logger.error("Failed to delete project #{project.path_with_namespace} (#{project.id}): #{error.message}")
   end
 end

From b5bdc55d239f3e19f8fe1e59b118da05ac81a0dd Mon Sep 17 00:00:00 2001
From: Tiago Botelho <tiagonbotelho@hotmail.com>
Date: Tue, 18 Jul 2017 16:09:14 +0100
Subject: [PATCH 5/5] Move exception handling to execute

---
 app/services/projects/destroy_service.rb      | 57 ++++++++++++-------
 app/views/projects/_deletion_failed.html.haml | 13 ++---
 app/views/projects/_flash_messages.html.haml  |  5 +-
 app/views/projects/empty.html.haml            |  1 -
 app/views/projects/show.html.haml             |  1 -
 app/workers/project_destroy_worker.rb         |  2 +-
 ...project-destroy-clean-up-after-failure.yml |  4 ++
 spec/features/projects/show_project_spec.rb   | 18 ++----
 .../services/projects/destroy_service_spec.rb | 37 ++++++------
 spec/workers/project_destroy_worker_spec.rb   | 19 +++----
 10 files changed, 81 insertions(+), 76 deletions(-)
 create mode 100644 changelogs/unreleased/29289-project-destroy-clean-up-after-failure.yml

diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 7e38aacc91a..f6e8b6655f2 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -15,29 +15,48 @@ module Projects
     def execute
       return false unless can?(current_user, :remove_project, project)
 
-      repo_path = project.path_with_namespace
-      wiki_path = repo_path + '.wiki'
-
       # Flush the cache for both repositories. This has to be done _before_
       # removing the physical repositories as some expiration code depends on
       # Git data (e.g. a list of branch names).
-      flush_caches(project, wiki_path)
+      flush_caches(project)
 
       Projects::UnlinkForkService.new(project, current_user).execute
 
-      attempt_destroy_transaction(project, repo_path, wiki_path)
+      attempt_destroy_transaction(project)
 
       system_hook_service.execute_hooks_for(project, :destroy)
-
       log_info("Project \"#{project.full_path}\" was removed")
+
       true
-    rescue Projects::DestroyService::DestroyError => error
-      Rails.logger.error("Deletion failed on #{project.full_path} with the following message: #{error.message}")
+    rescue => error
+      attempt_rollback(project, error.message)
       false
+    rescue Exception => error # rubocop:disable Lint/RescueException
+      # Project.transaction can raise Exception
+      attempt_rollback(project, error.message)
+      raise
     end
 
     private
 
+    def repo_path
+      project.path_with_namespace
+    end
+
+    def wiki_path
+      repo_path + '.wiki'
+    end
+
+    def trash_repositories!
+      unless remove_repository(repo_path)
+        raise_error('Failed to remove project repository. Please try again or contact administrator.')
+      end
+
+      unless remove_repository(wiki_path)
+        raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
+      end
+    end
+
     def remove_repository(path)
       # Skip repository removal. We use this flag when remove user or group
       return true if params[:skip_repo] == true
@@ -59,26 +78,24 @@ module Projects
       end
     end
 
-    def attempt_destroy_transaction(project, repo_path, wiki_path)
+    def attempt_rollback(project, message)
+      return unless project
+
+      project.update_attributes(delete_error: message, pending_delete: false)
+      log_error("Deletion failed on #{project.full_path} with the following message: #{message}")
+    end
+
+    def attempt_destroy_transaction(project)
       Project.transaction do
         unless remove_legacy_registry_tags
           raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
         end
 
-        unless remove_repository(repo_path)
-          raise_error('Failed to remove project repository. Please try again or contact administrator.')
-        end
-
-        unless remove_repository(wiki_path)
-          raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
-        end
+        trash_repositories!
 
         project.team.truncate
         project.destroy!
       end
-    rescue Exception => error # rubocop:disable Lint/RescueException
-      project.update_attributes(delete_error: error.message, pending_delete: false)
-      raise
     end
 
     ##
@@ -107,7 +124,7 @@ module Projects
       "#{path}+#{project.id}#{DELETED_FLAG}"
     end
 
-    def flush_caches(project, wiki_path)
+    def flush_caches(project)
       project.repository.before_delete
 
       Repository.new(wiki_path, project).before_delete
diff --git a/app/views/projects/_deletion_failed.html.haml b/app/views/projects/_deletion_failed.html.haml
index 028510b5671..4f3698f91e6 100644
--- a/app/views/projects/_deletion_failed.html.haml
+++ b/app/views/projects/_deletion_failed.html.haml
@@ -1,9 +1,6 @@
-- if @project.delete_error.present?
-  .project-deletion-failed-message.alert.alert-warning
-    This project was scheduled for deletion, but failed with the following message:
-    = @project.delete_error
+- project = local_assigns.fetch(:project)
+- return unless project.delete_error.present?
 
-    .alert-link-group
-      = link_to "Don't show again", profile_path(user: { hide_no_ssh_key: true }), method: :put, class: 'alert-link'
-      |
-      = link_to 'Remind later', '#', class: 'hide-no-ssh-message alert-link'
+.project-deletion-failed-message.alert.alert-warning
+  This project was scheduled for deletion, but failed with the following message:
+  = project.delete_error
diff --git a/app/views/projects/_flash_messages.html.haml b/app/views/projects/_flash_messages.html.haml
index 6c9d466c761..f47d84ef755 100644
--- a/app/views/projects/_flash_messages.html.haml
+++ b/app/views/projects/_flash_messages.html.haml
@@ -1,5 +1,8 @@
+- project = local_assigns.fetch(:project)
+- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
+
 = content_for flash_message_container do
-  = render 'deletion_failed'
+  = render partial: 'deletion_failed', locals: { project: project }
   - if current_user && can?(current_user, :download_code, project)
     = render 'shared/no_ssh'
     = render 'shared/no_password'
diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml
index 3d7c72ae61a..d17709380d5 100644
--- a/app/views/projects/empty.html.haml
+++ b/app/views/projects/empty.html.haml
@@ -1,5 +1,4 @@
 - @no_container = true
-- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
 
 = render partial: 'flash_messages', locals: { project: @project }
 
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index 3926149e790..a9b39cedb1d 100644
--- a/app/views/projects/show.html.haml
+++ b/app/views/projects/show.html.haml
@@ -1,7 +1,6 @@
 - @no_container = true
 - breadcrumb_title "Project"
 - @content_class = "limit-container-width" unless fluid_layout
-- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
 
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, project_path(@project, rss_url_options), title: "#{@project.name} activity")
diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb
index e695ec060f0..a9188b78460 100644
--- a/app/workers/project_destroy_worker.rb
+++ b/app/workers/project_destroy_worker.rb
@@ -8,6 +8,6 @@ class ProjectDestroyWorker
 
     ::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
   rescue ActiveRecord::RecordNotFound => error
-    logger.error("Failed to delete project #{project.path_with_namespace} (#{project.id}): #{error.message}")
+    logger.error("Failed to delete project (#{project_id}): #{error.message}")
   end
 end
diff --git a/changelogs/unreleased/29289-project-destroy-clean-up-after-failure.yml b/changelogs/unreleased/29289-project-destroy-clean-up-after-failure.yml
new file mode 100644
index 00000000000..488b37ac37f
--- /dev/null
+++ b/changelogs/unreleased/29289-project-destroy-clean-up-after-failure.yml
@@ -0,0 +1,4 @@
+---
+title: Handle errors while a project is being deleted asynchronously.
+merge_request: 11088
+author:
diff --git a/spec/features/projects/show_project_spec.rb b/spec/features/projects/show_project_spec.rb
index 5aa0d8f0026..1bc6fae9e7f 100644
--- a/spec/features/projects/show_project_spec.rb
+++ b/spec/features/projects/show_project_spec.rb
@@ -3,28 +3,18 @@ require 'spec_helper'
 describe 'Project show page', feature: true do
   context 'when project pending delete' do
     let(:project) { create(:project, :empty_repo, pending_delete: true) }
-    let(:worker) { ProjectDestroyWorker.new }
 
     before do
       sign_in(project.owner)
     end
 
-    it 'shows flash error if deletion for project fails' do
-      error_message = "some error message"
-      project.update_attributes(delete_error: error_message, pending_delete: false)
+    it 'shows error message if deletion for project fails' do
+      project.update_attributes(delete_error: "Something went wrong", pending_delete: false)
 
-      visit namespace_project_path(project.namespace, project)
+      visit project_path(project)
 
       expect(page).to have_selector('.project-deletion-failed-message')
-      expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{error_message}")
-    end
-
-    it 'renders 404 if project was successfully deleted' do
-      worker.perform(project.id, project.owner.id, {})
-
-      visit namespace_project_path(project.namespace, project)
-
-      expect(page).to have_http_status(404)
+      expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{project.delete_error}")
     end
   end
 end
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index a629afe723d..357e09bee95 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -130,30 +130,29 @@ describe Projects::DestroyService, services: true do
         it_behaves_like 'handles errors thrown during async destroy', "Failed to remove project repository"
       end
 
-      context 'when `execute` raises any other error' do
+      context 'when `execute` raises expected error' do
         before do
-          expect_any_instance_of(Projects::DestroyService)
-            .to receive(:execute).and_raise(ArgumentError.new("Other error message"))
+          expect_any_instance_of(Project)
+            .to receive(:destroy!).and_raise(StandardError.new("Other error message"))
         end
 
         it_behaves_like 'handles errors thrown during async destroy', "Other error message"
       end
-    end
-  end
 
-  context 'with execute' do
-    it_behaves_like 'deleting the project with pipeline and build'
+      context 'when `execute` raises unexpected error' do
+        before do
+          expect_any_instance_of(Project)
+            .to receive(:destroy!).and_raise(Exception.new("Other error message"))
+        end
 
-    context 'when `execute` raises an error' do
-      before do
-        expect_any_instance_of(Projects::DestroyService)
-          .to receive(:execute).and_raise(ArgumentError)
-      end
+        it 'allows error to bubble up and rolls back project deletion' do
+          expect do
+            Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+          end.to raise_error
 
-      it 'allows the error to bubble up' do
-        expect do
-          Sidekiq::Testing.inline! { Projects::DestroyService.new(project, user, {}).execute }
-        end.to raise_error(ArgumentError)
+          expect(project.reload.pending_delete).to be(false)
+          expect(project.delete_error).to include("Other error message")
+        end
       end
     end
   end
@@ -182,8 +181,7 @@ describe Projects::DestroyService, services: true do
           expect_any_instance_of(ContainerRepository)
             .to receive(:delete_tags!).and_return(false)
 
-          expect{ destroy_project(project, user) }
-            .to raise_error(ActiveRecord::RecordNotDestroyed)
+          expect(destroy_project(project, user)).to be false
         end
       end
     end
@@ -208,8 +206,7 @@ describe Projects::DestroyService, services: true do
           expect_any_instance_of(ContainerRepository)
             .to receive(:delete_tags!).and_return(false)
 
-          expect { destroy_project(project, user) }
-            .to raise_error(Projects::DestroyService::DestroyError)
+          expect(destroy_project(project, user)).to be false
         end
       end
     end
diff --git a/spec/workers/project_destroy_worker_spec.rb b/spec/workers/project_destroy_worker_spec.rb
index 29f0295de42..f19c9dff941 100644
--- a/spec/workers/project_destroy_worker_spec.rb
+++ b/spec/workers/project_destroy_worker_spec.rb
@@ -21,17 +21,16 @@ describe ProjectDestroyWorker do
       expect(Dir.exist?(path)).to be_truthy
     end
 
-    describe 'when StandardError is raised' do
-      it 'reverts pending_delete attribute with a error message' do
-        allow_any_instance_of(::Projects::DestroyService).to receive(:execute).and_raise(StandardError, "some error message")
+    it 'does not raise error when project could not be found' do
+      expect do
+        subject.perform(-1, project.owner.id, {})
+      end.not_to raise_error
+    end
 
-        expect do
-          subject.perform(project.id, project.owner.id, {})
-        end.to change { project.reload.pending_delete }.from(true).to(false)
-
-        expect(Project.all).to include(project)
-        expect(project.delete_error).to eq("some error message")
-      end
+    it 'does not raise error when user could not be found' do
+      expect do
+        subject.perform(project.id, -1, {})
+      end.not_to raise_error
     end
   end
 end