From ddac3bd14f14d1e830cf3b5fbd78483e1a87e8eb Mon Sep 17 00:00:00 2001 From: Daniele Belardi Date: Sun, 27 Dec 2020 18:48:54 +0100 Subject: [PATCH] http: add test case for req-res close ordering Since 55e83cbe957de2c752521557ffd74d4496fa8b81 has changed the ordering of the close event, add a test case. IncomingMessage will emit close before the response is sent in case the server is consuming data from it. Refs: https://github.com/nodejs/node/pull/33035#issuecomment-751489998 PR-URL: https://github.com/nodejs/node/pull/36645 Reviewed-By: Rich Trott Reviewed-By: Robert Nagy Reviewed-By: Matteo Collina --- test/parallel/test-http-req-res-close.js | 144 +++++++++++++++++++---- 1 file changed, 119 insertions(+), 25 deletions(-) diff --git a/test/parallel/test-http-req-res-close.js b/test/parallel/test-http-req-res-close.js index 329515ccd76ffb..8a0a9e5ab3fc96 100644 --- a/test/parallel/test-http-req-res-close.js +++ b/test/parallel/test-http-req-res-close.js @@ -4,34 +4,128 @@ const common = require('../common'); const http = require('http'); const assert = require('assert'); -const server = http.Server(common.mustCall((req, res) => { - let resClosed = false; - - res.end(); - let resFinished = false; - res.on('finish', common.mustCall(() => { - resFinished = true; - assert.strictEqual(resClosed, false); - assert.strictEqual(res.destroyed, false); - assert.strictEqual(resClosed, false); +// When the response is ended immediately, `req` should emit `close` +// after `res` +{ + const server = http.Server(common.mustCall((req, res) => { + let resClosed = false; + let reqClosed = false; + + res.end(); + let resFinished = false; + res.on('finish', common.mustCall(() => { + resFinished = true; + assert.strictEqual(resClosed, false); + assert.strictEqual(res.destroyed, false); + assert.strictEqual(resClosed, false); + })); + assert.strictEqual(req.destroyed, false); + res.on('close', common.mustCall(() => { + resClosed = true; + assert.strictEqual(resFinished, true); + assert.strictEqual(reqClosed, false); + assert.strictEqual(res.destroyed, true); + })); + assert.strictEqual(req.destroyed, false); + req.on('end', common.mustCall(() => { + assert.strictEqual(req.destroyed, false); + })); + req.on('close', common.mustCall(() => { + reqClosed = true; + assert.strictEqual(resClosed, true); + assert.strictEqual(req.destroyed, true); + assert.strictEqual(req._readableState.ended, true); + })); + res.socket.on('close', () => server.close()); })); - assert.strictEqual(req.destroyed, false); - res.on('close', common.mustCall(() => { - resClosed = true; - assert.strictEqual(resFinished, true); - assert.strictEqual(res.destroyed, true); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port }, common.mustCall()); })); - assert.strictEqual(req.destroyed, false); - req.on('end', common.mustCall(() => { +} + +// When there's no `data` handler attached to `req`, +// `req` should emit `close` after `res`. +{ + const server = http.Server(common.mustCall((req, res) => { + let resClosed = false; + let reqClosed = false; + + // This time, don't end the response immediately + setTimeout(() => res.end(), 100); + let resFinished = false; + res.on('finish', common.mustCall(() => { + resFinished = true; + assert.strictEqual(resClosed, false); + assert.strictEqual(res.destroyed, false); + assert.strictEqual(resClosed, false); + })); + assert.strictEqual(req.destroyed, false); + res.on('close', common.mustCall(() => { + resClosed = true; + assert.strictEqual(resFinished, true); + assert.strictEqual(reqClosed, false); + assert.strictEqual(res.destroyed, true); + })); assert.strictEqual(req.destroyed, false); + req.on('end', common.mustCall(() => { + assert.strictEqual(req.destroyed, false); + })); + req.on('close', common.mustCall(() => { + reqClosed = true; + assert.strictEqual(resClosed, true); + assert.strictEqual(req.destroyed, true); + assert.strictEqual(req._readableState.ended, true); + })); + res.socket.on('close', () => server.close()); })); - req.on('close', common.mustCall(() => { - assert.strictEqual(req.destroyed, true); - assert.strictEqual(req._readableState.ended, true); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port }, common.mustCall()); })); - res.socket.on('close', () => server.close()); -})); +} + +// When a `data` handler is `attached` to `req` +// (i.e. the server is consuming data from it), `req` should emit `close` +// before `res`. +// https://github.com/nodejs/node/pull/33035 introduced this change in behavior. +// See https://github.com/nodejs/node/pull/33035#issuecomment-751482764 +{ + const server = http.Server(common.mustCall((req, res) => { + let resClosed = false; + let reqClosed = false; -server.listen(0, common.mustCall(() => { - http.get({ port: server.address().port }, common.mustCall()); -})); + // Don't end the response immediately + setTimeout(() => res.end(), 100); + let resFinished = false; + req.on('data', () => {}); + res.on('finish', common.mustCall(() => { + resFinished = true; + assert.strictEqual(resClosed, false); + assert.strictEqual(res.destroyed, false); + assert.strictEqual(resClosed, false); + })); + assert.strictEqual(req.destroyed, false); + res.on('close', common.mustCall(() => { + resClosed = true; + assert.strictEqual(resFinished, true); + assert.strictEqual(reqClosed, true); + assert.strictEqual(res.destroyed, true); + })); + assert.strictEqual(req.destroyed, false); + req.on('end', common.mustCall(() => { + assert.strictEqual(req.destroyed, false); + })); + req.on('close', common.mustCall(() => { + reqClosed = true; + assert.strictEqual(resClosed, false); + assert.strictEqual(req.destroyed, true); + assert.strictEqual(req._readableState.ended, true); + })); + res.socket.on('close', () => server.close()); + })); + + server.listen(0, common.mustCall(() => { + http.get({ port: server.address().port }, common.mustCall()); + })); +}