From 6a8007e5fbe218eb507cf1a45af5e23d4dbafe8e Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Wed, 20 Feb 2019 15:15:38 +0100 Subject: [PATCH] FEATURE: Improve handling of backup storage errors --- .../javascripts/admin/models/backup.js.es6 | 14 +++++++++++--- .../discourse/components/backup-uploader.js.es6 | 11 +++++------ app/controllers/admin/backups_controller.rb | 16 +++++++++------- config/locales/client.en.yml | 1 + lib/backup_restore/s3_backup_store.rb | 3 +++ 5 files changed, 29 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/admin/models/backup.js.es6 b/app/assets/javascripts/admin/models/backup.js.es6 index 0f6663065dc..b79230829e5 100644 --- a/app/assets/javascripts/admin/models/backup.js.es6 +++ b/app/assets/javascripts/admin/models/backup.js.es6 @@ -1,4 +1,5 @@ import { ajax } from "discourse/lib/ajax"; +import { extractError } from "discourse/lib/ajax-error"; const Backup = Discourse.Model.extend({ destroy() { @@ -15,9 +16,16 @@ const Backup = Discourse.Model.extend({ Backup.reopenClass({ find() { - return ajax("/admin/backups.json").then(backups => - backups.map(backup => Backup.create(backup)) - ); + return ajax("/admin/backups.json") + .then(backups => backups.map(backup => Backup.create(backup))) + .catch(error => { + bootbox.alert( + I18n.t("admin.backups.backup_storage_error", { + error_message: extractError(error) + }) + ); + return []; + }); }, start(withUploads) { diff --git a/app/assets/javascripts/discourse/components/backup-uploader.js.es6 b/app/assets/javascripts/discourse/components/backup-uploader.js.es6 index 04588987bef..939777f0a7f 100644 --- a/app/assets/javascripts/discourse/components/backup-uploader.js.es6 +++ b/app/assets/javascripts/discourse/components/backup-uploader.js.es6 @@ -1,4 +1,5 @@ import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; import computed from "ember-addons/ember-computed-decorators"; import UploadMixin from "discourse/mixins/upload"; @@ -39,14 +40,12 @@ export default Ember.Component.extend(UploadMixin, { $upload.on("fileuploadadd", (e, data) => { ajax("/admin/backups/upload_url", { data: { filename: data.files[0].name } - }).then(result => { - if (!result.success) { - bootbox.alert(result.message); - } else { + }) + .then(result => { data.url = result.url; data.submit(); - } - }); + }) + .catch(popupAjaxError); }); }.on("didInsertElement") }); diff --git a/app/controllers/admin/backups_controller.rb b/app/controllers/admin/backups_controller.rb index 2b4ffd3512f..9071b068e66 100644 --- a/app/controllers/admin/backups_controller.rb +++ b/app/controllers/admin/backups_controller.rb @@ -37,7 +37,7 @@ class Admin::BackupsController < Admin::AdminController } BackupRestore.backup!(current_user.id, opts) rescue BackupRestore::OperationRunningError - render json: failed_json.merge(message: I18n.t("backup.operation_already_running")) + render_error("backup.operation_already_running") else StaffActionLogger.new(current_user).log_backup_create render json: success_json @@ -46,7 +46,7 @@ class Admin::BackupsController < Admin::AdminController def cancel BackupRestore.cancel! rescue BackupRestore::OperationRunningError - render json: failed_json.merge(message: I18n.t("backup.operation_already_running")) + render_error("backup.operation_already_running") else render json: success_json end @@ -117,7 +117,7 @@ class Admin::BackupsController < Admin::AdminController SiteSetting.set_and_log(:disable_emails, 'yes', current_user) BackupRestore.restore!(current_user.id, opts) rescue BackupRestore::OperationRunningError - render json: failed_json.merge(message: I18n.t("backup.operation_already_running")) + render_error("backup.operation_already_running") else render json: success_json end @@ -125,7 +125,7 @@ class Admin::BackupsController < Admin::AdminController def rollback BackupRestore.rollback! rescue BackupRestore::OperationRunningError - render json: failed_json.merge(message: I18n.t("backup.operation_already_running")) + render_error("backup.operation_already_running") else render json: success_json end @@ -192,15 +192,17 @@ class Admin::BackupsController < Admin::AdminController params.require(:filename) filename = params.fetch(:filename) - return render_error("backup.backup_file_should_be_tar_gz") unless valid_extension?(filename) - return render_error("backup.invalid_filename") unless valid_filename?(filename) + return render_json_error(I18n.t("backup.backup_file_should_be_tar_gz")) unless valid_extension?(filename) + return render_json_error(I18n.t("backup.invalid_filename")) unless valid_filename?(filename) store = BackupRestore::BackupStore.create begin upload_url = store.generate_upload_url(filename) rescue BackupRestore::BackupStore::BackupFileExists - return render_error("backup.file_exists") + return render_json_error(I18n("backup.file_exists")) + rescue BackupRestore::BackupStore::StorageError => e + return render_json_error(e) end render json: success_json.merge(url: upload_url) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ee951efbbe7..c32cada067a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3276,6 +3276,7 @@ en: location: local: "Local Storage" s3: "S3" + backup_storage_error: "Failed to access backup storage: %{error_message}" export_csv: success: "Export initiated, you will be notified via message when the process is complete." diff --git a/lib/backup_restore/s3_backup_store.rb b/lib/backup_restore/s3_backup_store.rb index fbbb18f1d75..84dad56ec67 100644 --- a/lib/backup_restore/s3_backup_store.rb +++ b/lib/backup_restore/s3_backup_store.rb @@ -49,6 +49,9 @@ module BackupRestore ensure_cors! presigned_url(obj, :put, UPLOAD_URL_EXPIRES_AFTER_SECONDS) + rescue Aws::Errors::ServiceError => e + Rails.logger.warn("Failed to generate upload URL for S3: #{e.message.presence || e.class.name}") + raise StorageError end private