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

Companion stream upload unknown size files #5489

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

mifi
Copy link
Contributor

@mifi mifi commented Oct 21, 2024

behind a new option streamingUploadSizeless / COMPANION_STREAMING_UPLOAD_SIZELESS

originally thought for Tus, but italso seems to be working with other protocols

closes #5305

todo:

  • e2e test

one issue is that the progress shows X kB of 0 B * 0s left:
Screenshot 2024-10-21 at 12 03 01

update: will now show better progress. in the following example, i upload 2 local files of known length, and 2 URLs (companion) with unknown size:

1
2
3
4
5
6
7

Script that used to test this:

const http = require('node:http');

const port = process.env.PORT || 1337

http.createServer((request, response) => {
    if (request.method === 'GET') {
        response.setHeader('Content-Type', 'text/html; charset=UTF-8');
        response.setHeader('Transfer-Encoding', 'chunked');
    
        let i = 0;
        setInterval(() => {
            if (i === 300 + Math.floor(Math.random() * 300)) {
                response.end();
                return;
            }
            response.write(Buffer.from(Array.from({ length: 100 }, () => '1').join('')));
            response.write('\n');
            i++;
        }, 10);
    } else if (request.method === 'HEAD') {
        response.setHeader('Content-Type', 'text/html; charset=UTF-8');
        response.setHeader('Transfer-Encoding', 'chunked');
        response.end();
    } else {
        response.end();
    }
}).listen(port, null);

console.log(`Server running at http://localhost:${port}/`);

behind a new option streamingUploadSizeless
COMPANION_STREAMING_UPLOAD_SIZELESS
for tus
seems to be working
closes #5305
Copy link
Contributor

github-actions bot commented Oct 21, 2024

Diff output files
diff --git a/packages/@uppy/companion-client/lib/RequestClient.js b/packages/@uppy/companion-client/lib/RequestClient.js
index 70e2b15..419aaf9 100644
--- a/packages/@uppy/companion-client/lib/RequestClient.js
+++ b/packages/@uppy/companion-client/lib/RequestClient.js
@@ -6,7 +6,6 @@ var id = 0;
 function _classPrivateFieldLooseKey(e) {
   return "__private_" + id++ + "_" + e;
 }
-import emitSocketProgress from "@uppy/utils/lib/emitSocketProgress";
 import ErrorWithCause from "@uppy/utils/lib/ErrorWithCause";
 import fetchWithNetworkError from "@uppy/utils/lib/fetchWithNetworkError";
 import getSocketHost from "@uppy/utils/lib/getSocketHost";
@@ -59,6 +58,22 @@ async function handleJSONResponse(res) {
     message: errMsg,
   });
 }
+function emitSocketProgress(uploader, progressData, file) {
+  const {
+    progress,
+    bytesUploaded,
+    bytesTotal,
+  } = progressData;
+  if (progress) {
+    var _file$progress$upload;
+    uploader.uppy.log(`Upload progress: ${progress}`);
+    uploader.uppy.emit("upload-progress", file, {
+      uploadStarted: (_file$progress$upload = file.progress.uploadStarted) != null ? _file$progress$upload : 0,
+      bytesUploaded,
+      bytesTotal,
+    });
+  }
+}
 var _companionHeaders = _classPrivateFieldLooseKey("companionHeaders");
 var _getUrl = _classPrivateFieldLooseKey("getUrl");
 var _requestSocketToken = _classPrivateFieldLooseKey("requestSocketToken");
