@uppy/companion: add COMPANION_TUS_DEFERRED_UPLOAD_LENGTH

[dschmidt]

Fix #5560

I'm no tus expert, I'm not sure if there would be any way to automatically detect what the server supports, which would be more elegant ... but this was straight forward to implement at least 🙈

[github-actions]

Diff output files

diff --git a/packages/@uppy/companion/lib/server/Uploader.js b/packages/@uppy/companion/lib/server/Uploader.js
index 3095586..ee2d553 100644
--- a/packages/@uppy/companion/lib/server/Uploader.js
+++ b/packages/@uppy/companion/lib/server/Uploader.js
@@ -452,9 +452,7 @@ class Uploader {
       const tusOptions = {
         endpoint: this.options.endpoint,
         uploadUrl: this.options.uploadUrl,
-        uploadLengthDeferred: !isFileStream,
         retryDelays: [0, 1000, 3000, 5000],
-        uploadSize: isFileStream ? this.size : undefined,
         chunkSize,
         headers: headerSanitize(this.options.headers),
         addRequestId: true,
@@ -493,6 +491,15 @@ class Uploader {
           resolve({ url: uploader.tus.url });
         },
       };
+      if (this.options.companionOptions.tusDeferredUploadLength && !isFileStream) {
+        tusOptions.uploadLengthDeferred = true;
+      } else {
+        if (!this.size) {
+          reject(new Error("tusDeferredUploadLength needs to be enabled if no file size is provided by the provider"));
+        }
+        tusOptions.uploadLengthDeferred = false;
+        tusOptions.uploadSize = this.size;
+      }
       this.tus = new tus.Upload(stream, tusOptions);
       this.tus.start();
     });
diff --git a/packages/@uppy/companion/lib/standalone/helper.js b/packages/@uppy/companion/lib/standalone/helper.js
index ccc5e1a..b3c9540 100644
--- a/packages/@uppy/companion/lib/standalone/helper.js
+++ b/packages/@uppy/companion/lib/standalone/helper.js
@@ -181,6 +181,9 @@ const getConfigFromEnv = () => {
     streamingUpload: process.env.COMPANION_STREAMING_UPLOAD
       ? process.env.COMPANION_STREAMING_UPLOAD === "true"
       : undefined,
+    tusDeferredUploadLength: process.env.COMPANION_TUS_DEFERRED_UPLOAD_LENGTH
+      ? process.env.COMPANION_TUS_DEFERRED_UPLOAD_LENGTH === "true"
+      : true,
     maxFileSize: process.env.COMPANION_MAX_FILE_SIZE ? parseInt(process.env.COMPANION_MAX_FILE_SIZE, 10) : undefined,
     chunkSize: process.env.COMPANION_CHUNK_SIZE ? parseInt(process.env.COMPANION_CHUNK_SIZE, 10) : undefined,
     clientSocketConnectTimeout: process.env.COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT

[Murderlon]

Unfortunately it's not as straightforward as we also support streaming files with unknown sizes (#5489) which requires deferred upload length. So we would have to disallow unknown sizes in general, not just change the upload option.

What do you think @mifi?

[mifi]

To summarize: Some servers (download sources in companion) send an incorrect header value, causing the the upload to fail in tus. In #4697 we started completely ignoring the content-length header and always set uploadLengthDeferred to true for this reason.

Unfortunately it's not as straightforward as we also support streaming files with unknown sizes (#5489) which requires deferred upload length. So we would have to disallow unknown sizes in general, not just change the upload option.

I think that's ok because if streams have an unknown size, isFileStream will also be false so uploadLengthDeferred will still be set to true unless the user has explicitly set companionOptions.deferredUploadLength to false (it defaults to true).

I think deferredUploadLength is here effectively an option that can be set to false in order to always trust the http content-length header (which we no longer trust by default). So maybe we should call it something else. ignoreContentLength? This also needs to be documented properly because it can be very confusing.

I'm not sure if adding this option is the best solution, but it does seem to be a relatively clean way to support tus servers that don't support uploadLengthDeferred.

Maybe @Acconut has some insights.

[dschmidt]

@Acconut @Murderlon @mifi ping :)

How do we want to progress here? After finally landing the WebDAV provider implementation (thanks again for making that happen!), this is still stopping us from fully switching to upstream Uppy/Companion - thus I would really like to find a solution here.

edit:
see also what I've written down in the bug ticket

[Acconut]

I'm not sure if there would be any way to automatically detect what the server supports

You can detect support for deferred length on the server-side by sending an OPTIONS request before starting the upload. However, doing this before every upload introduces an additional request for every tus upload, which is why tus-js-client does not check support for this extension on its own. For Companion, you could consider checking the server's supported extensions before starting an upload. If these results are cached, it's impact should be minimal. Just an idea for now that I wanted to share.

I'm not sure if adding this option is the best solution, but it does seem to be a relatively clean way to support tus servers that don't support uploadLengthDeferred.

I think adding such an option provides value. A HTTP server can send a response without a Content-Length header and Companion should be prepared to handle these responses. The best approach is to use tus' extension to defer the length. If that's disabled by the user, uploads without a known length should fail with a meaningful message with a helpful description.

Alternatively, we could consider buffering the file on disk when we don't know the length, but this opens up another can of worms that we had to deal with before. So this wouldn't be my preferred option.

[Murderlon]

Let me know if you're also up for creating a docs PR. Otherwise we can take a look after.

[dschmidt]

Let me know if you're also up for creating a docs PR. Otherwise we can take a look after.

Hehe, I'm not exactly keen on that ... but it's probably only fair to document my own changes.
Can try to send a PR later or later this week

[Murderlon]

Thanks!

[dschmidt]

Sweet! If you could cut a release with that, that would be amazing! :)

[Murderlon]

Probably somewhere today