From 753ca2b8fb8b75f33912a54594990007196be96a Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Sat, 14 Apr 2018 14:41:07 -0700 Subject: [PATCH] http: response should always emit close on finish Fixes: https://github.com/nodejs/node/issues/17352 --- lib/_http_client.js | 20 ++++--- test/parallel/test-http-response-close.js | 71 +++++++++++++++-------- 2 files changed, 58 insertions(+), 33 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index 5be1632fe59433..f39caeb1c23d85 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -328,15 +328,19 @@ 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) { - // Socket closed before we emitted 'end' below. - if (!req.res.complete) req.res.emit('aborted'); - var res = req.res; - res.on('end', function() { + var res = req.res; + if (res) { + if (res.readable) { + // Socket closed before we emitted 'end' below. + res.emit('aborted'); + res.on('end', function() { + this.emit('close'); + }); + res.push(null); + } else { res.emit('close'); - }); - res.push(null); - } else if (!req.res && !req.socket._hadError) { + } + } else if (!req.socket._hadError) { // This socket error fired before we started to // receive a response. The error needs to // fire on the request. 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(); + }) + ); + }) + ); +}