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

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

Open
2 tasks done
Tokov opened this issue Feb 9, 2023 · 12 comments
Assignees
Labels
AWS S3 Plugin that handles uploads to Amazon AWS S3 Bug

Comments

@Tokov
Copy link

Tokov commented Feb 9, 2023

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

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
Copy link
Contributor

arturi commented Feb 14, 2023

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
Copy link

sophisma commented Feb 14, 2023

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
Copy link
Contributor

arturi commented Feb 14, 2023

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
Copy link

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

@Tokov
Copy link
Author

Tokov commented Feb 14, 2023

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
Copy link
Member

Acconut commented Feb 14, 2023

@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
Copy link
Author

Tokov commented Feb 15, 2023

@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
Copy link

@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
Copy link
Member

Acconut commented Feb 15, 2023

@sophisma Glad to hear that it helped :)

@Murderlon
Copy link
Member

Seems like the issues were resolved. Closing this :)

@Murderlon Murderlon closed this as not planned Won't fix, can't repro, duplicate, stale Feb 27, 2023
@Tokov
Copy link
Author

Tokov commented Feb 27, 2023

@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 arturi reopened this Feb 27, 2023
@arturi
Copy link
Contributor

arturi commented Feb 27, 2023

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

@arturi arturi added the AWS S3 Plugin that handles uploads to Amazon AWS S3 label Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AWS S3 Plugin that handles uploads to Amazon AWS S3 Bug
Projects
None yet
Development

No branches or pull requests

6 participants