From 744be7771c1607273ac378a120d958628e6e9f7d Mon Sep 17 00:00:00 2001 From: Sean Abrahams Date: Tue, 7 Nov 2023 12:53:39 -0800 Subject: [PATCH 1/2] [Fix #49104] Add server response (xhr) to direct-upload:error event. ActiveStorage's javascript does not provide access to the server response for error messaging when an error occurs during a direct upload. This is a problem because we cannot distinguish between an upload failing due to authorization or the file being too large or any other validation error. This change updates the error event to include the `xhr` object so that the server response is accessible to error handling code. This allows us to display the specific issues that caused the upload to be rejected such as: - You must be an admin to upload this file. - File must be less than 100MB. - Uploading .exe files is not permitted. --- activestorage/CHANGELOG.md | 29 +++++++++++++++++++ .../assets/javascripts/activestorage.esm.js | 13 +++++++-- .../app/assets/javascripts/activestorage.js | 13 +++++++-- .../javascript/activestorage/blob_record.js | 5 +++- .../javascript/activestorage/blob_upload.js | 5 +++- .../activestorage/direct_upload_controller.js | 2 +- 6 files changed, 58 insertions(+), 9 deletions(-) diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 6704fd6ace9ff..b6253ead059fc 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,32 @@ +* Add `xhr` object to direct-upload:error event so that server generated error messages are accessible. + +Before: +```javascript +addEventListener("direct-upload:error", event => { + event.preventDefault() + const { id, error } = event.detail + const element = document.getElementById(`direct-upload-${id}`) + element.classList.add("direct-upload--error") + element.setAttribute("title", error) +}) +``` + +After: +```javascript +addEventListener("direct-upload:error", event => { + event.preventDefault() + const { id, error, xhr } = event.detail + const element = document.getElementById(`direct-upload-${id}`) + const errorMessage = xhr.response['error'] // Example: File size must be less than 100MB + element.classList.add("direct-upload--error") + element.setAttribute("title", errorMessage) +}) +``` + + *Sean Abrahams* + +Fixes #49104 + * Allow accepting `service` as a proc as well in `has_one_attached` and `has_many_attached`. *Yogesh Khater* diff --git a/activestorage/app/assets/javascripts/activestorage.esm.js b/activestorage/app/assets/javascripts/activestorage.esm.js index 0bdc87f7b4199..6c5c16a1dcf6e 100644 --- a/activestorage/app/assets/javascripts/activestorage.esm.js +++ b/activestorage/app/assets/javascripts/activestorage.esm.js @@ -562,7 +562,10 @@ class BlobRecord { } } requestDidError(event) { - this.callback(`Error creating Blob for "${this.file.name}". Status: ${this.status}`); + this.callback({ + xhr: this.xhr, + message: `Error creating Blob for "${this.file.name}". Status: ${this.status}` + }); } toJSON() { const result = {}; @@ -600,7 +603,10 @@ class BlobUpload { } } requestDidError(event) { - this.callback(`Error storing "${this.file.name}". Status: ${this.xhr.status}`); + this.callback({ + xhr: this.xhr, + message: `Error storing "${this.file.name}". Status: ${this.xhr.status}` + }); } } @@ -691,7 +697,8 @@ class DirectUploadController { } dispatchError(error) { const event = this.dispatch("error", { - error: error + error: error.message, + xhr: error.xhr }); if (!event.defaultPrevented) { alert(error); diff --git a/activestorage/app/assets/javascripts/activestorage.js b/activestorage/app/assets/javascripts/activestorage.js index 24a3fc84307fa..a397d0001390b 100644 --- a/activestorage/app/assets/javascripts/activestorage.js +++ b/activestorage/app/assets/javascripts/activestorage.js @@ -557,7 +557,10 @@ } } requestDidError(event) { - this.callback(`Error creating Blob for "${this.file.name}". Status: ${this.status}`); + this.callback({ + xhr: this.xhr, + message: `Error creating Blob for "${this.file.name}". Status: ${this.status}` + }); } toJSON() { const result = {}; @@ -594,7 +597,10 @@ } } requestDidError(event) { - this.callback(`Error storing "${this.file.name}". Status: ${this.xhr.status}`); + this.callback({ + xhr: this.xhr, + message: `Error storing "${this.file.name}". Status: ${this.xhr.status}` + }); } } let id = 0; @@ -681,7 +687,8 @@ } dispatchError(error) { const event = this.dispatch("error", { - error: error + error: error.message, + xhr: error.xhr }); if (!event.defaultPrevented) { alert(error); diff --git a/activestorage/app/javascript/activestorage/blob_record.js b/activestorage/app/javascript/activestorage/blob_record.js index f017de703bfcc..ce1c7667bc0fc 100644 --- a/activestorage/app/javascript/activestorage/blob_record.js +++ b/activestorage/app/javascript/activestorage/blob_record.js @@ -63,7 +63,10 @@ export class BlobRecord { } requestDidError(event) { - this.callback(`Error creating Blob for "${this.file.name}". Status: ${this.status}`) + this.callback({ + xhr: this.xhr, + message: `Error creating Blob for "${this.file.name}". Status: ${this.status}` + }) } toJSON() { diff --git a/activestorage/app/javascript/activestorage/blob_upload.js b/activestorage/app/javascript/activestorage/blob_upload.js index 277cc8ff8e0e2..d668545e38f62 100644 --- a/activestorage/app/javascript/activestorage/blob_upload.js +++ b/activestorage/app/javascript/activestorage/blob_upload.js @@ -30,6 +30,9 @@ export class BlobUpload { } requestDidError(event) { - this.callback(`Error storing "${this.file.name}". Status: ${this.xhr.status}`) + this.callback({ + xhr: this.xhr, + message: `Error storing "${this.file.name}". Status: ${this.xhr.status}` + }) } } diff --git a/activestorage/app/javascript/activestorage/direct_upload_controller.js b/activestorage/app/javascript/activestorage/direct_upload_controller.js index 987050889a750..d6db746bb8e99 100644 --- a/activestorage/app/javascript/activestorage/direct_upload_controller.js +++ b/activestorage/app/javascript/activestorage/direct_upload_controller.js @@ -48,7 +48,7 @@ export class DirectUploadController { } dispatchError(error) { - const event = this.dispatch("error", { error }) + const event = this.dispatch("error", { error: error.message, xhr: error.xhr }) if (!event.defaultPrevented) { alert(error) } From 4937ad332f831b151ef6755bc3ee4b0c76f11169 Mon Sep 17 00:00:00 2001 From: Sean Abrahams Date: Tue, 7 Nov 2023 13:05:02 -0800 Subject: [PATCH 2/2] Resolve lint errors. --- activestorage/CHANGELOG.md | 48 +++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index b6253ead059fc..fc57339824878 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,31 +1,31 @@ -* Add `xhr` object to direct-upload:error event so that server generated error messages are accessible. +* Add `xhr` object to direct-upload:error event so that server generated error messages are accessible. -Before: -```javascript -addEventListener("direct-upload:error", event => { - event.preventDefault() - const { id, error } = event.detail - const element = document.getElementById(`direct-upload-${id}`) - element.classList.add("direct-upload--error") - element.setAttribute("title", error) -}) -``` + Before: + ```javascript + addEventListener("direct-upload:error", event => { + event.preventDefault() + const { id, error } = event.detail + const element = document.getElementById(`direct-upload-${id}`) + element.classList.add("direct-upload--error") + element.setAttribute("title", error) + }) + ``` -After: -```javascript -addEventListener("direct-upload:error", event => { - event.preventDefault() - const { id, error, xhr } = event.detail - const element = document.getElementById(`direct-upload-${id}`) - const errorMessage = xhr.response['error'] // Example: File size must be less than 100MB - element.classList.add("direct-upload--error") - element.setAttribute("title", errorMessage) -}) -``` + After: + ```javascript + addEventListener("direct-upload:error", event => { + event.preventDefault() + const { id, error, xhr } = event.detail + const element = document.getElementById(`direct-upload-${id}`) + const errorMessage = xhr.response['error'] // Example: File size must be less than 100MB + element.classList.add("direct-upload--error") + element.setAttribute("title", errorMessage) + }) + ``` - *Sean Abrahams* + Fixes #49104 -Fixes #49104 + *Sean Abrahams* * Allow accepting `service` as a proc as well in `has_one_attached` and `has_many_attached`.