tus / tus-js-client

A pure JavaScript client for the tus resumable upload protocol
https://tus.io/
MIT License
2.12k stars 316 forks source link

Interface for storing additional data in web storage #726

Open perotom opened 2 months ago

perotom commented 2 months ago

Is your feature request related to a problem? Please describe. When using services like bunny.net, we need to send additional headers (Authorization, ...) for every request. Those headers are typically generated on the backend as the secret API key is needed. Right now, we use the onBeforeRequest to add those headers after we received the headers in onAfterResponse. Everything works well but not with resumable uploads as the library only saved the upload URL but not the headers needed to successfully upload.

Describe the solution you'd like Give a way to add additional data to the stored upload URL. One way would be to pass the storage identifier (tus::XXXXXXX:XXXX) to callbacks like onAfterResponse and expose functions like update in WebStorageUrlStorage and give user access to this class.

Describe alternatives you've considered One way is to use an alternative fingerprint function and do those changes via localStorage without knowledge of the library.

Can you provide help with implementing this feature? Happy to help.

Additional context

Acconut commented 2 months ago

For authorization headers it might be more appropriate to also fetch them from the server before resuming an upload. Then you can ensure that the current user is still allowed to perform this action and that the authorization tokens are still valid. The last point is especially import if a user resumes an upload after a longer duration.

Does that make sense?

perotom commented 2 months ago

The token expires anyways after a given time so I won't think thats a huge problem. Anyways I see your point.

perotom commented 2 months ago

One thing I noted: To be able to try to fetch the headers again from the server, we would need the fingerprint (better would be the id of the server object) available in PreviousUpload in upload.findPreviousUploads. Is it possible to store this?

Even better would be to give the option to store a custom id to the metadata in storage on onAfterResponse: function (req: tus.HttpRequest, res: tus.HttpResponse).

Acconut commented 2 months ago

I'm a bit surprised that you would need the fingerprint for fetching the headers. The fingerprint is a client-side value that has no meaning to the server usually.

In tus, the upload ID is the upload URL, which is part of PreviousUpload (see https://github.com/tus/tus-js-client/blob/bb8862e139b04aee59336c00a2f0806c1719c201/lib/upload.js#L166). We forgot to add this value to the documentation, but you can use it. I'll address this shortly :)

perotom commented 2 months ago

Yes, we use a custom fingerprint method to store it on the server as well to identify the upload object server-side.

Ok, I see, so we would need to search for the given upload URL which needs to be a unique identifier. If every provider fulfils this constraint, this would be a valid approach!

Acconut commented 2 months ago

Yes, the upload URL should be unique, but its exact format depends on the used server.

perotom commented 2 months ago

Ok thank you, then it is a good solution.

perotom commented 1 month ago

I got another question about resumable upload and storing upload URL: As far as I understood, resumable uploads are stored in web storage. Now we have the following issue:

User uploads file but closes browser at 50%. He tries on another device or browser to upload the other 50%. The resumable information is not stored in web storage. Our API will return the previously used upload URL (as described above). Now tus tries to upload the whole file and the server returns a 409 Conflict status.

Shouldn't tus somehow have a check before starting uploading if the file was already partially uploaded?

Acconut commented 1 month ago

We forgot to add this value to the documentation, but you can use it. I'll address this shortly :)

Done in https://github.com/tus/tus-js-client/commit/ad687710d7169ebe55744588fb8067be7a82e871

User uploads file but closes browser at 50%. He tries on another device or browser to upload the other 50%. The resumable information is not stored in web storage. Our API will return the previously used upload URL (as described above). Now tus tries to upload the whole file and the server returns a 409 Conflict status.

Shouldn't tus somehow have a check before starting uploading if the file was already partially uploaded?

That's not how resuming an upload is intended to work with tus. If a tus client creates a new upload resource, the client expects to start a fresh upload and not resume a previously started uploads. This is why tus-js-client does not send a HEAD request after a POST request.

Most applications do not need to resume uploads across devices, which is why tus-js-client by default stores the resumption information locally on the device. If you want to resume uploads on different devices, I would recommend you to implement an API endpoint on your server to fetch this resumption information whenever a user wants to upload a file. If the server already created an upload for this file, tus-js-client can resume the upload. If not, then tus-js-client can create a new upload. I hope this makes sense.

perotom commented 1 month ago

Thanks for the information. I thought as the server keeps state, it would be benefitical for the client to make use of this state.

Right now, we had to actually delete the partial data on the server to let the client resume upload (which is exactly the opposit effect of resumable uploads). Especially this affects users who are using privat browsing or use multiple web browsers.

An improvement could be that before the client tries to upload to the given URL, it should check if there is already partial data available and set the corresponding offset header. This would be backward compatible and avoids mentioned problems above. Just a thought.

Acconut commented 1 month ago

An improvement could be that before the client tries to upload to the given URL, it should check if there is already partial data available and set the corresponding offset header. This would be backward compatible and avoids mentioned problems above. Just a thought.

This would incur an additional HEAD request for most tus-js-client users, who just create "normal" uploads (i.e. not redirect to an existing resource). Due to the performance penalty, we must avoid such behavior in a standard configuration.

tus-js-client should retry the upload on 409 by sending a HEAD request, so I would expect the upload to finish nevertheless. Is this not the case for you?

We could consider optimizing this scenario even. If tus-js-client gets a 409 with an Upload-Offset header, it can use the new offset without sending a HEAD request first.

perotom commented 1 month ago

I was getting a 409 conflict response because it started uploading from the start. I guess it would be a good idea to handle this conflict situation like you described. This would give the mentioned benefits above!

Acconut commented 1 month ago

If tus-js-client gets a 409 (and you didn't disable retries), it should resume the upload by sending a HEAD request. Did that not happen for you?