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

Downloads "freezing" for no obvious reason? #40

Closed
Chaphasilor opened this issue Feb 4, 2021 · 27 comments
Closed

Downloads "freezing" for no obvious reason? #40

Chaphasilor opened this issue Feb 4, 2021 · 27 comments
Assignees
Labels
bug Something isn't working

Comments

@Chaphasilor
Copy link
Contributor

I've now experienced multiple times that while downloading a file, once in a while a download will just freeze and not continue to download. It doesn't fail, throws no error, doesn't change state, it just stops downloading. The dl object either stops emitting progress events or always reports the same stat, downloaded, progress, speed all stay the same.
In my application I have 5 download "slots" and if I download many files, those slots keep freezing on after the other, until every slot is filled with a frozen download and nothing gets downloaded anymore.

From my experience the downloads will stay frozen indefinitely, until I manually pause and then resume them (that is, if the download URL is still working and resume is supported).

I have no idea why this is happening or what exactly is causing this, so I can't tell you how to recreate the behavior. But I am pretty sure that the issue is with node-downloader-helper and not my app (the downloads really are frozen and don't just continue downloading in the background without the stats showing up).
The download also don't get slower before freezing (which could indicate server-side throttling), the last reported speed is similar to previous/average speeds.

Any idea why this could happen?

@Chaphasilor Chaphasilor changed the title Downloads "freezing" for now obvious reason? Downloads "freezing" for no obvious reason? Feb 4, 2021
@hgouveia
Copy link
Owner

hgouveia commented Feb 4, 2021

hello @Chaphasilor i need to check this out when i have the time but out of my head it seems this happened before but i cant remember the issue, i think was related with macOS , are you having this issue in macOS? what i could suggest for now is try to increase the timeout, there is an entry for this on the options

@Chaphasilor
Copy link
Contributor Author

I'm having the issue both on Windows 10 (x64) and Linux (Debian 10, arm64). So it doesn't really seem to be platform-dependent...

I'll try and see if the timeout helps, but as I said I can't force this issue to appear, so it might take some time until I can definitely say if it helped.
Would be nice if you could take a look at your code and think about if there's any way this might happen in the meantime :)

@Chaphasilor
Copy link
Contributor Author

A few insights:

  • It seems like downloads are more likely to freeze if the server is slow. ~200kB/s freezes rarely, ~40kB/s freezes constantly
  • I've tried setting the timeout, started with 30s, then 45, then 90s, it had no effect at all. Downloads would still freeze just as often.
  • I also managed to replicate the issue in CLI mode, at least I think so:
    Screenshot
    As you can see, the download suddenly stopped at 83.4% for no apparent reason, and I didn't force-quit it either (that would look like the lines below). The last reported download speed was also quite high for this server (usually between 20kB/s and 80kB/s).
    Like in my node app, the download didn't continue in the background without showing it (the target file really was only 184.4 MB in size at the end).

It might be some timeout that's causing this, but it seems like it's not the socket timeout of node's http/https.

@hgouveia
Copy link
Owner

hgouveia commented Feb 5, 2021

Hello @Chaphasilor ! this is definitely something i need to take a look, right now i am not in front of the PC but in the meantime, please checkout all the Issues we have here even the closed ones, because i really remember some case really similar if not, take a look inside the gitter chat, https://gitter.im/node-downloader-helper/Lobby , probably was discussed already there

anyways i will try my best to check this , this weekend and hopefully find the issue,

just to be correct, this happened to you only when you download multiple download at once right? if you download one by one it does work? i am thinking of probably nodejs sharing the same socket or something of like this that could be configured

@Chaphasilor
Copy link
Contributor Author

Hey @hgouveia, I checked the other issues before opening this issue but I still read every single one of them again, there is no similar issue here. There is #23, which is related to MacOS, but that issue is about network disconnects and retrying/auto-resuming.
I also read through the whole Gitter chat history and the closest I could find was about throttling download speeds.
So it seems like this is a new issue :)

just to be correct, this happened to you only when you download multiple download at once right? if you download one by one it does work?

So far I've not tested one by one downloads using node, but the issue with the CLI was just a single download, so I guess this will also happen without having multiple downloads at the same time.

I'll try to come up with a minimal example so you can test this better!

@Chaphasilor
Copy link
Contributor Author

Here's the code I've used for testing:

const { DownloaderHelper } = require(`node-downloader-helper`);

const url = `<a URL to a server that has very slow download speeds>`;
const dest = `C:\\Users\\Chaphasilor\\Downloads`;

let dl = new DownloaderHelper(url, dest, {
  fileName: `${process.argv[2]}_${(new Date().toDateString())}`,
  retry: {
    maxRetries: 5,
    delay: 30*1000
  },
  forceResume: false,
  removeOnStop: true,
  removeOnFail: true,
  override: true,
  httpRequestOptions: {
    timeout: 90*1000,
  },
  httpsRequestOptions: {
    timeout: 90*1000,
    rejectUnauthorized: false // ignore invalid certificates
  },
});

dl.on(`download`, (download) => console.log(`Download started!`));
dl.on(`error`, (err) => console.log(`Error:`, err));
dl.on(`progress.throttled`, (progress) => console.log(`progress:`, progress));
dl.start();

After a few tries I've managed to get a frozen download with only a single download instance running. However, it does seem like starting multiple instances makes freezing more likely.

@hgouveia
Copy link
Owner

hgouveia commented Feb 6, 2021

Hello @Chaphasilor thanks for the code for testing, I was checking it and tried multiple times like 20 attempt but was fine, as you mentioned you had tried in Windows and Linux, doesnt seems to be the OS but I am wondering if could be hardware or ISP related, do you have a chance to test this in a different compite and network?

Anyways I will keep investigate into this

@Chaphasilor
Copy link
Contributor Author

@hgouveia yeah I could try to recreate this with a different setup.

What URL did you test it with? What speeds were you getting?

@Chaphasilor
Copy link
Contributor Author

I've been able to recreate the issue with my work's WiFi. So it doesn't seem to be a network issue. I'll try to test it with as many devices as I can.

Here are some more screenshots:

