hgouveia / node-downloader-helper

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

Can I use this package limit the download speed? #20

Closed ZhangZheng-GIS closed 4 years ago

ZhangZheng-GIS commented 4 years ago

Can I use the module limit the download speed?

hgouveia commented 4 years ago

@ZhangZheng-GIS in theory you could by using the dl.pipe() function with some external module like stream-throttle or node-throttle but not sure about this, could you try it? I would be very interesting to see the result of this

ZhangZheng-GIS commented 4 years ago

I'll try it, thx

ZhangZheng-GIS commented 4 years ago

@ZhangZheng-GIS in theory you could by using the dl.pipe() function with some external module like stream-throttle or node-throttle but not sure about this, could you try it? I would be very interesting to see the result of this

There is a problem with the dl.pipe() function when I use the stream-throttle module, downloading files will not proceed.

...
let tg = new ThrottleGroup({ rate: 102400 });
dl.pipe(tg.throttle());
dl.start();

When I use native nodejs to download, it works.

let tg = new ThrottleGroup({ rate: 102400 });
...
someReadStream.pipe(tg.throttle()).pipe(writeStream);

I noticed that the order of the pipes is important. I hope you can improve this package. Thanks!

hgouveia commented 4 years ago

@ZhangZheng-GIS in theory you could by using the dl.pipe() function with some external module like stream-throttle or node-throttle but not sure about this, could you try it? I would be very interesting to see the result of this

There is a problem with the dl.pipe() function when I use the stream-throttle module, downloading files will not proceed.

...
let tg = new ThrottleGroup({ rate: 102400 });
dl.pipe(tg.throttle());
dl.start();

When I use native nodejs to download, it works.

let tg = new ThrottleGroup({ rate: 102400 });
...
someReadStream.pipe(tg.throttle()).pipe(writeStream);

I noticed that the order of the pipes is important. I hope you can improve this package. Thanks!

That's probably the issue since I applied external pipe after, I probably need to change the order,

could you try to modify that part of this library locally in node_modules/node-downloader-helper for me to verify?

from

        response.pipe(this.__fileStream);
        response.on('data', chunk => this.__calculateStats(chunk.length));

        // Add externals pipe
        this.__pipes.forEach(pipe => response.pipe(pipe.stream, pipe.options));

to

        // Add externals pipe
        this.__pipes.forEach(pipe => response.pipe(pipe.stream, pipe.options));

        response.pipe(this.__fileStream);
        response.on('data', chunk => this.__calculateStats(chunk.length));

https://github.com/hgouveia/node-downloader-helper/blob/master/src/index.js#L276

ZhangZheng-GIS commented 4 years ago

@ZhangZheng-GIS in theory you could by using the dl.pipe() function with some external module like stream-throttle or node-throttle but not sure about this, could you try it? I would be very interesting to see the result of this

There is a problem with the dl.pipe() function when I use the stream-throttle module, downloading files will not proceed.

...
let tg = new ThrottleGroup({ rate: 102400 });
dl.pipe(tg.throttle());
dl.start();

When I use native nodejs to download, it works.

let tg = new ThrottleGroup({ rate: 102400 });
...
someReadStream.pipe(tg.throttle()).pipe(writeStream);

I noticed that the order of the pipes is important. I hope you can improve this package. Thanks!

That's probably the issue since I applied external pipe after, I probably need to change the order,

could you try to modify that part of this library locally in node_modules/node-downloader-helper for me to verify?

from

        response.pipe(this.__fileStream);
        response.on('data', chunk => this.__calculateStats(chunk.length));

        // Add externals pipe
        this.__pipes.forEach(pipe => response.pipe(pipe.stream, pipe.options));

to

        // Add externals pipe
        this.__pipes.forEach(pipe => response.pipe(pipe.stream, pipe.options));

        response.pipe(this.__fileStream);
        response.on('data', chunk => this.__calculateStats(chunk.length));

https://github.com/hgouveia/node-downloader-helper/blob/master/src/index.js#L276

I tried to modify the pipes order according to the method you provided, but it didn't work and the download will stop not going. But using nodejs using pipe in reverse order download will download at full speed. I think it should be a problem elsewhere, maybe a problem with the use of the dl.pipe().

hgouveia commented 4 years ago

@ZhangZheng-GIS yes so probably is bug in the .pipe function , I will check it when I have the time and let you know

Thanks!

ZhangZheng-GIS commented 4 years ago

@hgouveia Looking forward to your reply!

hgouveia commented 4 years ago

@ZhangZheng-GIS I fixed this on the dev branch, please verify it

using npm install --save hgouveia/node-downloader-helper#dev

also, use Throttle instead of ThrottleGroup

ex:

const { Throttle } = require('stream-throttle');
dl.pipe( new Throttle({ rate: 102400 }) );

if everything ok, let me know to release the package

ZhangZheng-GIS commented 4 years ago

@ZhangZheng-GIS I fixed this on the dev branch, please verify it

using npm install --save hgouveia/node-downloader-helper#dev

also, use Throttle instead of ThrottleGroup

ex:

const { Throttle } = require('stream-throttle');
dl.pipe( new Throttle({ rate: 102400 }) );

if everything ok, let me know to release the package

Great, it works, it can limit download speed.

But I found that using dl.resume() would fail (state:FAILED) with an error and it would work when dl.pipe() was not used.

