From 6c2c323dc9269c64b42b1a0df8fabbcb8bc2ecff Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 23 Apr 2018 14:42:19 -0200 Subject: [PATCH] http: fix client response close & aborted 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. --- lib/_http_client.js | 39 ++++++++----- test/parallel/test-http-response-close.js | 71 +++++++++++++++-------- 2 files changed, 69 insertions(+), 41 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index 04880182dace91..3e9c6e994cd01b 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -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. diff --git a/test/parallel/test-http-response-close.js b/test/parallel/test-http-response-close.js index c58a5884d59d1a..4c36003357980b 100644 --- a/test/parallel/test-http-response-close.js +++ b/test/parallel/test-http-response-close.js @@ -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(); + }) + ); + }) + ); +}