hgouveia / node-downloader-helper

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

retry bug #100

Closed mumuyu66 closed 1 year ago

mumuyu66 commented 1 year ago

Hello, I encountered a bug when using the library and need your help!

When downloading large files, when the network fluctuates, there will be problems with retrying. The reproduction method is as follows:

  1. Download a large file (in my case the file is larger than 4G)
  2. Manually disconnect the network, and then restore it inside
  3. After receiving the end event, continue to retry, or report other errors

My running code is as follows:

import { basename } from 'path'; import { parse } from 'url'; const { Throttle } = require('stream-throttle'); const { DownloaderHelper} = require('node-downloader-helper'); enum DownloaderState { idle, waiting, downloading, paused, interrupted, downloaded, checking, done, }

class DownloaderItem { private save_path: string; private url: string private filename: string private downloaded_bytes: number private total_bytes: number private state: DownloaderState private dl;

constructor(url: string, saveDir: string, filename: string = "") {
    const parsedUrl = parse(url);
    if (filename == "") {
        filename = basename(parsedUrl.pathname ?? "");
    }
    this.filename = filename
    this.save_path = `${saveDir}/${filename}`;
    this.url = url
    this.downloaded_bytes = 0
    this.total_bytes = 0
    this.state = DownloaderState.idle
    this.dl = new DownloaderHelper(url, saveDir, {
        fileName: filename,
        override: { skip: true, skipSmaller: true },
        forceResume: true,
        resumeIfFileExists: true,
        retry: { maxRetries: 10, delay: 5000 },
    });
}

public get savePath(): string {
    return this.save_path;
}

public get State(): DownloaderState {
    return this.state
}

public get FileName(): string {
    return this.filename
}

public get TotalBytes(): number {
    return this.total_bytes
}

public get DownloadedBytes(): number {
    return this.downloaded_bytes
}

public download(speedLimit:number,onProgress: (speed: number) => void, onFinish: () => void, onError: (dl:any,error: string) => void): void {
    let startTime = new Date();
    this.dl
        .on('download', downloadInfo => console.log('Download Begins: ',
            {
                name: downloadInfo.fileName,
                total: downloadInfo.totalSize
            }))
        .on('end', (downloadInfo) => {
            console.log('Download Completed: ', downloadInfo)
            onFinish();
        })
        .on('skip', skipInfo =>
            console.log('Download skipped. File already exists: ', skipInfo))
        .on('error', (err) => {
            console.error('Something happened', err)
            onError(this.dl, err.message);
        })
        .on('retry', (attempt, opts, err) => {
            console.log({
                RetryAttempt: `${attempt}/${opts.maxRetries}`,
                StartsOn: `${opts.delay / 1000} secs`,
                Reason: err ? err.message : 'unknown'
            });
        })
        .on('resume', isResumed => {
            // is resume is not supported,  
            // a new pipe instance needs to be attached
            if (!isResumed) {
                console.warn("This URL doesn't support resume, it will start from the beginning");
            }
        })
        .on('stateChanged', state => console.log('State: ', state))
        .on('renamed', filePaths => console.log('File Renamed to: ', filePaths.fileName))
        .on('redirected', (newUrl, oldUrl) => console.log(`Redirect from '${newUrl}' => '${oldUrl}'`))
        .on('progress', (stats) => {
            const progress = stats.progress.toFixed(1);
            const speed = stats.speed;
            const downloaded = stats.downloaded;
            const total = stats.total;
            this.total_bytes = total
            this.downloaded_bytes = downloaded
            this.total_bytes = total
            // print every one second (`progress.throttled` can be used instead)
            const currentTime = new Date();
            const elaspsedTime = currentTime.getTime() - startTime.getTime();
            if (elaspsedTime > 500) {
                startTime = currentTime;
                //console.log(`${speed}/s - ${progress}% [${downloaded}/${total}]`);
                onProgress(speed);
            }
        });
    if(speedLimit > 0){
        this.dl.pipe(new Throttle({ rate: speedLimit }));
    }
    console.log(`Downloading [${this.dl.getMetadata()}]: ${this.url}`);
    this.dl.start().catch(err => { onError(this.dl, err.message);/* already listening on 'error' event but catch can be used too */ });
}

public SetSpeedLimit(speedLimit: number): void {
    console.log(`Setting speed limit to: ${speedLimit}`);
    if (speedLimit > 0) {
        this.dl.unpipe();
        this.dl.pipe(new Throttle({ rate: speedLimit }));
    }else {
        this.dl.unpipe();
    }
}

public Pause() {
    if (this.dl) {
        this.state = DownloaderState.paused
        this.dl.pause();
    }
}

public Resume() {
    if (this.state == DownloaderState.paused) {
        this.dl.resume()
        this.state = DownloaderState.downloading
    }
}
public Stop() {
    this.state = DownloaderState.idle
    this.dl.pause()
    //this.dl.stop()
}

public Delete() {
    this.state = DownloaderState.idle
    this.dl.stop()
}

}

export { DownloaderItem, DownloaderState };

hgouveia commented 1 year ago

Hello @mumuyu66, could you provide the error messages you have? I don’t understand the issue if I’m being honest. Could you tell me what’s happening and what you’re expecting to do?

But based on what you said, it seems that probably the URL could be a single-use URL. If that’s the case, you would need to request a new URL and start a new download.

mumuyu66 commented 1 year ago

Hello, @hgouveia I am working on a game downloader. Some time ago, a player reported that an exception would be reported when the network is unstable. In order to reproduce this bug, I turned off the computer network during the download process, and then repaired it immediately, and then rediscovered the bug reported by the player.

On the first graph, at 83.7%, I started to shut down, and at 83.9%, the network came back.

In the second picture, after 83.9%, although the progress has been going, the background is still retrying the connection, and the retry event can always be received.

After receiving the end event, I started to unzip the file. At this time, I received an error event, as shown in Figure 3 below

1 2 3
hgouveia commented 1 year ago

Oh, okay @mumuyu66. So the issue is that it’s still attempting to retry after it has finished downloading, right?, i will look into this

mumuyu66 commented 1 year ago

Yes, the bug I want to report is this @hgouveia

mumuyu66 commented 1 year ago

@hgouveia hi, I have submitted a pull request that fixes this bug. Please review the code.

hgouveia commented 1 year ago

Thanks very much! @mumuyu66 i will review it and if all good i will try to publish the fix today