@uppy/aws-s3: clarify and warn when incorrect buckets settings are us…

…ed (#5505) #5388 (comment)
transloadit · Nov 11, 2024 · 8cb65bb · 8cb65bb
1 parent 014f1da
commit 8cb65bb
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 11 deletions.
diff --git a/docs/uploader/aws-s3-multipart.mdx b/docs/uploader/aws-s3-multipart.mdx
@@ -101,7 +101,7 @@ The configuration required for Uppy and Companion is this:
 [
 	{
 		"AllowedOrigins": ["https://my-app.com"],
-		"AllowedMethods": ["GET", "PUT"],
+		"AllowedMethods": ["GET", "PUT", "POST"],
 		"MaxAgeSeconds": 3000,
 		"AllowedHeaders": [
 			"Authorization",

diff --git a/packages/@uppy/aws-s3/src/HTTPCommunicationQueue.ts b/packages/@uppy/aws-s3/src/HTTPCommunicationQueue.ts
@@ -242,7 +242,7 @@ export class HTTPCommunicationQueue<M extends Meta, B extends Body> {
     file: UppyFile<M, B>,
     chunk: Chunk,
     signal?: AbortSignal,
-  ): Promise<UploadPartBytesResult & B> {
+  ) {
     const {
       method = 'POST',
       url,
@@ -252,7 +252,7 @@ export class HTTPCommunicationQueue<M extends Meta, B extends Body> {
       signal,
     }).abortOn(signal)
 
-    let body
+    let body: FormData | Blob
     const data = chunk.getData()
     if (method.toUpperCase() === 'POST') {
       const formData = new FormData()
@@ -267,21 +267,24 @@ export class HTTPCommunicationQueue<M extends Meta, B extends Body> {
 
     const { onProgress, onComplete } = chunk
 
-    const result = await this.#uploadPartBytes({
+    const result = (await this.#uploadPartBytes({
       signature: { url, headers, method } as any,
       body,
       size: data.size,
       onProgress,
       onComplete,
       signal,
-    }).abortOn(signal)
+    }).abortOn(signal)) as unknown as B // todo this doesn't make sense
 
-    return 'location' in result ?
-        (result as UploadPartBytesResult & B)
-      : ({
+    // location will be missing from result if CORS is not correctly set up on the bucket.
+    return 'location' in result ? result : (
+        {
+          // todo `url` is not really the final location URL of the resulting file, it's just the base URL of the bucket
+          // https://github.com/transloadit/uppy/issues/5388
           location: removeMetadataFromURL(url),
           ...result,
-        } as any)
+        }
+      )
   }
 
   async uploadFile(

diff --git a/packages/@uppy/aws-s3/src/index.ts b/packages/@uppy/aws-s3/src/index.ts
@@ -760,14 +760,14 @@ export default class AwsS3Multipart<
         }
         const { etag, location } = headersMap
 
-        if (method.toUpperCase() === 'POST' && location === null) {
+        if (method.toUpperCase() === 'POST' && location == null) {
           // Not being able to read the Location header is not a fatal error.
           // eslint-disable-next-line no-console
           console.warn(
             'AwsS3/Multipart: Could not read the Location header. This likely means CORS is not configured correctly on the S3 Bucket. See https://uppy.io/docs/aws-s3-multipart#S3-Bucket-Configuration for instructions.',
           )
         }
-        if (etag === null) {
+        if (etag == null) {
           reject(
             new Error(
               'AwsS3/Multipart: Could not read the ETag header. This likely means CORS is not configured correctly on the S3 Bucket. See https://uppy.io/docs/aws-s3-multipart#S3-Bucket-Configuration for instructions.',