FEATURE: Improve handling of backup storage errors

FEATURE: Improve handling of backup storage errors

diff --git a/app/assets/javascripts/admin/models/backup.js.es6 b/app/assets/javascripts/admin/models/backup.js.es6
index 0f66630..b792308 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 0458898..939777f 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 2b4ffd3..9071b06 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 ee951ef..c32cada 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 fbbb18f..84dad56 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

GitHub sha: 6a8007e5

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there: