Description
Consider the following example:
const server = http.createServer((req, res) => {
setInterval(() => {
res.write(Buffer.alloc(64))
}, 1000)
setTimeout(() => res.destroy(), 10000)
});
server.listen(0, '127.0.0.1', async () => {
let dst = fs.createWriteStream('/tmp/test')
try {
await new Promise((resolve, reject) => http
.get({
host: '127.0.0.1',
port: this.address().port
}, res => {
res
.on('error', reject)
.pipe(dst)
.on('error', reject)
.on('finish', resolve)
})
.on('error', reject)
)
} finally {
dst.destroy()
}
})Unintuitively, this will just hang and never finish (post #27984) or unexpectedly not fail and create an incomplete file (pre #27984). This is because we are not subscribing to res.'aborted'. If we have not yet received response we will get req.'error' with ECONNRESET and if we have received a response we only get an res.'aborted' and no 'error'.
In the above example (which is how I believe most people assume it should be) we have a problem. This has been a common source of error both for myself and other developers I have worked with (although in 99% of the time it's not a problem or not noticed).
Would it make sense to deprecate aborted and make the response object behave the same way as the request object, i.e. emit an error on premature close? Which I believe would be more consistent with the streams API.
Alternatively, maybe a warning/note in the documentation or some other means of avoiding people making this mistake?
Currently the proper way to implement this is either:
let dst = fs.createWriteStream('/tmp/test')
try {
await new Promise((resolve, reject) => http
.get({
host: '127.0.0.1',
port: this.address().port
}, res => {
res
.on('error', reject)
.on('aborted', () => reject(new Error('premature close')))
.pipe(dst)
.on('error', reject)
.on('finish', resolve)
})
.on('error', reject)
)
} finally {
dst.destroy()
} Or post #27984:
await new Promise((resolve, reject) => http
.get({
host: '127.0.0.1',
port: this.address().port
}, res => {
stream.pipeline(
res,
fs.createWriteStream('/tmp/test'),
err => err ? reject(err) : resolve()
)
})
.on('error', reject)
)