Просмотр исходного кода

companion: drop parallel down/upload for S3 multipart (#2114)

* companion: drop parallel down/upload for S3 multipart

We were not ending the `fs-tail-stream` correctly. `fs-tail-stream` is
also not a battle-tested package and relying on file watching is never a
fun time, so it's better if we can avoid it entirely.

One good way to do that is to axe the entire parallel download/upload
feature, and instead download files in full before uploading them, like
we do for Tus and XHR :)

Fixes #1457.

* companion: add back the multiple start calls test
Renée Kooi 5 лет назад
Родитель
Сommit
849b12a655

+ 0 - 9
package-lock.json

@@ -8058,7 +8058,6 @@
       "version": "file:packages/@uppy/companion",
       "requires": {
         "@purest/providers": "1.0.1",
-        "@uppy/fs-tail-stream": "1.2.0",
         "atob": "2.1.2",
         "aws-sdk": "2.587.0",
         "body-parser": "1.19.0",
@@ -8548,14 +8547,6 @@
         "get-form-data": "^2.0.0"
       }
     },
-    "@uppy/fs-tail-stream": {
-      "version": "1.2.0",
-      "resolved": "https://registry.npmjs.org/@uppy/fs-tail-stream/-/fs-tail-stream-1.2.0.tgz",
-      "integrity": "sha512-Yil6Z4phfYBHneLK5aimQ4AtVleorcTRuy0TzJrhFx4OEdbGUQ8haLQklQUOhaFJbaeu20VGishz/kLWzQiiTg==",
-      "requires": {
-        "once": "^1.4.0"
-      }
-    },
     "@uppy/golden-retriever": {
       "version": "file:packages/@uppy/golden-retriever",
       "requires": {

+ 0 - 1
packages/@uppy/companion/package.json

@@ -30,7 +30,6 @@
   },
   "dependencies": {
     "@purest/providers": "1.0.1",
-    "@uppy/fs-tail-stream": "1.2.0",
     "atob": "2.1.2",
     "aws-sdk": "2.587.0",
     "body-parser": "1.19.0",

+ 23 - 28
packages/@uppy/companion/src/server/Uploader.js

@@ -2,7 +2,6 @@ const fs = require('fs')
 const path = require('path')
 const tus = require('tus-js-client')
 const uuid = require('uuid')
-const createTailReadStream = require('@uppy/fs-tail-stream')
 const emitter = require('./emitter')
 const request = require('request')
 const serializeError = require('serialize-error')
@@ -219,12 +218,26 @@ class Uploader {
     if (chunk === null) {
       this.writeStream.on('finish', () => {
         this.streamsEnded = true
-        if (this.options.endpoint && protocol === PROTOCOLS.multipart) {
-          this.uploadMultipart()
-        }
-
-        if (protocol === PROTOCOLS.tus && !this.tus) {
-          return this.uploadTus()
+        switch (protocol) {
+          case PROTOCOLS.multipart:
+            if (this.options.endpoint) {
+              this.uploadMultipart()
+            }
+            break
+          case PROTOCOLS.s3Multipart:
+            if (!this.s3Upload) {
+              this.uploadS3Multipart()
+            } else {
+              logger.warn('handleChunk() called multiple times', 'uploader.s3.duplicate', this.shortToken)
+            }
+            break
+          case PROTOCOLS.tus:
+            if (!this.tus) {
+              this.uploadTus()
+            } else {
+              logger.warn('handleChunk() called multiple times', 'uploader.tus.duplicate', this.shortToken)
+            }
+            break
         }
       })
 
@@ -233,19 +246,7 @@ class Uploader {
 
     this.writeStream.write(chunk, () => {
       logger.debug(`${this.bytesWritten} bytes`, 'uploader.download.progress', this.shortToken)
-      if (protocol === PROTOCOLS.multipart || protocol === PROTOCOLS.tus) {
-        return this.emitIllusiveProgress()
-      }
-
-      if (protocol === PROTOCOLS.s3Multipart && !this.s3Upload) {
-        return this.uploadS3Multipart()
-      }
-      // @TODO disabling parallel uploads and downloads for now
-      // if (!this.options.endpoint) return
-
-      // if (protocol === PROTOCOLS.tus && !this.tus) {
-      //   return this.uploadTus()
-      // }
+      return this.emitIllusiveProgress()
     })
   }
 
@@ -493,16 +494,10 @@ class Uploader {
   }
 
   /**
-   * Upload the file to S3 while it is still being downloaded.
+   * Upload the file to S3 using a Multipart upload.
    */
   uploadS3Multipart () {
-    const file = createTailReadStream(this.path, {
-      tail: true
-    })
-
-    this.writeStream.on('finish', () => {
-      file.close()
-    })
+    const file = fs.createReadStream(this.path)
 
     return this._uploadS3MultipartStream(file)
   }