1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

http: fix client response close & aborted

Browse Source 
Fixes: https://github.com/nodejs/node/issues/20102
Fixes: https://github.com/nodejs/node/issues/20101
Fixes: https://github.com/nodejs/node/issues/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: https://github.com/nodejs/node/pull/20075
Fixes: https://github.com/nodejs/node/issues/17352
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This commit is contained in:
Robert Nagy 2018-04-23 14:42:19 -02:00 committed by Trivikram Kamat
parent fcf2e4207e
commit ffb503be5f
2 changed files with 69 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
lib/_http_client.js
View File

@ -340,19 +340,25 @@ function socketCloseListener() {
// NOTE: It's important to get parser here, because it could be freed by // NOTE: It's important to get parser here, because it could be freed by
// the `socketOnData`. // the `socketOnData`.
var parser = socket.parser; const parser = socket.parser;
if (req.res && req.res.readable) { const res = req.res;
if (res) {
// Socket closed before we emitted 'end' below. // Socket closed before we emitted 'end' below.
if (!req.res.complete) { if (!res.complete) {
req.res.aborted = true; res.aborted = true;
req.res.emit('aborted'); res.emit('aborted');
} }
var res = req.res; req.emit('close');
if (res.readable) {
res.on('end', function() { res.on('end', function() {
res.emit('close'); this.emit('close');
}); });
res.push(null); res.push(null);
} else if (!req.res && !req.socket._hadError) { } else {
res.emit('close');
}
} else {
if (!req.socket._hadError) {
// This socket error fired before we started to // This socket error fired before we started to
// receive a response. The error needs to // receive a response. The error needs to
// fire on the request. // fire on the request.
@ -360,6 +366,7 @@ function socketCloseListener() {
req.emit('error', createHangUpError()); req.emit('error', createHangUpError());
} }
req.emit('close'); req.emit('close');
}
// Too bad. That output wasn't getting written. // Too bad. That output wasn't getting written.
// This is pretty terrible that it doesn't raise an error. // This is pretty terrible that it doesn't raise an error.

65
test/parallel/test-http-response-close.js
View File

@ -23,29 +23,50 @@
const common = require('../common'); const common = require('../common');
const http = require('http'); const http = require('http');
const server = http.createServer(common.mustCall(function(req, res) { {
const server = http.createServer(
common.mustCall((req, res) => {
res.writeHead(200); res.writeHead(200);
res.write('a'); res.write('a');
})
req.on('close', common.mustCall(function() { );
console.error('request aborted'); server.listen(
})); 0,
res.on('close', common.mustCall(function() { common.mustCall(() => {
console.error('response aborted'); http.get(
})); { port: server.address().port },
})); common.mustCall((res) => {
server.listen(0); res.on('data', common.mustCall(() => {
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(); res.destroy();
}));
res.on('close', common.mustCall(() => {
server.close(); 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();
})
);
})
);
}