From 452c4b8aa10ad18fef69ce34dac5c4b61eb5364c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 25 Nov 2017 23:58:05 +0900 Subject: [PATCH 1/2] test: add common.dns.errorLookupMock --- test/common/README.md | 21 ++++++++++++++++++++- test/common/dns.js | 26 +++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index 32a14382ff5c51..54604568681612 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -419,7 +419,26 @@ called before the callback is invoked. ## DNS Module -The `DNS` module provides a naïve DNS parser/serializer. +The `DNS` module provides utilities related to the `dns` built-in module. + +### errorLookupMock(code, syscall) + +* `code` [<String>] Defaults to `dns.mockedErrorCode`. +* `syscall` [<String>] Defaults to `dns.mockedSysCall`. +* return [<Function>] + + +A mock for the `lookup` option of `net.connect()` that would result in an error +with the `code` and the `syscall` specified. Returns a function that has the +same signature as `dns.lookup()`. + +### mockedErrorCode + +The default `code` of errors generated by `errorLookupMock`. + +### mockedSysCall + +The default `syscall` of errors generated by `errorLookupMock`. ### readDomainFromPacket(buffer, offset) diff --git a/test/common/dns.js b/test/common/dns.js index 432d1b764dde29..69c67ac541cf98 100644 --- a/test/common/dns.js +++ b/test/common/dns.js @@ -1,8 +1,6 @@ /* eslint-disable required-modules */ 'use strict'; -// Naïve DNS parser/serializer. - const assert = require('assert'); const os = require('os'); @@ -22,6 +20,8 @@ const classes = { IN: 1 }; +// Naïve DNS parser/serializer. + function readDomainFromPacket(buffer, offset) { assert.ok(offset < buffer.length); const length = buffer[offset]; @@ -287,6 +287,26 @@ function writeDNSPacket(parsed) { })); } +const mockedErrorCode = 'ENOTFOUND'; +const mockedSysCall = 'getaddrinfo'; + +function errorLookupMock(code = mockedErrorCode, syscall = mockedSysCall) { + return function lookupWithError(host, dnsopts, cb) { + const err = new Error(`${syscall} ${code} ${host}`); + err.code = code; + err.errno = code; + err.syscall = syscall; + err.hostname = host; + cb(err); + }; +} + module.exports = { - types, classes, writeDNSPacket, parseDNSPacket + types, + classes, + writeDNSPacket, + parseDNSPacket, + errorLookupMock, + mockedErrorCode, + mockedSysCall }; From a5403ecc999da519bf2d65c35c7809356efcc89f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 25 Nov 2017 23:58:24 +0900 Subject: [PATCH 2/2] test: mock the lookup function in parallel tests These tests should not make any DNS calls. The lookup would fail when the DNS requests are hijacked and time out instead of erroring out. --- ...-http-client-req-error-dont-double-fire.js | 16 ++++++++-- ...net-better-error-messages-port-hostname.js | 27 +++++++++++------ .../test-net-connect-immediate-finish.js | 29 ++++++++++++------- 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/test/parallel/test-http-client-req-error-dont-double-fire.js b/test/parallel/test-http-client-req-error-dont-double-fire.js index 996540fc96c72e..d2c526eab69c6c 100644 --- a/test/parallel/test-http-client-req-error-dont-double-fire.js +++ b/test/parallel/test-http-client-req-error-dont-double-fire.js @@ -1,11 +1,21 @@ 'use strict'; + +// This tests that the error emitted on the socket does +// not get fired again when the 'error' event handler throws +// an error. + const assert = require('assert'); const http = require('http'); const common = require('../common'); +const { addresses } = require('../common/internet'); +const { errorLookupMock } = require('../common/dns'); + +const host = addresses.INVALID_HOST; -// Invalid hostname as per https://tools.ietf.org/html/rfc2606#section-2 -const host = 'this.hostname.is.invalid'; -const req = http.get({ host }); +const req = http.get({ + host, + lookup: common.mustCall(errorLookupMock()) +}); const err = new Error('mock unexpected code error'); req.on('error', common.mustCall(() => { throw err; diff --git a/test/parallel/test-net-better-error-messages-port-hostname.js b/test/parallel/test-net-better-error-messages-port-hostname.js index 818ea4bfff41f6..1a8aa770b44a22 100644 --- a/test/parallel/test-net-better-error-messages-port-hostname.js +++ b/test/parallel/test-net-better-error-messages-port-hostname.js @@ -1,21 +1,30 @@ 'use strict'; + +// This tests that the error thrown from net.createConnection +// comes with host and port properties. +// See https://github.com/nodejs/node-v0.x-archive/issues/7005 + const common = require('../common'); const net = require('net'); const assert = require('assert'); +const { addresses } = require('../common/internet'); +const { + errorLookupMock, + mockedErrorCode +} = require('../common/dns'); + // Using port 0 as hostname used is already invalid. -const c = net.createConnection(0, 'this.hostname.is.invalid'); +const c = net.createConnection({ + port: 0, + host: addresses.INVALID_HOST, + lookup: common.mustCall(errorLookupMock()) +}); c.on('connect', common.mustNotCall()); c.on('error', common.mustCall(function(e) { - // If Name Service Switch is available on the operating system then it - // might be configured differently (/etc/nsswitch.conf). - // If the system is configured with no dns the error code will be EAI_AGAIN, - // but if there are more services after the dns entry, for example some - // linux distributions ship a myhostname service by default which would - // still produce the ENOTFOUND error. - assert.ok(e.code === 'ENOTFOUND' || e.code === 'EAI_AGAIN'); + assert.strictEqual(e.code, mockedErrorCode); assert.strictEqual(e.port, 0); - assert.strictEqual(e.hostname, 'this.hostname.is.invalid'); + assert.strictEqual(e.hostname, addresses.INVALID_HOST); })); diff --git a/test/parallel/test-net-connect-immediate-finish.js b/test/parallel/test-net-connect-immediate-finish.js index e2e5e1c6715b9a..9adf8c31128b00 100644 --- a/test/parallel/test-net-connect-immediate-finish.js +++ b/test/parallel/test-net-connect-immediate-finish.js @@ -20,28 +20,35 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; + +// This tests that if the socket is still in the 'connecting' state +// when the user calls socket.end() ('finish'), the socket would emit +// 'connect' and defer the handling until the 'connect' event is handled. + const common = require('../common'); const assert = require('assert'); const net = require('net'); +const { addresses } = require('../common/internet'); +const { + errorLookupMock, + mockedErrorCode, + mockedSysCall +} = require('../common/dns'); + const client = net.connect({ - host: 'this.hostname.is.invalid', - port: common.PORT + host: addresses.INVALID_HOST, + port: common.PORT, + lookup: common.mustCall(errorLookupMock()) }); client.once('error', common.mustCall((err) => { assert(err); assert.strictEqual(err.code, err.errno); - // If Name Service Switch is available on the operating system then it - // might be configured differently (/etc/nsswitch.conf). - // If the system is configured with no dns the error code will be EAI_AGAIN, - // but if there are more services after the dns entry, for example some - // linux distributions ship a myhostname service by default which would - // still produce the ENOTFOUND error. - assert.ok(err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN'); + assert.strictEqual(err.code, mockedErrorCode); assert.strictEqual(err.host, err.hostname); - assert.strictEqual(err.host, 'this.hostname.is.invalid'); - assert.strictEqual(err.syscall, 'getaddrinfo'); + assert.strictEqual(err.host, addresses.INVALID_HOST); + assert.strictEqual(err.syscall, mockedSysCall); })); client.end();