transloadit / uppy

The next open source file uploader for web browsers :dog:
https://uppy.io
MIT License
29.17k stars 2.01k forks source link

add way to reset file progress so that the multipart plugin generates a new Upload Id on next upload attempt #4313

Open Tokov opened 1 year ago

Tokov commented 1 year ago

Initial checklist

Problem

Scenario: Start multipart upload. All parts successful. Upload completion successful. Post-processor plugin is called. Unexpected network/resource error occurs in post-processor plugin. Current state of file: completed true, percentage 100%

If the user were to click on the retry button at this time, failure would be certain because the file has an upload Id corresponding to a completed upload. Put another way, the upload id is no longer valid because all parts were uploaded and the completion step was successful.

In a previous version of Uppy, my workaround was this: In the 'upload-retry' event, I removed the file and re-added the file. This caused the state of the file to reset.

uppy.removeFile(file.id); uppy.addFile(file);

The multipart uploader would detect that this file had no upload id and it would start the upload process from the very beginning by generating a new Upload via the "/multipart" companion endpoint.

Solution

I need some way to reset the file state, so that the next upload attempt for that specified file, starts from the beginning (new upload id generated).

Alternatives

My previous workaround (might have been version 2.x) of calling the following, in order to completely reset the state of a file's progress, no longer works:

uppy.removeFile(file.id); uppy.addFile(file);

Fast forward to today, and after updating to version 3.4.0, this workaround no longer works. In the multipart plugin's cache, the completed upload id is still there, so a new one will not be generated. I haven't been successful on figuring out way to clear it so the retry option has a chance of succeeding.

arturi commented 1 year ago

Would calling uppy.cancelAll() work for you? Alternatively, try something like this:

uppy.setFileState(file.id, {
    progress: {
      bytesTotal: 0,
      bytesUploaded: 0,
      percentage: 0,
      postprocess: null,
      uploadComplete: false,
      uploadStarted: null
    }
})

But I wonder how we can fix this properly in Uppy. What post-processor are you using?

sophisma commented 1 year ago

I'm running into an issue and I think it might be related to this. I'm doing a proof of concept of running a tusdotnet server behind a public api, that api is being called by a web application with Uppy. The entire flow is:

Web Application (Uppy) -> Public API -> tusdotnet Server

The first time I upload a file it transfers without any issues. When it finishes successfully, on the server side, the file is moved to a new folder and the rest of the files are deleted. If I try to reupload the same file I'm getting the error: [Uppy] [17:46:05] Failed to upload Cool Timelapse.mp4 tus: invalid or missing offset value, originated from request (method: HEAD, url: https://localhost:44330/files/3fa34cc256704ae98649012f4a8defd7, response code: 200, response text: , request id: n/a)

After a bit of digging I found out that on the browser development tools under Application->Local Storage there's a Key/Value pair:

Key - tus::tus-uppy-cool/timelapse/mp4-10-1e-video/mp4-68205978-1598562519041-https://localhost:44330/files/::945045104226 Value - {"size":68205978,"metadata":{"relativePath":null,"name":"Cool Timelapse.mp4","type":"video/mp4","filetype":"video/mp4","filename":"Cool Timelapse.mp4"},"creationTime":"Tue Feb 14 2023 17:32:40 GMT+0000 (Hora padrão da Europa Ocidental)","uploadUrl":"https://localhost:44330/files/3fa34cc256704ae98649012f4a8defd7"}

If I delete it I can reupload again. Is there a way to delete this Key/Value pair after each successful upload?

arturi commented 1 year ago

Is there a way to delete this Key/Value pair after each successful upload?

Yes, you can set removeFingerprintOnSuccess: true in @uppy/tus options. There’s also urlStorage option, which might work better for your use case?

@Acconut I’m thinking the upload probably shouldn’t it fail when a file from the fingerprint is not found?

sophisma commented 1 year ago

The removeFingerprintOnSuccess: true did the trick, thank you. I'll look a bit more on how I can use urlStorage,

Tokov commented 1 year ago

I'm using a custom post-processor that extends BasePlugin.

I tried out the setFileState suggestion but it didn't help.

Also, calling any method that affects more than one specific file, like cancelAll isn't something I can do because I can have more than one file uploading at a time. Having said that, I did try it to see what it would do, and it didn't work - it caused the file to be removed from the dashboard.

I'm actually seeing two issues, and they might be related. Issue #1: (S3 Upload ID) stale data in AWS multipart uploader Issue #2: (Uppy Upload ID) Stale data in Core Uppy.js - see step 2.3 below.

1.1) Upload file. 1.2) All parts upload. Completion step runs successfully. 1.3) Custom post-processor runs after file is completed. Force error in post-processor. Error is set in file and the retry icon appears. 1.4) Click on retry icon. 1.5) When the Multipart Uploader's getUploadId(file, signal) is called, the completed S3 upload ID is still cached, and attempts to re-use it, which can't work because it's no longer valid.

Acconut commented 1 year ago

@Acconut I’m thinking the upload probably shouldn’t it fail when a file from the fingerprint is not found?

Yes, you are correct. But in this case, it seems as if the server returns a weird response. Take a look at this error message:

invalid or missing offset value, originated from request (method: HEAD, url: https://localhost:44330/files/3fa34cc256704ae98649012f4a8defd7, response code: 200, response text: , request id: n/a)

The server responds with a 200, even though the file has been deleted, according to @sophisma. Apparently this response does not include the Upload-Offset value and therefore tus-js-client errors out. Either this is a bug in your API, @sophisma, or in tusdotnet. Make sure that there are not 200 returned for deleted files and then the entries in local storage will be cleaned up by tus-js-client automatically.

Tokov commented 1 year ago

@arturi

I think this simple change could fix the problem: In s3 multipart uploader, if completeMultipartUpload successfully runs, clear out the cached S3 upload id. There's no reason to keep it anymore.

sophisma commented 1 year ago

@Acconut I’m thinking the upload probably shouldn’t it fail when a file from the fingerprint is not found?

Yes, you are correct. But in this case, it seems as if the server returns a weird response. Take a look at this error message:

invalid or missing offset value, originated from request (method: HEAD, url: https://localhost:44330/files/3fa34cc256704ae98649012f4a8defd7, response code: 200, response text: , request id: n/a)

The server responds with a 200, even though the file has been deleted, according to @sophisma. Apparently this response does not include the Upload-Offset value and therefore tus-js-client errors out. Either this is a bug in your API, @sophisma, or in tusdotnet. Make sure that there are not 200 returned for deleted files and then the entries in local storage will be cleaned up by tus-js-client automatically.

@Acconut you were absolutely right, my API had a bug and was returning 200 all the times, instead of returning the tusdotnet status code. After fixing it it's working fine, even without using removeFingerprintOnSuccess: true. I've noticed that now I can't see any fingerprint on the browser's local storage, but everything is working fine, the files still resume if I close the browser during an upload and send the same file again. Good call, thanks!

Acconut commented 1 year ago

@sophisma Glad to hear that it helped :)

Murderlon commented 1 year ago

Seems like the issues were resolved. Closing this :)

Tokov commented 1 year ago

@Murderlon

The issue was not resolved. The fingerprint was a separate issue. The problem that this ticket covers is that the S3 upload ID is not cleared from uploader cache after the complete step is successfully executed.

See my last comment from 2 weeks ago

arturi commented 1 year ago

Yes, this was a misunderstanding. @aduh95 will have a look into this, when we’ll be working on the aws-multipart refactor.