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

data.uploadURL undefined in "upload-success" event, uploading images to S3 using @uppy/aws-s3 #4613

Closed
2 tasks done
lcabre opened this issue Aug 7, 2023 · 8 comments · Fixed by #4614
Closed
2 tasks done
Assignees
Labels

Comments

@lcabre
Copy link

lcabre commented Aug 7, 2023

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

@uppy/aws-s3: 3.2.1,
@uppy/core: 3.3.1,
@uppy/dashboard: 3.5.0,

let flyerUploader = new Uppy({
    id: 'flyer',
    debug: true,
    autoProceed: true,
    restrictions: {
        maxFileSize: 2*1024*1024,
        maxNumberOfFiles: 1,
        minNumberOfFiles: 1,
        allowedFileTypes: ['image/*']
    }
})
.use(Dashboard, {
    target: '#flyer',
    inline: true,
    height: 350,
})
.use(AwsS3, {
    shouldUseMultipart: (file) => false,
    getUploadParameters (file) {
        return fetch(Helpers.route('admin/storage/presigned_url'), {
            method: 'post',
            headers: {
                accept: 'application/json',
                'content-type': 'application/json',
                'X-CSRF-TOKEN': $('meta[name="csrf-token"]').attr('content')
            },
            body: JSON.stringify({
                filename: file.name,
                contentType: file.type
            })
        }).then((response) => {
            return response.json()
        }).then((data) => {
            return {
                method: data.method,
                url: data.url,
                fields: data.fields,
                headers: data.headers
            }
        })
    }
});

flyerUploader.on('upload-success', (file, data) => {
    console.log(data); //data.uploadURL = undefined
});

Expected behavior

I'am expecting that the image is uploaded to S3 and data.uploadURL contains the url to the file in the S3 Bucket

Actual behavior

The image is uploaded to S3 but data.uploadURL is undefined

data variable actually has this value:

{
    "status": 200,
    "body": {
        "ETag": "af60118fac80f4d332513b51c65a2068"
    },
    "uploadURL": undefined
}

Same test on early version like @uppy/aws-s3: 3.1.1 works fine.

@mifi
Copy link
Contributor

mifi commented Aug 8, 2023

this bug is in fact in aws-s3-multipart, right? because shouldUseMultipart: (file) => false

return new AwsS3Multipart(uppy, opts)

@aduh95
Copy link
Contributor

aduh95 commented Aug 8, 2023

The issue seems to be that we only return the ETag from here:

resolve({
ETag: etag,
})

shouldUseMultipart: (file) => false,

FYI you can pass shouldUseMultipart: false, no need for a function if the return value is not dynamic.

@jamesneverette
Copy link

Hi,

We are having this same issue when setting shouldUseMultipart to a function that returns a bool based on the file size.

Is there something additional that needs to be done when implementing with shouldUseMultipart in order to receive a value for uploadURL?

Our config:

const uppy = computed(() => {
		uppyInstance.value =  new Uppy({
			id: props.fieldId,
			autoProceed: true,
			locale: currentLocale.value,
			restrictions: {
				maxFileSize: props.maxFileSize,
				allowedFileTypes: props.accept,
				maxNumberOfFiles: props.maxNumberOfFiles,
			},
		});
		uppyInstance.value.use(ThumbnailGenerator, {
			thumbnailWidth: 400,
		});
		uppyInstance.value.use(AwsS3, {
			companionUrl: '/media/upload',
			companionHeaders: {
				'uppy-auth-token': `token here`,
			},
			limit: 6,
			shouldUseMultipart: (file) => file.size > 100 * 2 ** 20,
		});
		return uppyInstance.value;
	});

 uppy.value.on('upload-success', async (file?: UppyFile) => {
			console.log(uppy.value.getFile(file.id));
			
			// file.uploadURL = undefined
		});

@arturi
Copy link
Contributor

arturi commented Sep 22, 2023

@jamesneverette this was fixed (hopefully) recently #4614. Are you using the latest version of @uppy/aws-s3-multipart?

@jamesneverette
Copy link

jamesneverette commented Sep 22, 2023

We are currently on these versions for uppy packages.

"@uppy/aws-s3": "^3.3.0",
"@uppy/core": "^3.5.0",
"@uppy/locales": "^3.3.0",
"@uppy/thumbnail-generator": "^3.0.4",

Is @uppy/aws-s3-multipart required when setting shouldUseMultipart option? Currently the docs only reference installing @uppy/aws-s3 for the non-legacy uploader.

@arturi
Copy link
Contributor

arturi commented Sep 25, 2023

@aduh95 do you have an idea what might be off here?

@jamesneverette
Copy link

@arturi Just want to note that this is only an issue when setting shouldUseMultipart. If I don't use that then the uploadURL is returned.

@aduh95
Copy link
Contributor

aduh95 commented Sep 26, 2023

I sounds it might be related to #4635. I'll investigate.

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.

5 participants