hgouveia / node-downloader-helper

A simple http file downloader for node.js
MIT License
253 stars 54 forks source link

Stop downloading without deleting file #22

Closed ihor-zinchenko closed 3 years ago

ihor-zinchenko commented 4 years ago

How I can stop downloading without file deleting?

I have electron app with node-downloader-helper Before closing the application, I've got an error http://joxi.ru/brRap3JI7dLdBA, I think that may be related to downloading the file. I do pause before closing the application.

hgouveia commented 4 years ago

How I can stop downloading without file deleting?

I have electron app with node-downloader-helper Before closing the application, I've got an error http://joxi.ru/brRap3JI7dLdBA, I think that may be related to downloading the file. I do pause before closing the application.

Yeap this lib, remove the file automatically when Stop is called or fails, but if you call Pause it does the same as Stop but without the file removing, you mentioned that you pause on close, so it shouldn't remove your file, what I think is happening, is that the close is not finished after close, and this probably is a bug of my part,

I will investigate and fix it in the new release, in the meanwhile, I suggest to do is just a setTimeout after pause all of them with some milliseconds, like 1000, (1sec), and then trigger Close on electron

ihor-zinchenko commented 4 years ago

still happens but not always. if I pause and unpause downloading I've got error always but when I start and pause once before close sometimes I didn't get error

NickPepper commented 4 years ago

I got exactly the same issue in my Electron-app. Btw, Jose, thank you very much for your work - you gave us a good helpful tool.

hgouveia commented 4 years ago

@IhorVimmi @NickPepper ,

I just added a fix for this on my dev branch, could you test it? using npm install --save hgouveia/node-downloader-helper#dev

when using stop now will wait until the file is closed, and I added a new option called removeOnStop that you can set to false to avoid the file deletion

ihor-zinchenko commented 4 years ago

@hgouveia, unfortunately, the same, I used stop instead of pause but I got the same result

hgouveia commented 4 years ago

@IhorVimmi is possible you can setup a minimal electron app, reproducing your issue and send it to me?

ihor-zinchenko commented 4 years ago

@hgouveia sure, I'll prepare and send it tomorrow, thank you for your work!

hgouveia commented 4 years ago

@hgouveia sure, I'll prepare and send it tomorrow, thank you for your work!

hello @IhorVimmi any luck with this?

hgouveia commented 4 years ago

@hgouveia sure, I'll prepare and send it tomorrow, thank you for your work!

hello @IhorVimmi any luck with this?

@IhorVimmi any luck with this?

davidchalifoux commented 4 years ago

Looking forward to this being on the main branch. I'm using this in a CLI tool and I am calling .stop() when the process is killed, but the file is not being deleted.

hgouveia commented 4 years ago

Looking forward to this being on the main branch. I'm using this in a CLI tool and I am calling .stop() when the process is killed, but the file is not being deleted.

Hello @davidchalifoux , this feature was already published on 1.0.12 , where you have two new options removeOnStop and removeOnFail both set as true by default, but keep in mind that stop returns a promise, so you will need to wait until finishes to close, could you do something like this

process.on('SIGTERM',() =>{
   // finally available since node10
   dl.stop().finally( () => process.exit() );
});
davidchalifoux commented 4 years ago

Looking forward to this being on the main branch. I'm using this in a CLI tool and I am calling .stop() when the process is killed, but the file is not being deleted.

Hello @davidchalifoux , this feature was already published on 1.0.12 , where you have two new options removeOnStop and removeOnFail both set as true by default, but keep in mind that stop returns a promise, so you will need to wait until finishes to close, could you do something like this

process.on('SIGTERM',() =>{
   // finally available since node10
   dl.stop().finally( () => process.exit() );
});

This worked, thanks again! Totally forgot to try that.

hgouveia commented 3 years ago

closing this, fixed on 1.0.12