(node:94580) UnhandledPromiseRejectionWarning: Error [ERR_STREAM_WRITE_AFTER_END]: write after end
      at writeAfterEnd (_stream_writable.js:248:14)
      at Throttle.Writable.write (_stream_writable.js:296:5)
      at IncomingMessage.ondata (_stream_readable.js:706:22)
      at IncomingMessage.emit (events.js:194:13)
      at IncomingMessage.Readable.read (_stream_readable.js:497:10)
      at flow (_stream_readable.js:970:34)
      at resume_ (_stream_readable.js:951:3)
      at processTicksAndRejections (internal/process/task_queues.js:81:17)
  (node:94580) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
  (node:94580) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I need the resume download feature, do you know how to solve this problem?

hgouveia commented 4 years ago

@ZhangZheng-GIS I fixed this on the dev branch, please verify it using npm install --save hgouveia/node-downloader-helper#dev also, use Throttle instead of ThrottleGroup ex:

const { Throttle } = require('stream-throttle');
dl.pipe( new Throttle({ rate: 102400 }) );

if everything ok, let me know to release the package

Great, it works, it can limit download speed.

But I found that using dl.resume() would fail (state:FAILED) with an error and it would work when dl.pipe() was not used.

(node:94580) UnhandledPromiseRejectionWarning: Error [ERR_STREAM_WRITE_AFTER_END]: write after end
      at writeAfterEnd (_stream_writable.js:248:14)
      at Throttle.Writable.write (_stream_writable.js:296:5)
      at IncomingMessage.ondata (_stream_readable.js:706:22)
      at IncomingMessage.emit (events.js:194:13)
      at IncomingMessage.Readable.read (_stream_readable.js:497:10)
      at flow (_stream_readable.js:970:34)
      at resume_ (_stream_readable.js:951:3)
      at processTicksAndRejections (internal/process/task_queues.js:81:17)
  (node:94580) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
  (node:94580) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I need the resume download feature, do you know how to solve this problem?

hello @ZhangZheng-GIS

I couldn't find a suitable solution yet😩 , because of the way I implemented resume, it doesn't play well with pipes, i trying to find a solution , but for the meantime, since Throttle doesn't need the previous data , you could do this workaround

move the dl.pipe inside the download even instead, like this

dl
    .on('download', () => {
        dl.__pipes = [];
        dl.pipe(new Throttle({ rate: 102400 }));
    });
ZhangZheng-GIS commented 4 years ago

move the dl.pipe inside the download even instead, like this

dl
    .on('download', () => {
        dl.__pipes = [];
        dl.pipe(new Throttle({ rate: 102400 }));
    });

Hello @hgouveia

I use the above method to resume the download, it will download from the beginning, but I want to append downloading the downloaded file.

I tried the following way to resume the download, but the range of the append download file is wrong, the file size of the completed file is larger than the actual size.

const stats = fs.statSync(fileLocalPath);
const fileSizeInBytes = stats.size;
const dl = new DownloaderHelper(fileInfo.url, fileInfo.folder, { headers: { Range: 'bytes=' + fileSizeInBytes + '-' }, fileName: fileInfo.name });
dl.__filePath = fileLocalPath;
dl.__isResumable = true;
dl.__total = fileInfo.total;
dl.on('download', () => {
       dl.__pipes = [];
       dl.pipe(new Throttle({ rate: 102400 }));
});
dl.resume();

Would you please help me with this problem? Thx

ZhangZheng-GIS commented 4 years ago

Hello @hgouveia The above problem is because I used the wrong download link.😂 I can limit the download speed and resume downloading using the method you provided. Please release the package. Thanks again!

hgouveia commented 4 years ago

Ey @ZhangZheng-GIS I just updated the lib with a more proper way, so now you shouldn't need to re-pipe in the dl.on('download' event, now you can do it as fallow

dl.pipe(new Throttle({ rate: 102400 }));
dl.start();
ZhangZheng-GIS commented 4 years ago

Hello @hgouveia I verified it and had two questions.

  1. I use stream-throttle to limit the speed. In the new version, the speed limit is the speed limit for each downloading data, while the previous version is the overall speed limit. Can it be set?

  2. Why did you delete line 125 of the code in the new version so that when I reopen the application it cannot resume downloading the unfinished file? src/index.js L125 this.__downloaded = this.__getFilesizeInBytes(this.__filePath); I made the following modifications to achieve the function.

    const dl = new DownloaderHelper(fileInfo.url, fileInfo.path, { fileName: fileInfo.name})
        dl.__filePath = fullpath;
        dl.__isResumable = true;
        dl.__total = fileInfo.total;
        const stats = fs.statSync(dl.__filePath);
        const fileSizeInBytes = stats.size;
        dl.__downloaded = fileSizeInBytes
        if (downloadLimit) {
            dl.pipe(new Throttle({ rate: 1024 * downloadSpeed }));
        }
    ...
       dl.resume();
hgouveia commented 4 years ago

Hello @ZhangZheng-GIS

  1. This is really interesting, this should be working as the previous one, but you could try to re-use the pipe to all downloads? something like
//... all previous setup

const _speedLimiter = new Throttle({ rate: 1024 * downloadSpeed });
dl_1.pipe(_speedLimiter);
dl_2.pipe(_speedLimiter);
dl_3.pipe(_speedLimiter);

dl_1.resume();
dl_2.resume();
dl_3.resume();
  1. I needed to remove that line L125 this.__downloaded = this.__getFilesizeInBytes(this.__filePath);, because if you are using, for example, a compressing pipe like gzip, and you read the bytes in the files, when you sent this information to the server, you are sending the incorrect size, since you are sending the compress bytes instead the real bytes you downloaded, and you did right, in your case, you will need to populate that before like you did dl.__downloaded = fileSizeInBytes , in the future i will try to add a new function to take care of this for you, like dl.resumeFromFile( filePath, fileSize );

We can continue the conversation in here: https://gitter.im/node-downloader-helper/Lobby

hgouveia commented 4 years ago

Fixed pipe functionality on v1.0.12