Skip to content

Commit

Permalink
http: IncomingMessage destroyed state
Browse files Browse the repository at this point in the history
IncomingMessage should implement _destroy() instead of overriding
destroy() in order to behave properly like a stream.
  • Loading branch information
ronag committed Oct 6, 2019
1 parent ddcd235 commit c74652f
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 5 deletions.
8 changes: 5 additions & 3 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,9 @@ ClientRequest.prototype.abort = function abort() {

// If we're aborting, we don't care about any more response data.
if (this.res) {
this.res._dump();
// TODO(ronag): No more data events should occur after destroy.
this.res.removeAllListeners('data');
this.res.destroy();
}

// In the event that we don't have a socket, we will pop out of
Expand Down Expand Up @@ -365,11 +367,11 @@ function socketCloseListener() {
req.emit('close');
if (res.readable) {
res.on('end', function() {
this.emit('close');
res.destroy();
});
res.push(null);
} else {
res.emit('close');
res.destroy();
}
} else {
if (!req.socket._hadError) {
Expand Down
6 changes: 4 additions & 2 deletions lib/_http_incoming.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,11 @@ IncomingMessage.prototype._read = function _read(n) {
// It's possible that the socket will be destroyed, and removed from
// any messages, before ever calling this. In that case, just skip
// it, since something else is destroying this connection anyway.
IncomingMessage.prototype.destroy = function destroy(error) {
IncomingMessage.prototype._destroy = function destroy(err, cb) {
if (this.socket)
this.socket.destroy(error);
this.socket.destroy(err, cb);
else
cb(err);
};


Expand Down
39 changes: 39 additions & 0 deletions test/parallel/test-http-request-destroy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

{
const server = http.Server(function(req, res) {
req.destroy();
assert.strictEqual(req.destroyed, true);
});

server.listen(0, function() {
http.get({
port: this.address().port,
}).on('error', common.mustCall(() => {
server.close();
}));
});
}

{
const server = http.Server(function(req, res) {
req.destroy(new Error('kaboom'));
req.destroy(new Error('kaboom2'));
assert.strictEqual(req.destroyed, true);
req.on('error', common.mustCall((err) => {
assert.strictEqual(err.message, 'kaboom');
}));
});

server.listen(0, function() {
http.get({
port: this.address().port,
}).on('error', common.mustCall(() => {
server.close();
}));
});
}

0 comments on commit c74652f

Please sign in to comment.