Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.x]: Ambiguous error message when uploading multiple files #14003

Closed
MoritzLost opened this issue Dec 7, 2023 · 8 comments
Closed

[4.x]: Ambiguous error message when uploading multiple files #14003

MoritzLost opened this issue Dec 7, 2023 · 8 comments
Assignees

Comments

@MoritzLost
Copy link
Contributor

What happened?

Description

When uploading multiple files at once through the assets index, individual uploads sometimes fail. Not sure why this happens, probably due to timeouts when the server is at maximum capacity due to handling a bunch of files. In any case, you only get this error message as a notification:

Uploads failed.

craft-upload-failed

Other than that, there's no indicator that something has gone wrong. If you are uploading a bunch of files in the background while doing something else, you might even miss this notification entirely (since they disappear after a couple of seconds by default, unless you've changed that in the accessibility settings).

The error message is very ambiguous. Has the entire upload failed, or only some of the files? It's the latter, but the error message should indicate that. It also doesn't tell you which upload has failed, so you have to manually compare your local file folder with the uploaded assets to see which one is missing.

Steps to reproduce

  1. Upload a lot of files at once (unfortunately, I don't have a reproducible way to cause an upload to fail).

Expected behavior

The error message should be improved to indicate that some of your files have failed to upload. It should also tell you which file has failed to upload, so you can upload that specific file again.

Actual behavior

The error message is vague and unclear.

Craft CMS version

4.5.11.1

PHP version

8.2

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@i-just
Copy link
Contributor

i-just commented Dec 8, 2023

Hi, thanks for getting in touch! Could you please check the logs (storage/logs/web.log) to see if there are any hints in there as to why some file uploads failed?

@MoritzLost
Copy link
Contributor Author

@i-just I checked that when the error occurred, but there wasn't anything in the logs, unfortunately. But it happened multiple times when I was uploading a bunch of large-ish images at once (in batches of 100~150 files, at about 2~3 MB per image). I got one of those errors in one batch, three in another batch and none in two other batches. I checked the list manually to see which images had failed to upload, and uploaded those again individually. This worked fine, so it's definitely not an issue related to those specific images, but more likely a problem with high server loads / timeouts.

Does it matter what specific problem caused those errors? Regardless of the specific error, the error message needs to be more specific and at least mention which image failed to upload. As far as I can tell, the JS code on the assets page uses a separate request to the assets controller for every file, so it should be possible to catch the failing request and display a more specific error message – right?

@i-just
Copy link
Contributor

i-just commented Dec 8, 2023

Thanks @MoritzLost. Specific error messages help with replication. Based on a quick look, we already have specific error messages that mention the filename if that data is made available to the uploading script, and the fallback is the generic “Upload failed.” message. I’ll have a closer look and keep you updated.

@i-just i-just self-assigned this Dec 8, 2023
@i-just
Copy link
Contributor

i-just commented Dec 11, 2023

Hi @MoritzLost, I had a closer look into it, and the generic “Upload failed.” message is the expected behaviour when there’s no error/reason passed up the chain. I’ve triggered all sorts of errors related to uploading and saving an asset, which all resulted in a descriptive error message being presented to the user. I could only trigger the generic one when a 502 Bad Gateway was thrown.

Given all that, if you were to open developer tools in your browser before you start uploading multiple assets, you can follow the progress in the Network tab. If it happens again, you can then see what actually caused it (and which files didn’t upload). If you spot anything that you think should be passable up the chain, let me know, and I’ll have another look, but at the moment, it does look to be working as expected.

I’m going to close this now, but feel free to reply if you find further info that could mean that this is a bug.

@i-just i-just closed this as completed Dec 11, 2023
@MoritzLost
Copy link
Contributor Author

MoritzLost commented Dec 11, 2023

@i-just Thanks for looking into this issue, Iwona!

To test this issue, I've uploaded 600 images at once now to try to trigger the error. After about half, I've got this error again. Looks like a network timeout, network inspector shows NS_ERROR_NET_INTERRUPT with no response body. So this probably happens if the server is too busy to respond in time, so there's no error message from the server to display.

In the event of a network error like this, it would be nice to know which file failed to upload. I can see in the network tab that each image uses an individual request, and the request body includes the file name. I don't know how the JS code is structured, but in theory it should be possible to use the filename from the request body as a last fallback for the error message, in case of a network error where there's no other message to display. Right?

PS: An easy way to simulate this error is to block the URL for the asset upload through the dev tools. In Firefox:

  1. Upload a single image with the network tab open.
  2. Right-click on the request -> Block URL
  3. Upload another image. This should fail immediately and display the generic error message.

The failure reason displayed in the dev tools is different, but to the JS code it should look identical to a network error.

@i-just
Copy link
Contributor

i-just commented Dec 11, 2023

@MoritzLost, thanks for the suggestions! I found a way to improve those fallback error messages to contain the filename. PR has been raised and is pending review.

@MoritzLost
Copy link
Contributor Author

@i-just That looks great, thanks Iwona!

@brandonkelly
Copy link
Member

Craft 4.5.12 is out with the fix for this. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants