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

Pass getUploadParameters response value to upload() result #4635

Closed
2 tasks done
mittalyashu opened this issue Aug 18, 2023 · 6 comments · Fixed by #4701
Closed
2 tasks done

Pass getUploadParameters response value to upload() result #4635

mittalyashu opened this issue Aug 18, 2023 · 6 comments · Fixed by #4701
Assignees
Labels

Comments

@mittalyashu
Copy link

mittalyashu commented Aug 18, 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

Inside getUploadParameters function, in additional to generating a preSignedUrl, there is some additional information returned from same endpoint. We need to access that additional information on the client side once the upload is successful.

({
getUploadParameters: async (file) => {
  const response = fetch("api call");

  return {
    method: "PUT",
    url: // signed url,
    ...response.data 
  }
}
})

After executing the upload() method, there is no way to access that data after successful upload.

Solution

It would be nice to be able to access the data something like this:

({
getUploadParameters: async (file) => {
  const response = fetch("api call");

  return {
    method: "PUT",
    url: // signed url,
    data: {
      ...response.data
    } 
  }
}
})

uppy.upload()
  .then(async (result) => {
    console.log('result', result.data);
})

Alternatives

S3 API do not return the Location response header and Uppy just returns undefined for uploadURL.

Maybe add support for uploadURL.

uppy.upload()
  .then(async (result) => {
    console.log('result', result.successful[0].uploadURL);
})
@aduh95
Copy link
Contributor

aduh95 commented Aug 21, 2023

S3 API do not return the Location response header and Uppy just returns undefined for uploadURL.

Maybe add support for uploadURL.

I think that's no longer true since we fixed #4613.
You do need to tweak your bucket CORS config to allow Uppy to read the Location header, once that's done uploadURL should take the value of that header.

@mittalyashu
Copy link
Author

mittalyashu commented Aug 30, 2023

I have set the following CROS configuration in both Cloudflare R2 and AWS S3

[
    {
        "AllowedHeaders": [
            "Authorization",
            "x-amz-date",
            "x-amz-content-sha256",
            "content-type"
        ],
        "AllowedMethods": [
            "PUT",
            "GET"
        ],
        "AllowedOrigins": [
            "http://localhost:3000"
        ],
        "ExposeHeaders": [
            "ETag",
            "Location"
        ],
        "MaxAgeSeconds": 3000
    },
    {
        "AllowedHeaders": [],
        "AllowedMethods": [
            "GET"
        ],
        "AllowedOrigins": [
            "*"
        ],
        "ExposeHeaders": [],
        "MaxAgeSeconds": 3000
    }
]

One thing to note is that the file is still getting uploaded / store in the bucket.

I am still getting uploadURL: undefined.


Aside from that, I would also like to discuss about the feature request (as mentioned in this issue description) to pass additional data generated by the methods:

  • getUploadParameters
  • getChunkSize
    just to name a few

to make it available in the result object when the file is uploaded.

@aduh95
Copy link
Contributor

aduh95 commented Aug 31, 2023

uppy.upload()
  .then(async (result) => {
    console.log('result', result.successful[0].uploadURL);
})

Sorry, I didn't notice your code snippet contained a slight error, here's how to get the uploadURL:

uppy.upload()
  .then(async (result) => {
    console.log('result', result.successful[0].response.uploadURL);
})

Aside from that, I would also like to discuss about the feature request (as mentioned in this issue description) to pass additional data generated by the methods:

If you come up with a proposal, we can discuss it further, but I doubt we would be able to do it in a clean manner. Besides, getUploadParameters is meant to return the upload parameters to S3, not pass additional metadata client to client, you should be able to pass them from your app.

@mittalyashu
Copy link
Author

Hmm... It's still the same.

This screenshot is for AWS S3 response.

CleanShot 2023-09-02 at 19 45 31@2x

and it's the same for Cloudflare R2


I doubt we would be able to do it in a clean manner.

wdym?

@aduh95
Copy link
Contributor

aduh95 commented Sep 4, 2023

Hmm... It's still the same.

You're right, sorry I misread, result.successful[0].uploadURL and result.successful[0].response.uploadURL should always be the same. But anyway, Uppy is supposed to support returning the Location value is uploadURL, if it doesn't it either means there's a misconfiguration on your end, or a bug on Uppy's end. Let's get to the bottom of this.

Can you give more info on your setup? What package/version are you using? What options are you using? Specifically, are you using shouldUseMultipart option?

@mittalyashu
Copy link
Author

Can you give more info on your setup?

I have created a separate react hook for using Uppy for file upload without using Multipart upload.

return new Uppy({
    id: id || 'FileUploader',
    restrictions: {
      maxNumberOfFiles: 1,
      allowedFileTypes,
      maxFileSize,
    },
  }).use(AwsS3, {
    id: 'FileUpload',
    shouldUseMultipart: false,
    getUploadParameters: async (file) => {
      const response = // make API call to get `uploadFileUrl`

      return {
        method: 'PUT',
        url: response.uploadFileUrl,
      };
    },
  });

What package/version are you using?

"@uppy/aws-s3": "^3.3.0"

What options are you using? Specifically, are you using shouldUseMultipart option?

Just for context. Yes, I am using multipart upload in same project at other places and it is working fine.

For this specific use-case I am trying to upload a file in a single request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants