Skip to content

Commit 6dd8500

Browse files
committed
stream: pipeline should use req.abort() to destroy response
destroy(err) on http response will propagate the error to the request causing 'error' to be unexpectedly emitted. Furthermore, response.destroy() unlike request.abort() does not _dump buffered data. Fixes a breaking change introduced in 6480882. Prefer res.req.abort() over res.destroy() until this situation is clarified. Fixes: #31029 Refs: 6480882
1 parent fc553fd commit 6dd8500

File tree

2 files changed

+38
-13
lines changed

2 files changed

+38
-13
lines changed

lib/internal/streams/pipeline.js

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const {
1717
} = require('internal/errors').codes;
1818

1919
function isRequest(stream) {
20-
return stream.setHeader && typeof stream.abort === 'function';
20+
return stream && stream.setHeader && typeof stream.abort === 'function';
2121
}
2222

2323
function destroyer(stream, reading, writing, callback) {
@@ -43,22 +43,13 @@ function destroyer(stream, reading, writing, callback) {
4343

4444
// request.destroy just do .end - .abort is what we want
4545
if (isRequest(stream)) return stream.abort();
46-
if (typeof stream.destroy === 'function') {
47-
if (stream.req && stream._writableState === undefined) {
48-
// This is a ClientRequest
49-
// TODO(mcollina): backward compatible fix to avoid crashing.
50-
// Possibly remove in a later semver-major change.
51-
stream.req.on('error', noop);
52-
}
53-
return stream.destroy(err);
54-
}
46+
if (isRequest(stream.req)) return stream.req.abort();
47+
if (typeof stream.destroy === 'function') return stream.destroy(err);
5548

5649
callback(err || new ERR_STREAM_DESTROYED('pipe'));
5750
};
5851
}
5952

60-
function noop() {}
61-
6253
function pipe(from, to) {
6354
return from.pipe(to);
6455
}

test/parallel/test-stream-pipeline.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
'use strict';
22

33
const common = require('../common');
4-
const { Stream, Writable, Readable, Transform, pipeline } = require('stream');
4+
const {
5+
Stream,
6+
Writable,
7+
Readable,
8+
Transform,
9+
pipeline,
10+
PassThrough
11+
} = require('stream');
512
const assert = require('assert');
613
const http = require('http');
714
const { promisify } = require('util');
@@ -483,3 +490,30 @@ const { promisify } = require('util');
483490
{ code: 'ERR_INVALID_CALLBACK' }
484491
);
485492
}
493+
494+
{
495+
const server = http.Server(function(req, res) {
496+
res.write('asd');
497+
});
498+
server.listen(0, function() {
499+
http.request({
500+
port: this.address().port,
501+
path: '/',
502+
method: 'GET'
503+
}, (res) => {
504+
const stream = new PassThrough();
505+
506+
stream.on('error', common.mustCall());
507+
508+
pipeline(
509+
res,
510+
stream,
511+
common.mustCall((err) => {
512+
server.close();
513+
})
514+
);
515+
516+
stream.destroy(new Error('oh no'));
517+
}).end().on('error', common.mustNotCall());
518+
});
519+
}

0 commit comments

Comments
 (0)