node-downloader-helper: dl.pause() and dl.stop() not working properly on Node 16
When using the package with Node 16, calling dl.pause() or dl.stop() will result in the http response emitting an error event, which in turn causes dl to emit an error event as well. With dl.stop() this isn’t a big deal (although still unexpected), but with dl.pause() this results in a retry, which means that after waiting for retry.delay, the download will be resumed instead of staying paused.
After “some” investigating, I’ve found out the following:
- the original error is emitted by the http response
- the reason is emitted seems to be that the http request is being canceled, which leads to the response socket to be destroyed
- the error can be avoided by canceling the response before canceling the request, e.g. by calling
response.destroy() request.abort(), which is currently being used is deprecated and should be replaced withrequest.destroy()(which does the same)
Here’s some code for recreating the issue:
Error when calling `dl.pause()` or `dl.stop()`
const { DownloaderHelper } = require('./dist')
const dl = new DownloaderHelper('http://samples.mplayerhq.hu/4khdr/LaLaLand_cafe_4K.mkv', __dirname, {
fileName: 'test',
});
dl.start().catch(err => {
console.error(`Download failed:`, err)
})
dl.on(`error`, error => {
console.warn(`error:`, error)
})
dl.on(`download`, () => {
setTimeout(async () => {
let success = await dl.pause()
// let success = await dl.stop() // uncomment me
if (success) {
console.log(`PAUSED`)
} else {
console.log(`FAILED`)
}
}, 2500)
})
dl.on(`retry`, (attempt, retryOpts) => {
console.log(`RETRYING`)
console.log(`attempt:`, attempt)
console.log(`retryOpts:`, retryOpts)
})
dl.on(`progress.throttled`, progress => {
console.log(`progress:`, progress)
})
process.stdin.on(`data`, data => {
console.log(`data:`, data)
})
Error (and fix) for the `http` module
const http = require(`http`);
const fs = require(`fs`);
const url = "http://samples.mplayerhq.hu/4khdr/LaLaLand_cafe_4K.mkv";
let urlObj = new URL(url)
const requestOptions = {
protocol: urlObj.protocol,
host: urlObj.hostname,
port: urlObj.port,
path: urlObj.pathname,
method: `GET`,
};
let response
const request = http.request(requestOptions, res => {
response = res;
console.log(`response received`)
console.log(`statusCode: ${res.statusCode}`)
if (res.statusCode == 200) {
res.on(`data`, () => process.stdout.write(`.`))
res.on(`error`, err => console.error(`RESPONSE ERROR:`, err))
const fileStream = fs.createWriteStream(`test.file`, {});
res.pipe(fileStream)
} else {
console.error(`Bad response`);
}
});
request.on('error', error => {
console.error(`request error:`, error)
})
request.end()
setTimeout(() => {
// cancel response *before* canceling request
// response.destroy(); //!!! uncomment to fix error
request.destroy() // or `request.abort()`
}, 2500)
It should be a fairly easy fix, but I’d prefer not to contribute it myself because I’m not super-familiar with how the events and request/reponse streams should be handled.
If you really don’t have the time to fix it yourself, it would be very appreciated if you could give me a rough outline of how and where I should change things 😃
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 16 (13 by maintainers)
Commits related to this issue
- Fixed raise error event when paused in newer nodejs versions #62 — committed to hgouveia/node-downloader-helper by hgouveia 3 years ago
- Squashed commit of the following: commit 33aab440ce6d5bf996ffec07fbadad61862811c6 Author: Jose De Gouveia <dhgouveia@gmail.com> Date: Mon Oct 4 11:12:26 2021 +0200 Incremented 1.0.19 commit a... — committed to hgouveia/node-downloader-helper by hgouveia 3 years ago
hello @Chaphasilor , sorry the delay i will try to publishing today or tomorrow
@aleksey-hoffman cool thank! i will close it for now, if this happens in future version of node, please open it again i will try to find out the issue!
Hello @aleksey-hoffman , thanks for the report i will check this out as soon I can
@Chaphasilor published to npm v1.0.19
@Chaphasilor cool thanks, it seems it behaves differently in linux, i will check again on monday
hello @Chaphasilor i just did a fix not related, but it seems also fixed this, could you check? i’ve checked pause/resume with node 16 and didnt emitted the error, i will do the fixes you suggested anyways, but i am curious