onProgress() callback for BlockBlobClient.uploadData() in the browser isn't granular

[au5ton]

• Package Name: @azure/storage-blob
• Package Version: 12.26.0
• Operating system: Windows 23H2
• nodejs
  • version:
• browser
  • name/version: Edge/131.0.2903.112
• typescript
  • version:
• Is the bug related to documentation in
  • README.md
  • source code documentation
  • SDK API docs on https://learn.microsoft.com

Describe the bug

The onProgress() callback when using BlockBlobClient.uploadData() in the browser is not granular and only updates after each block. This makes it impractical for progress bars and progress reporting without making chunks very small and inefficient. 

To Reproduce
Steps to reproduce the behavior:

1. Acquire a sufficiently large test file: https://ash-speed.hetzner.com/1GB.bin
2. Acquire a SAS URL for a browser to upload a blob directly to Azure Blob Storage
3. Upload the blob in blocks:

const SAS_URL = '(...)';
const input = document.querySelector(`input[type=file]`);
const { BlockBlobClient, AnonymousCredential } = await import('@azure/storage-blob');
const blobClient = new BlockBlobClient(SAS_URL, new AnonymousCredential());
const res= await blobClient.uploadData(input.files.item(0), {
  maxSingleShotSize: 30_000_000, // ~30 megabytes
  blockSize: 30_000_000,
  concurrency: 1,
  onProgress(progress) {
    // only reports in chunks of 30_000_000 bytes, no intermediate progress
    console.log('Progress:', progress);
  }
});

4. The onProgress callback does not report the progress granularly in a useful way.

Expected behavior
Expected that upload progress be recorded more interactively and at a sub-block refresh rate.

Screenshots
N/A

Additional context

From what I've researched, I am aware of a couple of different factors that contribute to this being the case:

• To date, the Fetch API doesn't have a cross-platform, evergreen way to track upload progress that is widely available yet
  • Streaming upload whatwg/fetch#1438
  • https://caniuse.com/mdn-api_request_duplex
• XMLHttpRequest does have a way to do this: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequestUpload/progress_event
• The BlockBlobClient uses the Fetch HTTP client by default. This could be overridden by an option to be the XHR one ... if it was actually exported and available in @azure/core-rest-pipeline:
  • https://github.com/Azure/azure-sdk-for-js/blob/78dd27221a32c7abd81ee6503fe4fc239d6ebef8/sdk/core/core-rest-pipeline/src/index.ts
  • https://github.com/Azure/azure-sdk-for-js/blob/78dd27221a32c7abd81ee6503fe4fc239d6ebef8/sdk/core/core-rest-pipeline/src/xhrHttpClient.ts
