Uppy + Golden Retriever + aws-s3: completed files handled incorrectly on restore

[binairo]

I'm using the aws-s3 plugin and trying to get Golden Retriever to work with a custom interface.

    "@uppy/aws-s3": "4.3.2",
    "@uppy/core": "4.5.3",
    "@uppy/golden-retriever": "^4.2.3",

The issue is that Uppy and Golden Retriever do not treat completed files as completed.

To reproduce:

• upload a bunch of files
• refresh the page when at least one file has completed
• retriever restores the state
• uploaded files can be marked as isGhost = true
• emit restore-confirmed to continue upload
• uppy restores the upload and start uploading
• the COMPLETED files are also restarted
• the getUploadParameters/createMultipartUpload is called
• completed files are uploaded again

Expected behavior

I would expect the files to be skipped completely when already completed (progress.uploadComplete = true). And not to have to re-select them because they are 'ghosts'.

Actual behavior

Currently they can be marked as isGhost = true which is annoying because now the user has to reselect files that have already been completed.

Additionally, the files are actually treated as completely new files and are uploaded again. That leaves me two equally useless options:

• Recycle the existing target (url/multipart). This breaks for multipart when it tries to re-upload already uploaded parts
• Restart the files completely. This completely voids the use of Golden Retriever for completed files and leaves me with a lot of extra uploaded/orphaned files that need to be cleaned up.

Possible solution

This could be handled at the #runUpload stage: skip files that have already completed
But also don't mark completed files as isGhost because that's just unnecessary.

There is one catch I can think of: if the file is needed in a postprocessing step it will have to be retrieved from wherever it was uploaded (I don't use postprocessors so not sure about the exact usage). But maybe that is similar to remote files? If you want to postprocess those you need them locally too somehow.

[binairo]

Ok I've worked around this in a way that's quite specific to my usecase, but I'm sharing just in case it's useful for anyone.

isGhost for completed files issue

To prevent the user having to re-select files that have already been completed, I've added the following code to my restored event handler:

uppy.getFiles().forEach((file) => {
  // uploadComplete means completed so no need to re-select
  if (file.progress.uploadComplete && file.isGhost) {
    uppy.log(`${file.name} was completely uploaded but was marked as ghost 👻`)
    uppy.setFileState(file.id, { isGhost: false })
  }
})

Restarting completed files issue

Since I cannot touch the #runUpload code or restore function and obviously I don't want to rewrite the whole thing, I've come up with a workaround that basically removes the completed files from the uploads before restoring.

These are (roughly) the steps I've added to my custom handleRestoreConfirmed handler (see #5929):

• get all files and currentUploads from Uppy's state
• mark all files isRestored: false (not relevant anymore)
  • I use isRestored in my interface and after confirming I want this state reset
  • I think it makes sense to reset in case the upload is interrupted again
• for each upload in currentUploads: remove all files that are completed from that upload
  • this makes sure these files are not restarted
• emit a complete event with a dummy uploadId that contains all completed files in successful
  • this.uppy.emit('complete', { successful: completedFiles, failed: [], uploadID: nanoid(), })
  • this will allow GoldenRetriever to cleanup the blobs for completed files
  • technically this might contain files that have already been included in a complete event before but for me that's not a problem
• for each of the currentUploads, run uppy.restore(uploadId)
• emit complete for each restore result (see Golden retriever restore does not emit 'complete' event when all files have uploaded #5929)

I hope this can be of use to someone.

[binairo]

In addition to the way completed files are handled in uploads that are in progress, I discovered that retriever does not save files in localStorage that are completed if they are not in the currently active 'upload'.

In saveFilesStateToLocalStorage, allFiles = [...waitingFiles, ...uploadingFiles] means that files in an upload that was completed are not considered part of allFiles.

I can see where this is coming from (especially since retriever doesn't handle completed files well), but I think that if the developer did not clean up completed files from the files list in Uppy yet, they have a reason (for instance: I want to show completed files until all have been completed) and retriever shouldn't decide that those files are not worth restoring.

This clashes especially with the way I try to work around the main issue in the first place (by moving all completed files to a dummy completed upload).

In an increasingly hackish workaround, I now set progress.uploadStarted: 0 for all completed files on restore. This will still keep them as completed (uploadComplete === true) but retriever will see them as 'waitingFiles' when serializing the state to localStorage.