Skip to content

Fix progress reporting for compressed downloads in fetchRemotePackage #24144

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vjyanand
Copy link

@vjyanand vjyanand commented Apr 18, 2025

When a server uses compression (e.g., Brotli (br) or Gzip (gzip)), the Content-Length header in the HTTP response represents the compressed size of the data, not the uncompressed size. The fetchRemotePackage function, however, tracks the uncompressed size of the downloaded data (via value.length of each chunk after decompression). This leads to a discrepancy where the reported totalLoaded (uncompressed) exceeds the Content-Length (compressed), causing inaccurate progress reporting.

The Fetch API does not provide direct access to the compressed stream’s size for each chunk, as decompression is handled transparently. The updated code addresses this by aligning the total and loaded values with the compressed size from Content-Length when the content is compressed. Specifically:

  • The contentEncoding is checked to determine if compression (gzip or br) is used.
  • If the content is not compressed and Content-Length is available, total is set to the Content-Length value. Otherwise, it falls back to packageSize.

This ensures progress reporting reflects the compressed size, preventing totalLoaded from exceeding total.

@vjyanand vjyanand marked this pull request as ready for review April 18, 2025 12:13
if (isCompressed) {
// Estimate progress proportionally, assuming 3:1 compression ration
loaded = Math.min(total,
total * (decompressedLoaded / (packageSize || total * 3)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems a little too much like guesswork. Is there really no better way?

Copy link
Author

Choose a reason for hiding this comment

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

@sbc100 how about now?

@vjyanand vjyanand force-pushed the fix-download-progress-compression branch 2 times, most recently from 8d30366 to 0a9dfbf Compare April 20, 2025 05:57
@vjyanand vjyanand force-pushed the fix-download-progress-compression branch from 0a9dfbf to 290785d Compare April 23, 2025 03:02
@sbc100
Copy link
Collaborator

sbc100 commented Apr 23, 2025

Can you update the PR description and title now that we have changed the PR content?

@vjyanand vjyanand changed the title Under report download size when compression involved Fix Progress Reporting for Compressed Downloads in fetchRemotePackage Apr 23, 2025
@vjyanand vjyanand requested a review from sbc100 April 23, 2025 17:51
@sbc100 sbc100 changed the title Fix Progress Reporting for Compressed Downloads in fetchRemotePackage Fix progress reporting for compressed downloads in fetchRemotePackage Apr 23, 2025
@sbc100
Copy link
Collaborator

sbc100 commented Apr 23, 2025

Do you know why we cannot just use packageSize in all cases? Are there some cases where packageSize is not known?

@vjyanand
Copy link
Author

Do you know why we cannot just use packageSize in all cases? Are there some cases where packageSize is not known?

just my guess, packageSize is calculated while doing packaging, one can still change the file with different size on the server without doing packaging. in that case, content-length will be correct value,

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.

2 participants