diff --git a/packages/@uppy/companion/lib/server/Uploader.d.ts b/packages/@uppy/companion/lib/server/Uploader.d.ts
index 945e941..989d8d3 100644
--- a/packages/@uppy/companion/lib/server/Uploader.d.ts
+++ b/packages/@uppy/companion/lib/server/Uploader.d.ts
@@ -94,10 +94,11 @@ declare class Uploader {
     downloadedBytes: number;
     readStream: import("stream").Readable | fs.ReadStream;
     abortReadStream(err: any): void;
+    _getUploadProtocol(): string;
     _uploadByProtocol(req: any): Promise<any>;
     _downloadStreamAsFile(stream: any): Promise<void>;
     tmpPath: string;
-    _needDownloadFirst(): boolean;
+    _canStream(): any;
     /**
      *
      * @param {import('stream').Readable} stream
diff --git a/packages/@uppy/companion/lib/server/Uploader.js b/packages/@uppy/companion/lib/server/Uploader.js
index 8368897..3095586 100644
--- a/packages/@uppy/companion/lib/server/Uploader.js
+++ b/packages/@uppy/companion/lib/server/Uploader.js
@@ -192,10 +192,13 @@ class Uploader {
       this.readStream.destroy(err);
     }
   }
-  async _uploadByProtocol(req) {
+  _getUploadProtocol() {
     // todo a default protocol should not be set. We should ensure that the user specifies their protocol.
     // after we drop old versions of uppy client we can remove this
-    const protocol = this.options.protocol || PROTOCOLS.multipart;
+    return this.options.protocol || PROTOCOLS.multipart;
+  }
+  async _uploadByProtocol(req) {
+    const protocol = this._getUploadProtocol();
     switch (protocol) {
       case PROTOCOLS.multipart:
         return this.#uploadMultipart(this.readStream);
@@ -226,8 +229,8 @@ class Uploader {
     const fileStream = createReadStream(this.tmpPath);
     this.readStream = fileStream;
   }
-  _needDownloadFirst() {
-    return !this.options.size || !this.options.companionOptions.streamingUpload;
+  _canStream() {
+    return this.options.companionOptions.streamingUpload;
   }
   /**
    * @param {import('stream').Readable} stream
@@ -243,7 +246,7 @@ class Uploader {
       }
       this.#uploadState = states.uploading;
       this.readStream = stream;
-      if (this._needDownloadFirst()) {
+      if (!this._canStream()) {
         logger.debug("need to download the whole file first", "controller.get.provider.size", this.shortToken);
         // Some streams need to be downloaded entirely first, because we don't know their size from the provider
         // This is true for zoom and drive (exported files) or some URL downloads.
@@ -385,7 +388,7 @@ class Uploader {
     // If fully downloading before uploading, combine downloaded and uploaded bytes
     // This will make sure that the user sees half of the progress before upload starts (while downloading)
     let combinedBytes = bytesUploaded;
-    if (this._needDownloadFirst()) {
+    if (!this._canStream()) {
       combinedBytes = Math.floor((combinedBytes + (this.downloadedBytes || 0)) / 2);
     }
     // Prevent divide by zero
@@ -536,7 +539,7 @@ class Uploader {
       const httpMethod = (this.options.httpMethod || "").toUpperCase() === "PUT" ? "put" : "post";
       const runRequest = (await got)[httpMethod];
       const response = await runRequest(url, reqOptions);
-      if (bytesUploaded !== this.size) {
+      if (this.size != null && bytesUploaded !== this.size) {
         const errMsg = `uploaded only ${bytesUploaded} of ${this.size} with status: ${response.statusCode}`;
         logger.error(errMsg, "upload.multipart.mismatch.error");
         throw new Error(errMsg);
diff --git a/packages/@uppy/core/lib/Uppy.js b/packages/@uppy/core/lib/Uppy.js
index abb82f2..708341e 100644
--- a/packages/@uppy/core/lib/Uppy.js
+++ b/packages/@uppy/core/lib/Uppy.js
@@ -42,6 +42,10 @@ var _assertNewUploadAllowed = _classPrivateFieldLooseKey("assertNewUploadAllowed
 var _transformFile = _classPrivateFieldLooseKey("transformFile");
 var _startIfAutoProceed = _classPrivateFieldLooseKey("startIfAutoProceed");
 var _checkAndUpdateFileState = _classPrivateFieldLooseKey("checkAndUpdateFileState");
+var _handleUploadProgress = _classPrivateFieldLooseKey("handleUploadProgress");
+var _updateTotalProgress = _classPrivateFieldLooseKey("updateTotalProgress");
+var _updateTotalProgressThrottled = _classPrivateFieldLooseKey("updateTotalProgressThrottled");
+var _calculateTotalProgress = _classPrivateFieldLooseKey("calculateTotalProgress");
 var _addListeners = _classPrivateFieldLooseKey("addListeners");
 var _updateOnlineStatus = _classPrivateFieldLooseKey("updateOnlineStatus");
 var _requestClientById = _classPrivateFieldLooseKey("requestClientById");
@@ -66,6 +70,12 @@ export class Uppy {
     Object.defineProperty(this, _addListeners, {
       value: _addListeners2,
     });
+    Object.defineProperty(this, _calculateTotalProgress, {
+      value: _calculateTotalProgress2,
+    });
+    Object.defineProperty(this, _updateTotalProgress, {
+      value: _updateTotalProgress2,
+    });
     Object.defineProperty(this, _checkAndUpdateFileState, {
       value: _checkAndUpdateFileState2,
     });
@@ -117,9 +127,10 @@ export class Uppy {
     });
     this.scheduledAutoProceed = null;
     this.wasOffline = false;
-    this.calculateProgress = throttle(
-      (file, data) => {
-        const fileInState = this.getFile(file == null ? void 0 : file.id);
+    Object.defineProperty(this, _handleUploadProgress, {
+      writable: true,
+      value: (file, progress) => {
+        const fileInState = file ? this.getFile(file.id) : undefined;
         if (file == null || !fileInState) {
           this.log(`Not setting progress for a file that has been removed: ${file == null ? void 0 : file.id}`);
           return;
@@ -128,23 +139,38 @@ export class Uppy {
           this.log(`Not setting progress for a file that has been already uploaded: ${file.id}`);
           return;
         }
-        const canHavePercentage = Number.isFinite(data.bytesTotal) && data.bytesTotal > 0;
-        this.setFileState(file.id, {
-          progress: {
-            ...fileInState.progress,
-            bytesUploaded: data.bytesUploaded,
-            bytesTotal: data.bytesTotal,
-            percentage: canHavePercentage ? Math.round(data.bytesUploaded / data.bytesTotal * 100) : 0,
-          },
-        });
-        this.calculateTotalProgress();
+        const newProgress = {
+          bytesTotal: progress.bytesTotal,
+          percentage: progress.bytesTotal != null && Number.isFinite(progress.bytesTotal) && progress.bytesTotal > 0
+            ? Math.round(progress.bytesUploaded / progress.bytesTotal * 100)
+            : undefined,
+        };
+        if (fileInState.progress.uploadStarted != null) {
+          this.setFileState(file.id, {
+            progress: {
+              ...fileInState.progress,
+              ...newProgress,
+              bytesUploaded: progress.bytesUploaded,
+            },
+          });
+        } else {
+          this.setFileState(file.id, {
+            progress: {
+              ...fileInState.progress,
+              ...newProgress,
+            },
+          });
+        }
+        _classPrivateFieldLooseBase(this, _updateTotalProgressThrottled)[_updateTotalProgressThrottled]();
       },
-      500,
-      {
+    });
+    Object.defineProperty(this, _updateTotalProgressThrottled, {
+      writable: true,
+      value: throttle(() => _classPrivateFieldLooseBase(this, _updateTotalProgress)[_updateTotalProgress](), 500, {
         leading: true,
         trailing: true,
-      },
-    );
+      }),
+    });
     Object.defineProperty(this, _updateOnlineStatus, {
       writable: true,
       value: this.updateOnlineStatus.bind(this),
@@ -626,7 +652,7 @@ export class Uppy {
       stateUpdate.recoveredState = null;
     }
     this.setState(stateUpdate);
-    this.calculateTotalProgress();
+    _classPrivateFieldLooseBase(this, _updateTotalProgressThrottled)[_updateTotalProgressThrottled]();
     const removedFileIDs = Object.keys(removedFiles);
     removedFileIDs.forEach(fileID => {
       this.emit("file-removed", removedFiles[fileID]);
@@ -752,52 +778,8 @@ export class Uppy {
       (_provider = plugin.provider) == null || _provider.logout == null || _provider.logout();
     });
   }
-  calculateTotalProgress() {
-    const files = this.getFiles();
-    const inProgress = files.filter(file => {
-      return file.progress.uploadStarted || file.progress.preprocess || file.progress.postprocess;
-    });
-    if (inProgress.length === 0) {
-      this.emit("progress", 0);
-      this.setState({
-        totalProgress: 0,
-      });
-      return;
-    }
-    const sizedFiles = inProgress.filter(file => file.progress.bytesTotal != null);
-    const unsizedFiles = inProgress.filter(file => file.progress.bytesTotal == null);
-    if (sizedFiles.length === 0) {
-      const progressMax = inProgress.length * 100;
-      const currentProgress = unsizedFiles.reduce((acc, file) => {
-        return acc + file.progress.percentage;
-      }, 0);
-      const totalProgress = Math.round(currentProgress / progressMax * 100);
-      this.setState({
-        totalProgress,
-      });
-      return;
-    }
-    let totalSize = sizedFiles.reduce((acc, file) => {
-      var _file$progress$bytesT;
-      return acc + ((_file$progress$bytesT = file.progress.bytesTotal) != null ? _file$progress$bytesT : 0);
-    }, 0);
-    const averageSize = totalSize / sizedFiles.length;
-    totalSize += averageSize * unsizedFiles.length;
-    let uploadedSize = 0;
-    sizedFiles.forEach(file => {
-      uploadedSize += file.progress.bytesUploaded;
-    });
-    unsizedFiles.forEach(file => {
-      uploadedSize += averageSize * (file.progress.percentage || 0) / 100;
-    });
-    let totalProgress = totalSize === 0 ? 0 : Math.round(uploadedSize / totalSize * 100);
-    if (totalProgress > 100) {
-      totalProgress = 100;
-    }
-    this.setState({
-      totalProgress,
-    });
-    this.emit("progress", totalProgress);
+  [Symbol.for("uppy test: updateTotalProgress")]() {
+    return _classPrivateFieldLooseBase(this, _updateTotalProgress)[_updateTotalProgress]();
   }
   updateOnlineStatus() {
     var _window$navigator$onL;
@@ -1239,6 +1221,47 @@ function _checkAndUpdateFileState2(filesToAdd) {
     errors,
   };
 }
+function _updateTotalProgress2() {
+  var _totalProgressPercent, _totalProgressPercent2;
+  const totalProgress = _classPrivateFieldLooseBase(this, _calculateTotalProgress)[_calculateTotalProgress]();
+  let totalProgressPercent = null;
+  if (totalProgress != null) {
+    totalProgressPercent = Math.round(totalProgress * 100);
+    if (totalProgressPercent > 100) totalProgressPercent = 100;
+    else if (totalProgressPercent < 0) totalProgressPercent = 0;
+  }
+  this.emit("progress", (_totalProgressPercent = totalProgressPercent) != null ? _totalProgressPercent : 0);
+  this.setState({
+    totalProgress: (_totalProgressPercent2 = totalProgressPercent) != null ? _totalProgressPercent2 : 0,
+  });
+}
+function _calculateTotalProgress2() {
+  const files = this.getFiles();
+  const filesInProgress = files.filter(file => {
+    return file.progress.uploadStarted || file.progress.preprocess || file.progress.postprocess;
+  });
+  if (filesInProgress.length === 0) {
+    return 0;
+  }
+  if (filesInProgress.every(file => file.progress.uploadComplete)) {
+    return 1;
+  }
+  const isSizedFile = file => file.progress.bytesTotal != null && file.progress.bytesTotal !== 0;
+  const sizedFilesInProgress = filesInProgress.filter(isSizedFile);
+  const unsizedFilesInProgress = filesInProgress.filter(file => !isSizedFile(file));
+  if (
+    sizedFilesInProgress.every(file => file.progress.uploadComplete) && unsizedFilesInProgress.length > 0
+    && !unsizedFilesInProgress.every(file => file.progress.uploadComplete)
+  ) {
+    return null;
+  }
+  const totalFilesSize = sizedFilesInProgress.reduce((acc, file) => {
+    var _file$progress$bytesT;
+    return acc + ((_file$progress$bytesT = file.progress.bytesTotal) != null ? _file$progress$bytesT : 0);
+  }, 0);
+  const totalUploadedSize = sizedFilesInProgress.reduce((acc, file) => acc + (file.progress.bytesUploaded || 0), 0);
+  return totalFilesSize === 0 ? 0 : totalUploadedSize / totalFilesSize;
+}
 function _addListeners2() {
   const errorHandler = (error, file, response) => {
     let errorMsg = error.message || "Unknown error";
@@ -1312,7 +1335,6 @@ function _addListeners2() {
       progress: {
         uploadStarted: Date.now(),
         uploadComplete: false,
-        percentage: 0,
         bytesUploaded: 0,
         bytesTotal: file.size,
       },
@@ -1320,7 +1342,7 @@ function _addListeners2() {
     this.patchFilesState(filesState);
   };
   this.on("upload-start", onUploadStarted);
-  this.on("upload-progress", this.calculateProgress);
+  this.on("upload-progress", _classPrivateFieldLooseBase(this, _handleUploadProgress)[_handleUploadProgress]);
   this.on("upload-success", (file, uploadResp) => {
     if (file == null || !this.getFile(file.id)) {
       this.log(`Not setting progress for a file that has been removed: ${file == null ? void 0 : file.id}`);
@@ -1348,7 +1370,7 @@ function _addListeners2() {
         size: uploadResp.bytesUploaded || currentProgress.bytesTotal,
       });
     }
-    this.calculateTotalProgress();
+    _classPrivateFieldLooseBase(this, _updateTotalProgressThrottled)[_updateTotalProgressThrottled]();
   });
   this.on("preprocess-progress", (file, progress) => {
     if (file == null || !this.getFile(file.id)) {
@@ -1413,7 +1435,7 @@ function _addListeners2() {
     });
   });
   this.on("restored", () => {
-    this.calculateTotalProgress();
+    _classPrivateFieldLooseBase(this, _updateTotalProgressThrottled)[_updateTotalProgressThrottled]();
   });
   this.on("dashboard:file-edit-complete", file => {
     if (file) {
diff --git a/packages/@uppy/progress-bar/lib/ProgressBar.js b/packages/@uppy/progress-bar/lib/ProgressBar.js
index 45a8401..6479ef7 100644
--- a/packages/@uppy/progress-bar/lib/ProgressBar.js
+++ b/packages/@uppy/progress-bar/lib/ProgressBar.js
@@ -19,8 +19,10 @@ export default class ProgressBar extends UIPlugin {
     this.render = this.render.bind(this);
   }
   render(state) {
-    const progress = state.totalProgress || 0;
-    const isHidden = (progress === 0 || progress === 100) && this.opts.hideAfterFinish;
+    const {
+      totalProgress,
+    } = state;
+    const isHidden = (totalProgress === 0 || totalProgress === 100) && this.opts.hideAfterFinish;
     return h(
       "div",
       {
@@ -33,12 +35,12 @@ export default class ProgressBar extends UIPlugin {
       h("div", {
         className: "uppy-ProgressBar-inner",
         style: {
-          width: `${progress}%`,
+          width: `${totalProgress}%`,
         },
       }),
       h("div", {
         className: "uppy-ProgressBar-percentage",
-      }, progress),
+      }, totalProgress),
     );
   }
   install() {
diff --git a/packages/@uppy/status-bar/lib/Components.js b/packages/@uppy/status-bar/lib/Components.js
index 0267dfb..32754d1 100644
--- a/packages/@uppy/status-bar/lib/Components.js
+++ b/packages/@uppy/status-bar/lib/Components.js
@@ -243,6 +243,7 @@ function ProgressDetails(props) {
     i18n,
   } = props;
   const ifShowFilesUploadedOfTotal = numUploads > 1;
+  const totalUploadedSizeStr = prettierBytes(totalUploadedSize);
   return h(
     "div",
     {
@@ -258,12 +259,16 @@ function ProgressDetails(props) {
         className: "uppy-StatusBar-additionalInfo",
       },
       ifShowFilesUploadedOfTotal && renderDot(),
-      i18n("dataUploadedOfTotal", {
-        complete: prettierBytes(totalUploadedSize),
-        total: prettierBytes(totalSize),
-      }),
+      totalSize != null
+        ? i18n("dataUploadedOfTotal", {
+          complete: totalUploadedSizeStr,
+          total: prettierBytes(totalSize),
+        })
+        : i18n("dataUploadedOfUnknown", {
+          complete: totalUploadedSizeStr,
+        }),
       renderDot(),
-      i18n("xTimeLeft", {
+      totalETA != null && i18n("xTimeLeft", {
         time: prettyETA(totalETA),
       }),
     ),
@@ -379,7 +384,7 @@ function ProgressBarUploading(props) {
       },
       h("div", {
         className: "uppy-StatusBar-statusPrimary",
-      }, supportsUploadProgress ? `${title}: ${totalProgress}%` : title),
+      }, supportsUploadProgress && totalProgress !== 0 ? `${title}: ${totalProgress}%` : title),
       renderProgressDetails(),
       showUploadNewlyAddedFiles
         ? h(UploadNewlyAddedFiles, {
diff --git a/packages/@uppy/status-bar/lib/StatusBar.js b/packages/@uppy/status-bar/lib/StatusBar.js
index 9a199d9..9f8c50d 100644
--- a/packages/@uppy/status-bar/lib/StatusBar.js
+++ b/packages/@uppy/status-bar/lib/StatusBar.js
@@ -142,16 +142,21 @@ export default class StatusBar extends UIPlugin {
     const newFilesOrRecovered = recoveredState ? Object.values(files) : newFiles;
     const resumableUploads = !!capabilities.resumableUploads;
     const supportsUploadProgress = capabilities.uploadProgress !== false;
-    let totalSize = 0;
+    let totalSize = null;
     let totalUploadedSize = 0;
+    if (startedFiles.every(f => f.progress.bytesTotal != null && f.progress.bytesTotal !== 0)) {
+      totalSize = 0;
+      startedFiles.forEach(file => {
+        totalSize += file.progress.bytesTotal || 0;
+        totalUploadedSize += file.progress.bytesUploaded || 0;
+      });
+    }
     startedFiles.forEach(file => {
-      totalSize += file.progress.bytesTotal || 0;
       totalUploadedSize += file.progress.bytesUploaded || 0;
     });
     const totalETA = _classPrivateFieldLooseBase(this, _computeSmoothETA)[_computeSmoothETA]({
       uploaded: totalUploadedSize,
       total: totalSize,
-      remaining: totalSize - totalUploadedSize,
     });
     return StatusBarUI({
       error,
@@ -213,8 +218,12 @@ export default class StatusBar extends UIPlugin {
 }
 function _computeSmoothETA2(totalBytes) {
   var _classPrivateFieldLoo, _classPrivateFieldLoo2;
-  if (totalBytes.total === 0 || totalBytes.remaining === 0) {
-    return 0;
+  if (totalBytes.total == null || totalBytes.total === 0) {
+    return null;
+  }
+  const remaining = totalBytes.total - totalBytes.uploaded;
+  if (remaining <= 0) {
+    return null;
   }
   (_classPrivateFieldLoo2 =
       (_classPrivateFieldLoo = _classPrivateFieldLooseBase(this, _lastUpdateTime))[_lastUpdateTime]) != null
@@ -250,7 +259,7 @@ function _computeSmoothETA2(totalBytes) {
       dt,
     );
   _classPrivateFieldLooseBase(this, _previousSpeed)[_previousSpeed] = filteredSpeed;
-  const instantETA = totalBytes.remaining / filteredSpeed;
+  const instantETA = remaining / filteredSpeed;
   const updatedPreviousETA = Math.max(_classPrivateFieldLooseBase(this, _previousETA)[_previousETA] - dt, 0);
   const filteredETA = _classPrivateFieldLooseBase(this, _previousETA)[_previousETA] == null
     ? instantETA
diff --git a/packages/@uppy/status-bar/lib/locale.js b/packages/@uppy/status-bar/lib/locale.js
index 99b4190..88e85d6 100644
--- a/packages/@uppy/status-bar/lib/locale.js
+++ b/packages/@uppy/status-bar/lib/locale.js
@@ -14,6 +14,7 @@ export default {
       1: "%{complete} of %{smart_count} files uploaded",
     },
     dataUploadedOfTotal: "%{complete} of %{total}",
+    dataUploadedOfUnknown: "%{complete} of unknown",
     xTimeLeft: "%{time} left",
     uploadXFiles: {
       0: "Upload %{smart_count} file",

@Murderlon
Copy link
Member

one issue is that the progress shows X kB of 0 B * 0s left:

I think we should at least have a plan for this before we merge this? Perhaps we can detect unknown file sizes in Uppy and exclude it from the total progress, once there is only the unknown file left we show some sort of continuous loading state without progress?

and fix bug where progress was not always emitted
only do it on total progress
- only show progress percent and total bytes for files that we know the size of. (but all files will still be included in number of files)
- use `null` as an unknown value for progress and ETA, allowing us to remove ETA from UI when unknown
- `percentage` make use of `undefined` when progress is not yet known - don't show percentage in UI when unknown
- add a new state field `progress` that's the same as `totalProgress` but can also be `null`
if not, then it leaves zombie companion instances running in the background when e2e stops
have to be manually killed before running e2e again
Copy link

socket-security bot commented Nov 12, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher

View full report↗︎

@mifi mifi requested a review from Murderlon November 12, 2024 14:59
@@ -640,6 +641,15 @@ enabled, it will lead to _faster uploads_ because companion will start uploading
at the same time as downloading using `stream.pipe`. If `false`, files will be
fully downloaded first, then uploaded. Defaults to `true`.

#### `streamingUploadSizeless` `COMPANION_STREAMING_UPLOAD_SIZELESS`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if it's not enabled? Hard crash? Silent ignore?

Why would we put it behind on option and not simply also allow it and return the error from the tus server if it doesn't support it (which is highly unlikely, all tus servers I know support it)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if it's not enabled? Hard crash? Silent ignore?

it will go back to the previous behavior which is fully download files first before uploading them. i will update the doc

Why would we put it behind on option and not simply also allow it and return the error from the tus server if it doesn't support it (which is highly unlikely, all tus servers I know support it)?

It's not only about Tus, it's now also for Multipart form and S3. Will update docs. I put it behind an option to make it a non-breaking change.

@@ -794,7 +794,7 @@ const state = {
capabilities: {
resumableUploads: false,
},
totalProgress: 0,
progress: null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of this change. Let's keep it totalProgress and reduce the tech debt, confusion, and PR diff.

@Murderlon
Copy link
Member

The new status bar behavior it not clear to me yet.

This is how it normally looks:

Screenshot 2024-11-18 at 12 16 03

I would simply replace "X KB of Y KB" with "X KB of unknown".


Have you tested with showProgressDetails set to false?

@mifi
Copy link
Contributor Author

mifi commented Nov 26, 2024

have addressed the feedback but we should probably implement an e2e test before merging

mifi added a commit to transloadit/uppy.io that referenced this pull request Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow streaming upload also for unknown length streams
2 participants