• Even for the platforms that support Fetch upload progress, the Blob Storage business logic doesn't leverage it and has stub code with a relevant "TODO":
  • In the snippet below, a callback for onProgress() could be specified as an option after line 4287. (I don't mean hypothetically, I mean that stageBlock() implements it already)

  • azure-sdk-for-js/sdk/storage/storage-blob/src/Clients.ts

    Lines 4275 to 4299 in 594557a

    	const batch = new Batch(options.concurrency);
    	for (let i = 0; i < numBlocks; i++) {
    	batch.addOperation(async (): Promise<any> => {
    	const blockID = generateBlockID(blockIDPrefix, i);
    	const start = blockSize * i;
    	const end = i === numBlocks - 1 ? size : start + blockSize;
    	const contentLength = end - start;
    	blockList.push(blockID);
    	await this.stageBlock(blockID, bodyFactory(start, contentLength), contentLength, {
    	abortSignal: options.abortSignal,
    	conditions: options.conditions,
    	encryptionScope: options.encryptionScope,
    	tracingOptions: updatedOptions.tracingOptions,
    	});
    	// Update progress after block is successfully uploaded to server, in case of block trying
    	// TODO: Hook with convenience layer progress event in finer level
    	transferProgress += contentLength;
    	if (options.onProgress) {
    	options.onProgress!({
    	loadedBytes: transferProgress,
    	});
    	}
    	});
    	}
    	await batch.do();

• Behind BlockBlobClient.stageBlock(...) the stack grows more complex and abstracted to fit the needs of a variety of environments and runtimes, but it would seem that onProgress it pretty respected for the most part underneath (from what I could tell at least).

I (naively) think this could be mostly resolved with a few steps:

1. Export createXhrHttpClient() in @azure/core-rest-pipeline to allow browser consumers to import it again. Maybe there's a history with this being deprecated/removed?
2. Implement block-level tracking in BlockBlobClient.uploadSeekableInternal() and as a result in BlockBlobClient.uploadData() (see snippets to stub code above)

What is the team's stance on this? Thanks.

[github-actions]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

[jeremymeng]

related: #32247

[xirzec]

@au5ton we do have some code for stream bodies specifically to enable progress with fetch:

azure-sdk-for-js/sdk/core/ts-http-runtime/src/fetchHttpClient.ts

Line 250 in 1a03c44

function buildBodyStream(

I wonder if passing the progress callback as you suggest in stageBlock would address this, though I think with block upload there are two subtly different scenarios:

1. using onProgress to track how many blocks have successfully made their way to the service
2. tracking the individual progress of streaming each block to the service.

Perhaps there could be a separate callback to this operation (e.g. blockOnProgress) that is passed through to the underlying HttpClient?

[au5ton]

@xirzec

we do have some code for stream bodies specifically to enable progress with fetch:

Thanks for clarifying this!

Perhaps there could be a separate callback to this operation (e.g. blockOnProgress) that is passed through to the underlying HttpClient?

I'm not suggesting that the exported API surface of BlobBlobClient.uploadData() be changed, just that it publishes data in the same format/scale more often. From a consumer perspective, it seems unintuitive to me that that progress is reported exclusively in terms of block size, because the API doesn't suggest that and is more granular in a Node.js context.

Currently, transfer progress is calculated over the entire file...

azure-sdk-for-js/sdk/storage/storage-blob/src/Clients.ts

Line 4273 in 594557a

let transferProgress: number = 0;

... and incremented and reported at the conclusion of each block.

azure-sdk-for-js/sdk/storage/storage-blob/src/Clients.ts

Lines 4289 to 4296 in 594557a

	// Update progress after block is successfully uploaded to server, in case of block trying
	// TODO: Hook with convenience layer progress event in finer level
	transferProgress += contentLength;
	if (options.onProgress) {
	options.onProgress!({
	loadedBytes: transferProgress,
	});
	}

It could be possible that this is replaced so that each block is tracked separately and reported whenever stageBlock(..., { onProgress: () => {}) reports something.

Maybe something like this? If this approach is acceptable, I can make a PR.

diff --git a/sdk/storage/storage-blob/src/Clients.ts b/sdk/storage/storage-blob/src/Clients.ts
index 7e5dcee0a7..e95d71185e 100644
--- a/sdk/storage/storage-blob/src/Clients.ts
+++ b/sdk/storage/storage-blob/src/Clients.ts
@@ -4270,28 +4270,45 @@ export class BlockBlobClient extends BlobClient {

         const blockList: string[] = [];
         const blockIDPrefix = randomUUID();
-        let transferProgress: number = 0;
+        // Stores the amount of bytes progressed in each block
+        let transferProgressPerBlock: number[] = [];

         const batch = new Batch(options.concurrency);
         for (let i = 0; i < numBlocks; i++) {
+          // Initialize at 0
+          transferProgressPerBlock[i] = 0;
+          // Calculate block parameters
+          const start = blockSize * i;
+          const end = i === numBlocks - 1 ? size : start + blockSize;
+          const contentLength = end - start;
+          // Queue the block upload
           batch.addOperation(async (): Promise<any> => {
             const blockID = generateBlockID(blockIDPrefix, i);
-            const start = blockSize * i;
-            const end = i === numBlocks - 1 ? size : start + blockSize;
-            const contentLength = end - start;
             blockList.push(blockID);
             await this.stageBlock(blockID, bodyFactory(start, contentLength), contentLength, {
               abortSignal: options.abortSignal,
               conditions: options.conditions,
               encryptionScope: options.encryptionScope,
               tracingOptions: updatedOptions.tracingOptions,
+              onProgress(progress) {
+                // Record the progress in this block by index. Will overwrite if block is retried.
+                transferProgressPerBlock[i] = progress.loadedBytes;
+                // Report progress externally
+                if (options.onProgress) {
+                  options.onProgress!({
+                    // Report the sum of the array
+                    loadedBytes: transferProgressPerBlock.reduce((sum, a) => sum + a, 0),
+                  });
+                }
+              },
             });
             // Update progress after block is successfully uploaded to server, in case of block trying
+            // In case of inconsistencies in `onProgress` report, write the final value ourselves
+            transferProgressPerBlock[i] = contentLength;
-            // TODO: Hook with convenience layer progress event in finer level
-            transferProgress += contentLength;
             if (options.onProgress) {
               options.onProgress!({
-                loadedBytes: transferProgress,
+                // Report the sum of the array
+                loadedBytes: transferProgressPerBlock.reduce((sum, a) => sum + a, 0),
               });
             }
           });