Surface real server error message on attachment upload failure - #3122
Open
wakqasahmed wants to merge 2 commits into
Open
Surface real server error message on attachment upload failure#3122wakqasahmed wants to merge 2 commits into
wakqasahmed wants to merge 2 commits into
Conversation
…rchidsoftware#2927) Both attach_controller.js and cropper_controller.js swallowed the actual server response and always showed a hardcoded 'file must be an image' / 'File upload error' toast on any upload failure, so a 403 permission error (system.attachment) was mislabeled as a file type error with no useful information for the user. Surface error.response?.data?.message when the server provides one, falling back to the existing generic message otherwise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2927
Problem
When an attachment upload fails (e.g. the user lacks the
system.attachmentpermission and the server responds403 Forbidden), bothattach_controller.jsandcropper_controller.jsignore the actual server response and always show a hardcoded generic message:attach_controller.js: always shows "The attached file must be an image", regardless of the real cause.cropper_controller.js: always shows "File upload error".This mislabels permission/authorization failures as file-type errors, giving users no useful information.
Fix
Both catch handlers now surface
error.response?.data?.message(the message Laravel's exception handler returns in the JSON error body, e.g."Forbidden"for an unhandledabort(403)) when present, falling back to the existing generic message otherwise. This mirrors the response shape already produced byAttachmentController's permission check (checkPermission()->abort(403)).Test plan
src/Platform/Http/Controllers/Controller.php'scheckPermission()that a permission failure results inabort(403), and Laravel's default JSON exception response for this case is{"message": "Forbidden"}.node --checkon both modified files (no test runner/framework configured in this repo for JS controllers — no existing.test.js/.spec.jsfiles or vitest/jest config to extend).system.attachmentfrom a user, attempt an upload via an Attachment/Cropper field, confirm the toast now shows "Forbidden" instead of the file-type message.