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

Uploading additional batches with Transloadit is broken allowMultipleUploadBatches: true #5397

Closed
2 tasks done
719media opened this issue Aug 9, 2024 · 12 comments · Fixed by #5400
Closed
2 tasks done
Labels
Bug prio-1 High-priority issues

Comments

@719media
Copy link

719media commented Aug 9, 2024

Initial checklist

  • I understand this is a bug report and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Link to runnable example

No response

Steps to reproduce

const uppyInstance = new Uppy({
  allowMultipleUploadBatches: true,
  autoProceed: true,
}).use(Transloadit, {
  waitForEncoding: true,
  assemblyOptions: {
    params: {
      auth: { key: TRANSLOADIT_AUTH_KEY },
      template_id: TEMPLATE_ID,
    },
  },
});

Set up transloadit like this, upload some files, then do it again. You will get an error: tus: neither an endpoint or an upload URL is provided

See community post made by someone else with issue as well:
https://community.transloadit.com/t/getting-error-while-uploading-second-batch-of-files-in-uppy/17201

Isolated this behavior as starting to occur with uppy v4 release

Expected behavior

Upload should work

Actual behavior

Upload fails

@719media 719media added the Bug label Aug 9, 2024
@mifi mifi changed the title uppy with transloadit throws error on 2nd (4th, 6th, etc.) attempt at upload Uploading additional batches with Transloadit is broken (allowMultipleUploadBatches: true) Aug 9, 2024
@mifi mifi changed the title Uploading additional batches with Transloadit is broken (allowMultipleUploadBatches: true) Uploading additional batches with Transloadit is broken allowMultipleUploadBatches: true Aug 9, 2024
@mifi mifi added the prio-1 High-priority issues label Aug 9, 2024
@mifi
Copy link
Contributor

mifi commented Aug 9, 2024

I can confirm this is broken, and it only happens with allowMultipleUploadBatches: true.

mifi added a commit that referenced this issue Aug 9, 2024
fixes #5397
also refactor from promise.then to async/await
and fix what seems like broken logic with recursive this.#afterUpload call
aduh95 pushed a commit that referenced this issue Aug 15, 2024
fixes #5397
also refactor from promise.then to async/await
and fix what seems like broken logic with recursive this.#afterUpload call
@aduh95 aduh95 closed this as completed in 013f4ea Aug 15, 2024
@elliotdickison
Copy link
Contributor

I'm still getting the exact same error with the same config for @uppy/transloadit@4.1.3

@elliotdickison
Copy link
Contributor

For anyone else running up against this issue, I was able to work around it by disabling autoProceed, setting up a queue with concurrency 1 (p-queue works great), and then waiting on manual calls to uppy.upload() to complete. Obviously this is not ideal, but it allowed me to move on for now. I'm not using the dashboard, not sure how you'd accomplish something like this with the dashboard.

import PQueue from "p-queue"

const uploadFilesQueue = new PQueue({ concurrency: 1 })

const uppyInstance = new Uppy({
  allowMultipleUploadBatches: true,
  // autoProceed: true,
}).use(Transloadit, {
  waitForEncoding: true,
  assemblyOptions: {
    params: {
      auth: { key: TRANSLOADIT_AUTH_KEY },
      template_id: TEMPLATE_ID,
    },
  },
})

// Use this to guarantee that only one batch of uploads will be processed at a time.
function uploadFiles(files) {
  uploadFilesQueue.add(async () => {
    files.forEach(file => uppyInstance.addFile(file))
    await uppyInstance.upload()
  })
}

@mifi Any chance we could get this issue re-opened?

@mifi mifi reopened this Nov 28, 2024
@elliotdickison
Copy link
Contributor

One more datapoint: this only occurs with the Transloadit plugin, not when using the Tus plugin and a custom companion server.

@Murderlon
Copy link
Member

Are you on 4.x? Because I just tested this locally and it works.

@elliotdickison
Copy link
Contributor

Package versions are:

