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

The uploadFile function in aws-s3-multipart is not properly validating for null chunks. #4648

Open
2 tasks done
sergeykosik opened this issue Aug 24, 2023 · 6 comments
Open
2 tasks done
Assignees
Labels
AWS S3 Plugin that handles uploads to Amazon AWS S3 Bug

Comments

@sergeykosik
Copy link

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

aws-s3-multipart

Steps to reproduce

Multipe clicks (monkey testing) of Pause / Resume buttons are throwing the following exception:

[Uppy] [11:44:50] TypeError: Cannot read properties of null (reading 'shouldUseMultipart')
at index.js:276:43
at Generator.next ()
at asyncGeneratorStep (asyncToGenerator.js:3:1)
at _next (asyncToGenerator.js:25:1)
at asyncToGenerator.js:32:1
at new ZoneAwarePromise (zone.js:1429:1)
at asyncToGenerator.js:21:1
at HTTPCommunicationQueue.resumeUploadFile (index.js:274:48)
at MultipartUploader._resumeUpload2 (MultipartUploader.js:266:1)
at MultipartUploader.start (MultipartUploader.js:182:1)

Which is related to
https://github.com/transloadit/uppy/blob/main/packages/%40uppy/aws-s3-multipart/src/index.js#L283 :

if (chunks.length === 1 && !chunks[0].shouldUseMultipart) { return this.#nonMultipartUpload(file, chunks[0], signal) }

Expected behavior

There should be a check for chunk null and no error to be thrown.

Actual behavior

The type error is thrown.

@Murderlon
Copy link
Member

Are you on the latest version? This was fixed here: #4581

@sergeykosik
Copy link
Author

sergeykosik commented Aug 24, 2023

Using "@uppy/aws-s3-multipart": "3.5.3".
See the demo: https://vimeo.com/857502690/55c721e1b3?share=copy

@Murderlon
Copy link
Member

Do you get the same error on 3.5.4?

@sergeykosik
Copy link
Author

Do you get the same error on 3.5.4?

I can confirm that I am able to reproduce the same error on 3.5.4.

@Doerge
Copy link

Doerge commented Jan 30, 2024

Seeing this too. I read through #4737 and am also using a self-rolled backend, and suspect it might be because I'm doing something wrong there.

    const uppy = new Uppy({ logger: debugLogger })
      .use(AwsS3, {
        shouldUseMultipart: (file: UppyFile) => true,
        createMultipartUpload: (file: UppyFile) => {
          return new Promise((resolve, reject) => {
            // Websocket API
            this.pushEventTo(
              this.el,
              "create_multipart_upload",
              {
                name: file.name,
                mime_type: file.type,
                size: file.size,
                id: file.id,
              },
              (reply, ref) => {
                resolve(reply);
              },
            );
          });
        },

Backend in pseudo code:

function createMultipartUploadEndpoint(req) {
  file = createFileInDb(
     name = req.name,
     client_mime = req.mime_type,
     client_size = req.size,
     upload_key = UUID.generate()
  )
  aws_upload_id = AWS.createMultiPartUpload(file)
  return { uploadId: aws_upload_id, key: file.upload_key }
}

I suspect this whole issue might arise because I generate a new key on the backend (the UUID.generate())
I found that the UppyFile.id is not unique, so can't jam that into my db.
Is this an anti-pattern? I thought it was allowed since I'm required to return the key in the response.

Versions:

    "@uppy/aws-s3": "^3.6.0",
    "@uppy/drag-drop": "^3.0.3",
    "@uppy/status-bar": "^3.2.5",

@Doerge
Copy link

Doerge commented Jan 30, 2024

Found the problem: The AWS lib I use (Auto generated from specs) ListParts call returns ETag and PartNumber wrapped in strings. Like:

{ ETag: "\"foo\"", PartNumber: "123" }

I parsed that into:

{ ETag: "foo", PartNumber: 123 }

and now resume works.

I can still see null-chunks in the internal structure. So might still be issues lurking around.

@Murderlon Murderlon added the AWS S3 Plugin that handles uploads to Amazon AWS S3 label Feb 5, 2024
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

Successfully merging a pull request may close this issue.

4 participants