Skip to content

Commit

Permalink
http: fix client response close & aborted
Browse files Browse the repository at this point in the history
Fixes: #20102
Fixes: #20101
Fixes: #1735

- Response should always emit close.
- Response should always emit aborted if aborted.
- Response should always emit close after request has emitted close.

PR-URL: #20075
Fixes: #17352
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
  • Loading branch information
ronag authored and addaleax committed May 14, 2018
1 parent 30aceed commit 4c6bfbd
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 41 deletions.
39 changes: 23 additions & 16 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,26 +328,33 @@ function socketCloseListener() {

// NOTE: It's important to get parser here, because it could be freed by
// the `socketOnData`.
var parser = socket.parser;
if (req.res && req.res.readable) {
const parser = socket.parser;
const res = req.res;
if (res) {
// Socket closed before we emitted 'end' below.
if (!req.res.complete) {
req.res.aborted = true;
req.res.emit('aborted');
if (!res.complete) {
res.aborted = true;
res.emit('aborted');
}
var res = req.res;
res.on('end', function() {
req.emit('close');
if (res.readable) {
res.on('end', function() {
this.emit('close');
});
res.push(null);
} else {
res.emit('close');
});
res.push(null);
} else if (!req.res && !req.socket._hadError) {
// This socket error fired before we started to
// receive a response. The error needs to
// fire on the request.
req.socket._hadError = true;
req.emit('error', createHangUpError());
}
} else {
if (!req.socket._hadError) {
// This socket error fired before we started to
// receive a response. The error needs to
// fire on the request.
req.socket._hadError = true;
req.emit('error', createHangUpError());
}
req.emit('close');
}
req.emit('close');

// Too bad. That output wasn't getting written.
// This is pretty terrible that it doesn't raise an error.
Expand Down
71 changes: 46 additions & 25 deletions test/parallel/test-http-response-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,50 @@
const common = require('../common');
const http = require('http');

const server = http.createServer(common.mustCall(function(req, res) {
res.writeHead(200);
res.write('a');
{
const server = http.createServer(
common.mustCall((req, res) => {
res.writeHead(200);
res.write('a');
})
);
server.listen(
0,
common.mustCall(() => {
http.get(
{ port: server.address().port },
common.mustCall((res) => {
res.on('data', common.mustCall(() => {
res.destroy();
}));
res.on('close', common.mustCall(() => {
server.close();
}));
})
);
})
);
}

req.on('close', common.mustCall(function() {
console.error('request aborted');
}));
res.on('close', common.mustCall(function() {
console.error('response aborted');
}));
}));
server.listen(0);

server.on('listening', function() {
console.error('make req');
http.get({
port: this.address().port
}, function(res) {
console.error('got res');
res.on('data', function(data) {
console.error('destroy res');
res.destroy();
server.close();
});
});
});
{
const server = http.createServer(
common.mustCall((req, res) => {
res.writeHead(200);
res.end('a');
})
);
server.listen(
0,
common.mustCall(() => {
http.get(
{ port: server.address().port },
common.mustCall((res) => {
res.on('close', common.mustCall(() => {
server.close();
}));
res.resume();
})
);
})
);
}

0 comments on commit 4c6bfbd

Please sign in to comment.