"@uppy/box": "^3.1.1",
"@uppy/core": "4.2.3",
"@uppy/dashboard": "4.1.2",
"@uppy/drag-drop": "4.0.4",
"@uppy/dropbox": "4.1.1",
"@uppy/file-input": "4.0.3",
"@uppy/google-drive": "4.1.1",
"@uppy/progress-bar": "4.0.1",
"@uppy/react": "4.0.3",
"@uppy/transloadit": "4.1.3",
"@uppy/url": "4.1.1"

@elliotdickison
Copy link
Contributor

I should also clarify that I'm experiencing this when using Uppy's addFile API in conjunction with Transloadit, autoProceed: true, and allowMultipleUploadBatches: true. I haven't tried via the dashboard.

@Murderlon
Copy link
Member

I still can't reproduce with this:

  const uppy = new Uppy({
    debug: true,
    autoProceed: true,
    allowMultipleUploads: true,
  })
    .use(StatusBar, { target: '#status-bar', hideAfterFinish: false })
    .use(FileInput, { target: '#file-input' })
    .use(Transloadit, {
      service: TRANSLOADIT_SERVICE_URL,
      waitForEncoding: true,
      assemblyOptions,
    })

FileInput is basically the tiniest wrapper around directly calling addFile() so it should be the same.

@elliotdickison
Copy link
Contributor

elliotdickison commented Dec 4, 2024

Perhaps I'm not using Uppy in the intended way, but in this particular case I'm actually picking the files from elsewhere (react dropzone from another component) and passing the list of picked files to Uppy via uppy.addFile. Below is pretty close to the exact code that's triggering the error (react dropzone code omitted and some config values redacted). If I call addFilesToUppy a second time before the first batch of files is uploaded then I get an error like [Uppy] [08:55:26] tus: neither an endpoint or an upload URL is provided.

const uppy = new Uppy({
  restrictions: {
    allowedFileTypes: ["audio/mpeg", "audio/flac"],
    maxFileSize: null,
    maxTotalFileSize: null,
    maxNumberOfFiles: null,
    minFileSize: null,
    minNumberOfFiles: null,
    requiredMetaFields: [],
  },
  allowMultipleUploadBatches: true,
  autoProceed: true,
})
uppy.use(Transloadit, {
  limit: 10,
  waitForEncoding: true,
  assemblyOptions: {
    params: {
      steps: {
        ":original": { robot: "/upload/handle" },
        exported: {
          use: ":original",
          robot: "/cloudflare/store",
          credentials: "treefort-r2",
          headers: {
            "Cache-Control": "public, max-age=31536000, immutable",
          },
          path: "fb8161dc-4493-4cf6-9481-8f8b05078f02/uploads/${file.id}.${file.ext}",
        },
      },
      fields: { type: "uploadAsset" },
      auth: {
        key: "<redacted>",
        expires: "2024/12/05 04:49:42+00:00",
      },
      notify_url:
        "<redacted>",
    },
    signature:
      "<redacted>",
  },
})

// This files array is coming from react dropzone
function addFilesToUppy(filesFromDropzone) {
  filesFromDropzone.forEach(file => uppy.addFile({data: file, name: file.name, type: file.type })
}

@Murderlon
Copy link
Member

Can you show how you integrate Uppy in your React code?

And unrelated but how are you setting the signature? Are you safely computing it on the server instead of the client?

@elliotdickison
Copy link
Contributor

elliotdickison commented Dec 4, 2024

Yes we generate assembly options and sign them on the server and then send those down to authenticated clients. To isolate the problem here though I copied one of the assembly responses from our server and hard-coded the assembly JSON in my call to Uppy. The code pasted above is much simpler than our React-based, server-integrated Uppy setup but I was able to reproduce the issue with it. Uppy is instantiated at the module level, no React involved with the exception that addFilesToUppy is called from React-land.

@Murderlon
Copy link
Member

If you have a reproducible example for me I can take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug prio-1 High-priority issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants