tus/tusd

Is lockUpload really needed for HEAD?

jimydavis opened this issue · 5 comments

jimydavis commented

I had a situation where the uppy javascript client called PATCH and HEAD at the same time. I am not very sure but it looks like

tusd/pkg/handler/unrouted_handler.go

Line 1352 in 585d7a5

if err := lock.Lock(ctx, releaseLock); err != nil {
is called by both HEAD and PATCH.

Due to HEAD and PATCH being called at the same time from frontend, the HEAD succeeded but the PATCH got "interrupted" error. Since HEAD is theoretically non-write, why does HEAD lead to this locking?

Maybe I understood the whole code path wrong. Thanks for the help!

Acconut commented

Yes, acquiring the lock is also needed for HEAD requests because read access to a resource that is concurrently modified is not safe. It's not relevant that HEAD is not modifying the resource. But without the lock, it might retrieve an invalid state about the upload. Maybe the documentation can help you understand this in more detail: https://tus.github.io/tusd/advanced-topics/locks/

Due to HEAD and PATCH being called at the same time from frontend

I would recommend to not send HEAD requests during the upload. This shouldn't be necessary.

jimydavis commented

Thank you. The HEAD request is called by the uppy and tusjs libraries (I am not sure which). It calls it on the exact timestamp in my server logs. My frontend has a vanilla implementation of uppy with the tusjs library with. At first I thought the onProgress flag (that updates a progress bar) calls HEAD but it seems like that is done client side and no HEAD is sent out. Should I post it as a tusjs issue or is my interpretation wrong? The Tus server error is below for additional clarity

{
    "time": "2024-07-29T04:48:44.724241995Z",
    "level": "ERROR",
    "source": {
      "function": "github.com/tus/tusd/v2/pkg/handler.(*UnroutedHandler).writeChunk",
      "file": "redacted/github.com/tus/tusd/v2/pkg/handler/unrouted_handler.go",
      "line": 902
    },
    "msg": "BodyReadError",
    "method": "PATCH",
    "path": "/upload/[ID]",
    "requestId": "",
    "id": "[ID]",
    "error": "ERR_UPLOAD_INTERRUPTED: upload has been interrupted by another request for this upload resource"
  },
Acconut commented

Please open an issue in the Uppy repository about this and include your code. Please be as detailed as you can and mention ways to reproduce this.

At first I thought the onProgress flag (that updates a progress bar) calls HEAD but it seems like that is done client side and no HEAD is sent out.

Correct. Progress updates are retrieved without the use of HEAD. HEAD is only used for resuming uploads, so I assume you are accidentally triggering resumption while the upload is still running.

jimydavis commented

Thanks @Acconut . I found out that the true error underneath was that the file size (Upload-Length) was X but upon calling HEAD, the response from server for Upload-Offset was greater than X. I am using the golang tusd server. I can't reproduce it either so going to close this issue. Thank you for the help.

Acconut commented

I found out that the true error underneath was that the file size (Upload-Length) was X but upon calling HEAD, the response from server for Upload-Offset was greater than X

That is very odd and should never happen. Please let us know if you encounter this problem again!

  • 😄1