From 3152307e3fd25d4288d117c74e9f930d12d3b264 Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 19 Jun 2026 09:08:18 -0400 Subject: [PATCH 1/2] fix(upload): harden Upload progress and status tracking - ignore stale progress updates after assembling or terminal states - clamp tracked uploaded bytes to the valid range - tighten Status typing on the status getter - improve Upload model documentation and inline comments Signed-off-by: Josh --- lib/upload.ts | 77 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 14 deletions(-) diff --git a/lib/upload.ts b/lib/upload.ts index 59dfea2a..5ea6ccf0 100644 --- a/lib/upload.ts +++ b/lib/upload.ts @@ -5,6 +5,9 @@ import type { AxiosResponse } from 'axios' import { getMaxChunksSize } from './utils/config.js' +/** + * Lifecycle states for a single logical upload. + */ export enum Status { INITIALIZED = 0, UPLOADING = 1, @@ -13,8 +16,12 @@ export enum Status { CANCELLED = 4, FAILED = 5, } -export class Upload { +/** + * Represents a single logical upload (a file or directory upload) and tracks + * its progress, status, and cancellation signal. + */ +export class Upload { private _source: string private _file: File private _isChunked: boolean @@ -29,19 +36,29 @@ export class Upload { private _response: AxiosResponse|null = null constructor(source: string, chunked = false, size: number, file: File) { - const chunks = Math.min(getMaxChunksSize() > 0 ? Math.ceil(size / getMaxChunksSize()) : 1, 10000) + const maxChunkSize = getMaxChunksSize() + // Limit the computed chunk count to the maximum supported by chunked upload v2. + const chunks = Math.min(maxChunkSize > 0 ? Math.ceil(size / maxChunkSize) : 1, 10000) + this._source = source - this._isChunked = chunked && getMaxChunksSize() > 0 && chunks > 1 + this._isChunked = chunked && maxChunkSize > 0 && chunks > 1 + // Non-chunked uploads are tracked as a single logical chunk. this._chunks = this._isChunked ? chunks : 1 this._size = size this._file = file this._controller = new AbortController() } + /** + * Destination path of this upload item on the remote server. + */ get source(): string { return this._source } + /** + * Underlying file object associated with this upload item. + */ get file(): File { return this._file } @@ -50,35 +67,68 @@ export class Upload { return this._isChunked } + /** + * Number of logical chunks used to upload this item. + */ get chunks(): number { return this._chunks } + /** + * Total byte size of this upload item. + * + * Meta or directory uploads may report `0`. + */ get size(): number { return this._size } + /** + * Time when the first upload progress event was observed. + * + * This is a Unix timestamp in milliseconds. It is set on the first observed + * progress update, not when the upload object is created. + */ get startTime(): number { return this._startTime } + /** + * Store the latest HTTP response associated with this upload. + */ set response(response: AxiosResponse|null) { this._response = response } + /** + * The latest HTTP response associated with this upload, if any. + */ get response(): AxiosResponse|null { return this._response } + /** + * Tracked uploaded byte count for this upload item. + */ get uploaded(): number { return this._uploaded } /** - * Update the uploaded bytes of this upload + * Update the tracked uploaded byte count for this upload. + * + * Progress updates are ignored once the upload enters the assembling state or + * reaches a terminal state, because byte-transfer progress should no longer + * change in those states. The stored byte count is clamped to the range `0..size`. */ set uploaded(length: number) { - if (length >= this._size) { + if ([Status.ASSEMBLING, Status.CANCELLED, Status.FAILED, Status.FINISHED].includes(this._status)) { + return + } + + const clampedLength = Math.min(Math.max(length, 0), this._size) + + if (clampedLength >= this._size) { this._status = this._isChunked ? Status.ASSEMBLING : Status.FINISHED @@ -87,38 +137,37 @@ export class Upload { } this._status = Status.UPLOADING - this._uploaded = length + this._uploaded = clampedLength - // If first progress, let's log the start time + // Record the time of the first observed progress update. if (this._startTime === 0) { - this._startTime = new Date().getTime() + this._startTime = Date.now() } } - get status(): number { + get status(): Status { return this._status } /** - * Update this upload status + * Set the current upload status explicitly. */ set status(status: Status) { this._status = status } /** - * Returns the axios cancel token source + * Returns the AbortSignal used to cancel ongoing requests. */ get signal(): AbortSignal { return this._controller.signal } /** - * Cancel any ongoing requests linked to this upload + * Abort any ongoing requests linked to this upload and mark it as cancelled. */ - cancel() { + cancel(): void { this._controller.abort() this._status = Status.CANCELLED } - } From 7d5e8de2990a6c3e00bf4412765ae7e04a38a63a Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 19 Jun 2026 09:46:00 -0400 Subject: [PATCH 2/2] fix(uploader): harden uploader progress handling - ignore stale progress updates after uploads reach terminal states - avoid unnecessary progress mutations and stats recalculations from late async callbacks - preserve existing retry and queue orchestration behavior Signed-off-by: Josh --- lib/uploader/uploader.ts | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/uploader/uploader.ts b/lib/uploader/uploader.ts index 3181c79f..e844d454 100644 --- a/lib/uploader/uploader.ts +++ b/lib/uploader/uploader.ts @@ -226,11 +226,16 @@ export class Uploader { // If already paused keep it that way if (this._queueStatus !== UploaderStatus.PAUSED) { - const pending = this._uploadQueue.find(({ status }) => [UploadStatus.INITIALIZED, UploadStatus.UPLOADING, UploadStatus.ASSEMBLING].includes(status)) + const pending = this._uploadQueue.find(({ status }) => + [ + UploadStatus.INITIALIZED, + UploadStatus.UPLOADING, + UploadStatus.ASSEMBLING, + ].includes(status)) if (this._jobQueue.size > 0 || pending) { this._queueStatus = UploaderStatus.UPLOADING } else { - this.eta.reset() + this._eta.reset() this._queueStatus = UploaderStatus.IDLE } } @@ -520,6 +525,10 @@ export class Uploader { destinationFile: encodedDestinationFile, retries, onUploadProgress: ({ bytes }) => { + if ([UploadStatus.ASSEMBLING, UploadStatus.CANCELLED, UploadStatus.FAILED, UploadStatus.FINISHED].includes(upload.status)) { + return + } + // Only count 90% of bytes as the request is not yet processed by server // we set the remaining 10% when the request finished (server responded). const progressBytes = bytes * 0.9 @@ -528,6 +537,10 @@ export class Uploader { this.updateStats() }, onUploadRetry: () => { + if ([UploadStatus.ASSEMBLING, UploadStatus.CANCELLED, UploadStatus.FAILED, UploadStatus.FINISHED].includes(upload.status)) { + return + } + // Current try failed, so reset the stats for this chunk // meaning remove the uploaded chunk bytes from stats upload.uploaded -= chunkBytes @@ -544,6 +557,10 @@ export class Uploader { ) // Update upload progress on chunk completion .then(() => { + if ([UploadStatus.ASSEMBLING, UploadStatus.CANCELLED, UploadStatus.FAILED, UploadStatus.FINISHED].includes(upload.status)) { + return + } + // request fully done so we uploaded the full chunk // we first remove the intermediate chunkBytes from progress events // and then add the real full size @@ -632,12 +649,20 @@ export class Uploader { { signal: upload.signal, onUploadProgress: ({ bytes }) => { + if ([UploadStatus.ASSEMBLING, UploadStatus.CANCELLED, UploadStatus.FAILED, UploadStatus.FINISHED].includes(upload.status)) { + return + } + // As this is only the sent bytes not the processed ones we only count 90%. // When the upload is finished (server acknowledged the upload) the remaining 10% will be correctly set. upload.uploaded += bytes * 0.9 this.updateStats() }, onUploadRetry: () => { + if ([UploadStatus.ASSEMBLING, UploadStatus.CANCELLED, UploadStatus.FAILED, UploadStatus.FINISHED].includes(upload.status)) { + return + } + upload.uploaded = 0 this.updateStats() },