Improve file type restriction error messages to include rejected file names

[benjiroArthur]

Initial checklist

• I understand this is a feature request and questions should be posted in the Community Forum
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

When uploading multiple files, users have no clear indication of which file(s) caused the error. This makes troubleshooting frustrating, especially in bulk upload scenarios.

Solution

Update the error message to include the rejected file name(s). For example:
'document.exe' is not supported. You can only upload: .png, .jpg

This makes it much easier for users to identify and correct problematic files during batch uploads.

Alternatives

None.

[russellcain]

hi @benjiroArthur! I took a look at implementing your proposed solution tonight and have the following thoughts/notes for the Uppy folks (the first of which is: A+ job on the contributing docs; you've made it super easy to jump in and make changes so kudos for that).

• When uploading multiple files, it looks like errors are thrown as they are encountered, preventing the validation/processing of the next one in the batch. Whereas I agree that your change will go a long way in helping to illuminate which file is holding up the process, I would posit that a slight redesign of collecting the RestrictionErrors and finding a way by which to display them wholesale would be a better user experience.
  • I only bring this up as your comment mentioned "rejected file name(s)" so wanted to clarify that a direct extensions of the error handling would return back at most one file name per batch upload.
• I went ahead and coded up the change in a way that I thought seemed fit:
  • I extended the file type validation block to include the localized string which (in English) reads: Failed to upload {fileName} and used it to prefix the localized string for the file type issue. (see snippet)

const localizedFailToUploadMsg = this.getI18n()('failedToUpload', {
          file: `${file.name}`
        })
        const localizedUnsupportedFileTypeMsg = this.getI18n()('youCanOnlyUploadFileTypes', {
          types: allowedFileTypesString,
        })
        throw new RestrictionError(`${localizedFailToUploadMsg}: ${localizedUnsupportedFileTypeMsg}`,
          { file } as { file: UppyFile<M, B> },
        )

• I think this solution has a few shortcomings:
  • It overloads the idea of reporting back a failed validation check, which is meant to happen on a characteristic of the file vs being an overall check on the file itself.
    • Again, this is the idea of collecting all the errors in the upload transaction to provide the user with all the feedback they need in one go so their second upload can be a success.
  • It isn't consistent with other Validation Errors:
    • I had to take a step back from my implementation when I was writing up some tests and noticed that the localized string for the file size follows a different pattern:

this.getI18n()('exceedsSize', {
          size: prettierBytes(maxFileSize),
          file: file.name ?? this.getI18n()('unnamed'),
        }),

which would generate something along the lines of {fileName} exceeds maximum allowed size of {sizeInBytes}. This made me think that it might be best to respond with a comment and see how the primary maintainers of Uppy would like to proceed.

To summarize:

• I'd be in favor of an extension on generating error messages in general which, as a default, prefixes the localized string indicating an error with the fileName, similar to my implementation above (not to sound too too biased).
• I'd also be in favor of introducing a larger notion of transactionality in the validation of an upload, even for an individual file:
  • i.e. "Wow you uploaded a file which isn't the right type and, for future reference, is too big even if it were the right type." is a more helpful bit of feedback. I do see how this, again, dilutes the intention of validation checks being direct and a "fail early" approach. No hard preference on overall design choice, just an option that would extend and lend itself well to the batch offering.
• If the above two ideas are too large of a fix at this time, my proposed PR can be found here: feat: include the filename in the file type error message #5933.
  • For the sake of consistency, I would argue that my PR should be shot down and replaced by a larger one which updates the localized text of the file extension error message to keep it a bit more similar to the 'exceedsSize' one.

Let me know if I garbled up any of the thoughts above -- thanks for letting me take a crack at it!

[russellcain]

hi @benjiroArthur - just checking in to see if you have thoughts on the above. thanks!

[benjiroArthur]

Hi @russellcain I like your solution, I earlier opened a similar PR and one of the issues raised was handling long filenames. PR-5787

[nx-benjamin-arthur]

Hi, @russellcain I checked your PR and realized the issue of long file name is still not addressed, do you plan on doing that anytime soon?