From ad35687888e63b886c6ddca87b42ca0fced21c1e Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Fri, 1 Sep 2017 15:20:30 -0400 Subject: [PATCH 1/2] test: increase Http2ServerResponse test coverage Modify existing header tests for Http2ServerResponse to include sendDate (get and set) and headersSent. Expand existing test for end to include a check for closed. Add a new test for destroy. Refs: https://github.com/nodejs/node/issues/14985 --- ...est-http2-compat-serverresponse-destroy.js | 91 +++++++++++++++++++ .../test-http2-compat-serverresponse-end.js | 2 + ...ttp2-compat-serverresponse-flushheaders.js | 3 + ...est-http2-compat-serverresponse-headers.js | 4 + 4 files changed, 100 insertions(+) create mode 100644 test/parallel/test-http2-compat-serverresponse-destroy.js diff --git a/test/parallel/test-http2-compat-serverresponse-destroy.js b/test/parallel/test-http2-compat-serverresponse-destroy.js new file mode 100644 index 00000000000000..5a8cc3b5103ef7 --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-destroy.js @@ -0,0 +1,91 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const http2 = require('http2'); + +// Check that destroying the Http2ServerResponse stream produces +// the expected result, including the ability to throw an error +// which is emitted on server.streamError + +const errors = [ + 'test-error', + Error('test') +]; +let nextError; + +const server = http2.createServer(common.mustCall((req, res) => { + req.once('error', common.mustNotCall()); + req.once('error', common.mustNotCall()); + + res.on('finish', common.mustCall(() => { + assert.doesNotThrow(() => res.destroy(nextError)); + assert.strictEqual(res.closed, true); + })); + + if (req.path !== '/') { + nextError = errors.shift(); + } + res.destroy(nextError); +}, 3)); + +server.on( + 'streamError', + common.mustCall((err) => assert.strictEqual(err, nextError), 2) +); + +server.listen(0, common.mustCall(() => { + const port = server.address().port; + const client = http2.connect(`http://localhost:${port}`); + const req = client.request({ + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }); + + req.on('response', common.mustNotCall()); + req.on('error', common.mustNotCall()); + req.on('end', common.mustCall()); + + req.resume(); + req.end(); + + const req2 = client.request({ + ':path': '/error', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }); + + req2.on('response', common.mustNotCall()); + req2.on('error', common.mustNotCall()); + req2.on('end', common.mustCall()); + + req2.resume(); + req2.end(); + + const req3 = client.request({ + ':path': '/error', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }); + + req3.on('response', common.mustNotCall()); + req3.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + type: Error, + message: 'Stream closed with error code 2' + })); + req3.on('end', common.mustCall(() => { + server.close(); + client.destroy(); + })); + + req3.resume(); + req3.end(); +})); diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index 5fb93e93dd5ff7..6c07f6d83e62e0 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -18,10 +18,12 @@ const { // Http2ServerResponse.end callback is called only the first time, // but may be invoked repeatedly without throwing errors. const server = createServer(mustCall((request, response) => { + strictEqual(response.closed, false); response.end(mustCall(() => { server.close(); })); response.end(mustNotCall()); + strictEqual(response.closed, true); })); server.listen(0, mustCall(() => { const { port } = server.address(); diff --git a/test/parallel/test-http2-compat-serverresponse-flushheaders.js b/test/parallel/test-http2-compat-serverresponse-flushheaders.js index 671b0c7b94e661..f02732df87b050 100644 --- a/test/parallel/test-http2-compat-serverresponse-flushheaders.js +++ b/test/parallel/test-http2-compat-serverresponse-flushheaders.js @@ -15,8 +15,11 @@ const server = h2.createServer(); server.listen(0, common.mustCall(function() { const port = server.address().port; server.once('request', common.mustCall(function(request, response) { + assert.strictEqual(response.headersSent, false); response.flushHeaders(); + assert.strictEqual(response.headersSent, true); response.flushHeaders(); // Idempotent + common.expectsError(() => { response.writeHead(400, { 'foo-bar': 'abc123' }); }, { diff --git a/test/parallel/test-http2-compat-serverresponse-headers.js b/test/parallel/test-http2-compat-serverresponse-headers.js index 512a14c1c0cf8f..290c8a515fd2db 100644 --- a/test/parallel/test-http2-compat-serverresponse-headers.js +++ b/test/parallel/test-http2-compat-serverresponse-headers.js @@ -80,6 +80,10 @@ server.listen(0, common.mustCall(function() { response.getHeaders()[fake] = fake; assert.strictEqual(response.hasHeader(fake), false); + assert.strictEqual(response.sendDate, true); + response.sendDate = false; + assert.strictEqual(response.sendDate, false); + response.on('finish', common.mustCall(function() { server.close(); })); From 12dfd8feebd72cc0f0dcdcebbe70881cbd1acbe1 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 2 Sep 2017 08:18:00 -0400 Subject: [PATCH 2/2] Fix to test both req & res error listeners --- test/parallel/test-http2-compat-serverresponse-destroy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-http2-compat-serverresponse-destroy.js b/test/parallel/test-http2-compat-serverresponse-destroy.js index 5a8cc3b5103ef7..40c73b0887c32f 100644 --- a/test/parallel/test-http2-compat-serverresponse-destroy.js +++ b/test/parallel/test-http2-compat-serverresponse-destroy.js @@ -18,8 +18,8 @@ const errors = [ let nextError; const server = http2.createServer(common.mustCall((req, res) => { - req.once('error', common.mustNotCall()); - req.once('error', common.mustNotCall()); + req.on('error', common.mustNotCall()); + res.on('error', common.mustNotCall()); res.on('finish', common.mustCall(() => { assert.doesNotThrow(() => res.destroy(nextError));