Download 1/3 (all in parallel, using the code above and https://dl1.zoopix.ir/Series/Rick%20And%20Morty/season1/480/Rick.and.Morty.S01E01.480p.BRrip.mkv as the URL.
The download is still running at 16:27
Screenshot 2021-02-08 162752

Download 2/3
The download has been frozen for over half an hour. There are no newer logs, but the process is still running
Screenshot 2021-02-08 162819

Download 3/3
The download quit without any error message or status change.
The previous progress.throttled events were at 15:27, 15:23, 15:19 and 15:07 (and a lot more before that of course). This means that the progress.throttled event didn't fire for about 12 minutes! It normally should fire every second.
There was no status change logged and no error logged between 15:07 and 15:19. It seem like it was frozen, but recovered.

The timing also seems strange. With the timeout settings in my test code (retries with 30s delay, 5 retries max and a 90s http(s) timeout), I have no idea how there can't be anything happening for 12 whole minutes...

Screenshot 2021-02-08 162906

@Chaphasilor
Copy link
Contributor Author

@hgouveia if you want I could provide you with some log files, maybe that helps...
I could also add additional logging if you want me to?

@hgouveia
Copy link
Owner

@hgouveia if you want I could provide you with some log files, maybe that helps...
I could also add additional logging if you want me to?

hello @Chaphasilor with this is enough, for now i was able to kind of of reproduce the issue, i did not have this freezing, but i did have disconnection after a while but sadly is not something i could control, is just the Socket drops the connection, due connection reset from the server or similar stuff, so in that case, the only possibility is use the Retry functionality that i believe you already use,

i am wondering if you could replicate the download without this library but instead using plain native http request from nodejs and see if the result is the same? i could provide a code if you want , i am wondering if the issue is happening from the native http request itself

@Chaphasilor
Copy link
Contributor Author

i did not have this freezing, but i did have disconnection after a while but sadly is not something i could control, is just the Socket drops the connection, due connection reset from the server or similar stuff, so in that case, the only possibility is use the Retry functionality that i believe you already use,

@hgouveia yeah, but shouldn't there at least be an error event?

i am wondering if you could replicate the download without this library but instead using plain native http request from nodejs and see if the result is the same? i could provide a code if you want , i am wondering if the issue is happening from the native http request itself

Sure, I could try that. If you want to provide some code, go ahead, but I also have some testing code that I could use instead...

@hgouveia
Copy link
Owner

hgouveia commented Feb 12, 2021

@hgouveia yeah, but shouldn't there at least be an error event?

for my experience is just close the connection without any message, in fact that was one of the reason i then added the total size and downloaded size into the "end" event, so users could check that and retry until finishes

here i prepared a plain https request using nodejs, you could try with this, and see if the issue persist, i also added all possible event, probably one of them could be missing in my implementation https://gist.github.com/hgouveia/f807dfcb967c9fbc63b4aa4c0ac4e49b

@Chaphasilor
Copy link
Contributor Author

Okay, tested once more and it's really difficult to consistently reproduce the issue...

But I did observe that with the vanilla http implementation is can happen, that the timeout event is emitted and then...nothing. No other event ever follows, download doesn't continue, pipe/stream doesn't close, program doesn't quit.

So maybe it would be a good idea to add some additional handling for the timeout event inside node-downloader-helper? Like trying to restart the download if nothing happens for a while (using the range header, if supported).

@hgouveia
Copy link
Owner

Okay, tested once more and it's really difficult to consistently reproduce the issue...

But I did observe that with the vanilla http implementation is can happen, that the timeout event is emitted and then...nothing. No other event ever follows, download doesn't continue, pipe/stream doesn't close, program doesn't quit.

So maybe it would be a good idea to add some additional handling for the timeout event inside node-downloader-helper? Like trying to restart the download if nothing happens for a while (using the range header, if supported).

yeap that was the whole idea of the retry event in the first place, but the native timeout event is not consistent either, sometimes it just timed out and the download doesn't stop and just freeze, and sometimes even continue like nothing happened

the question now is, should the library handle this situation and just force it stop when timed out and if retry if available, or the user should handle this in the emited timeout event from the library?

probably makes sense the library just take care of this, so is transparent for the user, since when timed out, the user will expect that the download stop anyways

@Chaphasilor
Copy link
Contributor Author

Yeah, given that this is a downloader and not some http client, edge cases like this should be handled automatically to make it more robust.
We could consider a timeout that results in a "freeze" as a fail and use up one of the retries (if there are any).

@Chaphasilor
Copy link
Contributor Author

@hgouveia any thoughts on my comment?
I'd really like to solve this issue :)

@hgouveia
Copy link
Owner

@Chaphasilor I actually missed this in the newer version I just release and is something I wanted to add, I just forgot it hehe, so i will implement this behavior this upcoming week and release new version

Basically if the timout event is triggered from the request, I will stop the download, if retry option is enable it will retry, if not just stop and emit timeout

What you think?

@Chaphasilor
Copy link
Contributor Author

Basically if the timout event is triggered from the request, I will stop the download, if retry option is enable it will retry, if not just stop and emit timeout

@hgouveia sounds good! I suggest the specific events would be:

  • If retry is disabled (or no retries are left): emit timeout and end
  • If retry is enabled and there are retries left: emit timeout and retry

I don't recommend emitting stop, because if the documentation it says:

stop: Emitted when the .stop method is called

@dae54
Copy link

dae54 commented Apr 13, 2021

I face almost the same issue, but for my case, the package tends to freeze on issue download (doesn't respond immediately to dl.start()). Sometimes it takes up to 20 seconds to start downloading. I tried pausing and resuming by issuing

dl.pause().then(()=>{
 dl.resume()
})

Sometimes it works correctly, the other times, it downloads the same file twice, in parallel ;( ...
P.S

  • I use electronjs latest version, if that may matter
  • I use Ubuntu 20.04 LTS, NodeJS v14.15.0

@Chaphasilor
Copy link
Contributor Author

@dae54 could it be that the server is slow to respond in your case? It also takes a few seconds to connect for me sometimes, although usually it's faster and I'm not sure if it ever took 20 seconds...

@dae54
Copy link

dae54 commented Apr 20, 2021

@Chaphasilor sometimes it used to freeze forever... the behaviour was random... i found a temporary fix by specifying retry option. The problem that i currently face is that sometimes two simultaneous downloads happen. I think its just my bad logics,

@hgouveia
Copy link
Owner

hello @dae54 , i think this issue is related with how much time take your server to respond ratter than the lib itself,

something you could do, is create an Agent , and reused it in all your DownloadHelper instances, in case your are using the same domain, this will avoid to do the handshake for every request,

const agent = new https.Agent({keepAlive: true); 
const dl = new DownloaderHelper(url, destFolder, {
  httpsRequestOptions: { agent }
});

for more documentation https://nodejs.org/api/https.html#https_class_https_agent

let me know if this improve the performance

@hgouveia hgouveia self-assigned this Aug 19, 2021
@hgouveia hgouveia added the bug Something isn't working label Aug 19, 2021
@hgouveia
Copy link
Owner

Hello @Chaphasilor , i just added a fix for this, i wonder if you could try it by install it like this npm install --save hgouveia/node-downloader-helper#dev , since for me is hard to repro

@Chaphasilor
Copy link
Contributor Author

Hey! Sure, I'll try it out right away, but it's hard for me to reproduce too :)
I'll try my best!

What did you change? 🤔

@hgouveia
Copy link
Owner

Hey! Sure, I'll try it out right away, but it's hard for me to reproduce too :)
I'll try my best!

What did you change? 🤔

Now the behavior is that if i detect a timeout it will abort the request and retry if available, if not, it will fail

@Chaphasilor
Copy link
Contributor Author

@hgouveia I can't really recreate the issue anymore. Maybe it got fixed by nodejs/node#38665, maybe it's a server-side issue, or maybe you fixed it. I haven't had the problem in a while, and the new release doesn't seem to break anything...

I'll close the issue now; if anything like this happens again we can either re-open it or someone will create a new issue :)

Also a heads-up, your command (npm install --save hgouveia/node-downloader-helper#dev) didn't work, the folder inside node_modules doesn't contain a src directory, so you can't build the source files, and the dist directory doesn't exist either. I had to manually clone the repo and build it myself :)

hgouveia added a commit that referenced this issue Oct 4, 2021
commit 33aab44
Author: Jose De Gouveia <[email protected]>
Date:   Mon Oct 4 11:12:26 2021 +0200

    Incremented 1.0.19

commit ac8387f
Author: Jose De Gouveia <[email protected]>
Date:   Tue Aug 24 17:43:35 2021 +0200

    Fixed attempt to ECONNRESET at TLSWrap.onStreamRead #61

commit 305676e
Author: Jose De Gouveia <[email protected]>
Date:   Tue Aug 24 14:42:20 2021 +0200

    Fixed raise error event when paused in newer nodejs versions #62

commit 9d1bd9c
Author: Jose De Gouveia <[email protected]>
Date:   Fri Aug 20 16:44:12 2021 +0200

    Fixed muiltiple 'error' emits when retry #57

commit e8c00d8
Author: Jose De Gouveia <[email protected]>
Date:   Thu Aug 19 16:44:57 2021 +0200

    Fixed error emitting when retry #57

commit bc20b86
Author: Jose De Gouveia <[email protected]>
Date:   Thu Aug 19 14:51:48 2021 +0200

    When timeout, it will retry the download if available if not emit timeout #40

commit 1306bc8
Author: Henrique Bruno Fantauzzi de Almeida <[email protected]>
Date:   Tue Aug 17 04:42:25 2021 -0300

    fileName {ext} type now also optional and boolean (#51)

    This fixes the typing for fileName property, now accepting:

    `fileName: {name: 'xyz'}`
    `fileName: {name: 'xyz', ext: true}` or false, stating false is the default
    `fileName: {name: 'xyz', ext: 'jpg'}` as already was accepted

commit bed68d5
Author: Chaphasilor <[email protected]>
Date:   Tue Aug 17 09:41:05 2021 +0200

    Fix leading dots being removed (#59)

    * fix leading dots being removed

    * only modify auto-generated filenames + added test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants