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

Progress listener Total is 0 with large files #328

Open
3 tasks done
Olyster opened this issue Feb 21, 2022 · 10 comments
Open
3 tasks done

Progress listener Total is 0 with large files #328

Olyster opened this issue Feb 21, 2022 · 10 comments
Labels

Comments

@Olyster
Copy link

Olyster commented Feb 21, 2022

Bug Report

Problem

The "total" property of ProgressEvent Interface equals zero when downloading large file (ex: 6 gig)

interface ProgressEvent extends Event {
readonly lengthComputable: boolean;
readonly loaded: number;
readonly target: T | null;
readonly total: number;
}

What is expected to happen?

total property should be equal to file size

What does actually happen?

total = 0 when downloading large file (tested with 6.6 gig file)
loaded goes up to the file size so it's not a data type problem and total has correct filesize with smaller files

Information

download a large file (ex: 6 gig) and set onProgress listener

Command or Code

let currentProgress
ft.onProgress((evt:ProgressEvent)=> {
currentProgress = (evt.loaded / evt.total) <-- currentProgress = "Infinity" since evt.total = 0
})

Environment, Platform, Device

Android 10 - ionic-native - cordova

Version information

Android 10
Cordova 9
"cordova-plugin-file-transfer": "git+https://github.com/apache/cordova-plugin-file-transfer.git",
import { FileTransfer, FileTransferObject } from '@ionic-native/file-transfer/ngx';
Ionic 5.4.15
Win10
VSCode Version: 1.56.2 (system setup)
Electron: 12.0.4
Chrome: 89.0.4389.114
Node.js: 14.16.0
V8: 8.9.255.24-electron.0
OS: Windows_NT x64 10.0.18363

Checklist

  • I searched for existing GitHub issues
  • I updated all Cordova tooling to most recent version
  • I included all the necessary information above
@SPIE-InfoGraph
Copy link

I have the same problem. Has anyone written something special for Electron?

@jsleijh
Copy link

jsleijh commented Apr 30, 2024

I also have the same problem. Is it solved?

@breautek
Copy link
Contributor

breautek commented Apr 30, 2024

Not all download requests signals the client how many bytes there are to be downloaded.

Generally speaking the server must send a Content-Length response header for progress events to have a computable length. For large files, it's fairly common practice to use Chunked Transfer Encoding instead in which case the response will not have a Content-Length. Services that uses Chunked Transfer Encoding generally use it because the service itself doesn't know how large the asset is.

In the case of electron/browser platform, the underlying implementation is the browser XMLHttpRequest API itself, and lengthComputable may have other requirements for it to become true. The standard just specifies when it should be true, but doesn't necessary dictate how, and it might be up for interpretation.

For iOS/Android platform, Cordova looks for the Content-Length response header.

It's up to the application to check the lengthComputable field before attempting to calculate the transfer rate or progress. Because the data might not be provided by the server that you're making the download request from.

I don't believe this is a bug but I won't close this issue just yet. If someone can demonstrate this issue occurring on android/ios on a download request in which provides a Content-Length response header, then I think this warrants investigating further. Otherwise I don't think this a bug.

@jsleijh
Copy link

jsleijh commented Apr 30, 2024

微信截图_20240501014051
I really provide a Content-Length response header, thanks a lot!
If Content-Length is 2147483640(<2147483648),progressEvent.total is right(not zero).

@breautek
Copy link
Contributor

Thanks, I think that suggests that your server end-point is indeed sending the Content-Length. It looks like the screenshot is from a debug window on your native server application. If possible can you hit your API with a standard browser, (or use a tool such as Postman) and confirm that the Content-Length makes it through to the client?

If it's missing by the time it reaches the standard browser/postman, then the header could be getting lost between the client and the server application and would explain why the client won't have a computable length.

If it's also present in postman, then Cordova should be receiving it as well. In this case, please let us know what platforms you're observing this issue (Android, iOS, etc..)

@jsleijh
Copy link

jsleijh commented Apr 30, 2024

Thanks, This issus appears in Android.
Other questions you have asked, I will try again soon.

@breautek
Copy link
Contributor

breautek commented Apr 30, 2024

If Content-Length is 2147483640(<2147483648),progressEvent.total is right(not zero).

Actually, this is probably because we use getContentLength when generating progress events here.

the getContentLength API uses an int so it has a max range of 2147483640 and if the content length exceeds it, then -1 is returned (and as a result is treated as if the request is a chunked transfer request). So that probably explains what you're seeing...

There is a long version of the API but returning a long to the webview is dangerous because it exceeds the max safe integer of the javascript engine. If the file size is between Number.MAX_SAFE_INTEGER and the Long.MAX then it will also introduce subtle issues as the numbers may incur floating point errors once it exceeds Number.MAX_SAFE_INTEGER

Despite this danger, it seems like iOS is using long long in it's implementation.

The JS Number.MAX_SAFE_INTEGER is 9007199254740991, or about 8TB if I did my math right, so for the purpose of reporting file size, I doubt anyone is downloading 8TB files, so I guess using a Long would be ok.

@breautek breautek added bug and removed Info Needed labels Apr 30, 2024
@jsleijh
Copy link

jsleijh commented Apr 30, 2024

If i change it myself, would i change two places (775 and 777),and replace getContentLength with getContentLengthLong ?

@breautek
Copy link
Contributor

If i change it myself, would i change two places (775 and 777),and replace getContentLength with getContentLengthLong ?

I believe so, I'd also support a PR that introduces that change.

@jsleijh
Copy link

jsleijh commented Apr 30, 2024

Thanks so much, this will solve a tricky problem for me! @breautek

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants