From ba0dbaa1930db93a9e988ce1a525c8b2054a2970 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Fri, 5 May 2017 06:10:43 -0500 Subject: [PATCH 01/25] Revert "net: remove unnecessary process.nextTick()" This reverts commit 571882c5a45872ac67e4e29513c4c8f23af9f781. Removing the process.nextTick() call can prevent the consumer from being able to catch error events. PR-URL: https://github.com/nodejs/node/pull/12854 Fixes: https://github.com/nodejs/node/issues/12841 Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- lib/net.js | 5 ++++- test/async-hooks/test-tcpwrap.js | 11 +++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/net.js b/lib/net.js index a3d778a4488..67e191fa213 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1005,7 +1005,10 @@ function lookupAndConnect(self, options) { // If host is an IP, skip performing a lookup var addressType = cares.isIP(host); if (addressType) { - internalConnect(self, host, port, addressType, localAddress, localPort); + nextTick(self[async_id_symbol], function() { + if (self.connecting) + internalConnect(self, host, port, addressType, localAddress, localPort); + }); return; } diff --git a/test/async-hooks/test-tcpwrap.js b/test/async-hooks/test-tcpwrap.js index cbad3f3ddf3..08277792292 100644 --- a/test/async-hooks/test-tcpwrap.js +++ b/test/async-hooks/test-tcpwrap.js @@ -48,13 +48,16 @@ const server = net { port: server.address().port, host: server.address().address }, common.mustCall(onconnected)); const tcps = hooks.activitiesOfTypes('TCPWRAP'); - const tcpconnects = hooks.activitiesOfTypes('TCPCONNECTWRAP'); assert.strictEqual( tcps.length, 2, '2 TCPWRAPs present when client is connecting'); - assert.strictEqual( - tcpconnects.length, 1, - '1 TCPCONNECTWRAP present when client is connecting'); + process.nextTick(() => { + const tcpconnects = hooks.activitiesOfTypes('TCPCONNECTWRAP'); + assert.strictEqual( + tcpconnects.length, 1, + '1 TCPCONNECTWRAP present when client is connecting'); + }); + tcp2 = tcps[1]; assert.strictEqual(tcps.length, 2, '2 TCPWRAP present when client is connecting'); From 2cf4882136b2c17c37aeb7b793a3040e64936b9f Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Thu, 11 May 2017 10:35:24 -0500 Subject: [PATCH 02/25] test: add regression test for immediate socket errors This test ensures that a http client request with the default agent that has a socket that is immediately destroyed can still be caught by adding an error event listener to the request object. PR-URL: https://github.com/nodejs/node/pull/12854 Fixes: https://github.com/nodejs/node/issues/12841 Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- test/parallel/test-http-client-immediate-error.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 test/parallel/test-http-client-immediate-error.js diff --git a/test/parallel/test-http-client-immediate-error.js b/test/parallel/test-http-client-immediate-error.js new file mode 100644 index 00000000000..9fbe052efd4 --- /dev/null +++ b/test/parallel/test-http-client-immediate-error.js @@ -0,0 +1,12 @@ +'use strict'; + +// Make sure http.request() can catch immediate errors in +// net.createConnection(). + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); +const req = http.get({ host: '127.0.0.1', port: 1 }); +req.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ECONNREFUSED'); +})); From 4f4184c10b90a4979424c72a568cf517de651396 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Fri, 12 May 2017 13:13:15 +0200 Subject: [PATCH 03/25] crypto: remove unnecessary template class I came across this template class but I don't understand why it is there. It is not used in the template specialization following it. I just wanted to bring it up just in case this is something that has been overlooked. PR-URL: https://github.com/nodejs/node/pull/12993 Reviewed-By: Fedor Indutny Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- src/node_crypto.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 5ef7d96007b..19cc30ee41f 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -155,7 +155,6 @@ std::string extra_root_certs_file; // NOLINT(runtime/string) X509_STORE* root_cert_store; // Just to generate static methods -template class SSLWrap; template void SSLWrap::AddMethods(Environment* env, Local t); template void SSLWrap::InitNPN(SecureContext* sc); From 7a1dc1fba30eb6e883235d98aac144b39b97cf1b Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Thu, 11 May 2017 12:09:50 +1000 Subject: [PATCH 04/25] build: avoid /docs/api and /docs/doc/api upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/nodejs/node/issues/12833 PR-URL: https://github.com/nodejs/node/pull/12957 Reviewed-By: João Reis Reviewed-By: Gibson Fahnestock Reviewed-By: Michael Dawson --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 94e6ff455cc..f77f1746f62 100644 --- a/Makefile +++ b/Makefile @@ -754,9 +754,9 @@ ifeq ($(XZ), 0) endif doc-upload: doc - ssh $(STAGINGSERVER) "mkdir -p nodejs/$(DISTTYPEDIR)/$(FULLVERSION)" + ssh $(STAGINGSERVER) "mkdir -p nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/docs/" chmod -R ug=rw-x+X,o=r+X out/doc/ - scp -pr out/doc/ $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/docs/ + scp -pr out/doc/* $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/docs/ ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/docs.done" $(TARBALL)-headers: release-only From 594b5d7b8970a68d9cd66f629cf1862cfca68412 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Sat, 13 May 2017 12:28:18 +0430 Subject: [PATCH 05/25] cmd: support dash as stdin alias PR-URL: https://github.com/nodejs/node/pull/13012 Reviewed-By: Jeremiah Senkpiel Reviewed-By: Refael Ackermann Reviewed-By: Gibson Fahnestock --- doc/api/cli.md | 13 ++++- doc/api/synopsis.md | 2 +- doc/node.1 | 11 ++++- lib/internal/bootstrap_node.js | 2 +- src/node.cc | 6 ++- .../test-stdin-script-child-option.js | 17 +++++++ test/parallel/test-stdin-script-child.js | 49 ++++++++++--------- 7 files changed, 71 insertions(+), 29 deletions(-) create mode 100644 test/parallel/test-stdin-script-child-option.js diff --git a/doc/api/cli.md b/doc/api/cli.md index d53052dbdc8..f6589f6264f 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -10,7 +10,7 @@ To view this documentation as a manual page in a terminal, run `man node`. ## Synopsis -`node [options] [v8 options] [script.js | -e "script"] [--] [arguments]` +`node [options] [v8 options] [script.js | -e "script" | -] [--] [arguments]` `node debug [script.js | -e "script" | :] …` @@ -345,6 +345,17 @@ added: v0.11.15 Specify ICU data load path. (overrides `NODE_ICU_DATA`) + +### `-` + + +Alias for stdin, analogous to the use of - in other command line utilities, +meaning that the script will be read from stdin, and the rest of the options +are passed to that script. + + ### `--` -`node [options] [v8 options] [script.js | -e "script"] [arguments]` +`node [options] [v8 options] [script.js | -e "script" | - ] [arguments]` Please see the [Command Line Options][] document for information about different options and ways to run scripts with Node.js. diff --git a/doc/node.1 b/doc/node.1 index 46771b2faa3..c7844b67b6b 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -36,7 +36,10 @@ node \- Server-side JavaScript runtime .RI [ v8\ options ] .RI [ script.js \ | .B -e -.RI \&" script \&"] +.RI \&" script \&" +.R | +.B - +.R ] .B [--] .RI [ arguments ] .br @@ -225,6 +228,12 @@ See \fBSSL_CERT_DIR\fR and \fBSSL_CERT_FILE\fR. .BR \-\-icu\-data\-dir =\fIfile\fR Specify ICU data load path. (overrides \fBNODE_ICU_DATA\fR) +.TP +.BR \-\fR +Alias for stdin, analogous to the use of - in other command line utilities, +meaning that the script will be read from stdin, and the rest of the options +are passed to that script. + .TP .BR \-\-\fR Indicate the end of node options. Pass the rest of the arguments to the script. diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 86875566b08..ed1edf7138b 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -123,7 +123,7 @@ const internalModule = NativeModule.require('internal/module'); internalModule.addBuiltinLibsToObject(global); evalScript('[eval]'); - } else if (process.argv[1]) { + } else if (process.argv[1] && process.argv[1] !== '-') { // make process.argv[1] into a full path const path = NativeModule.require('path'); process.argv[1] = path.resolve(process.argv[1]); diff --git a/src/node.cc b/src/node.cc index a5c86d4c363..efa090cf1f8 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3592,7 +3592,7 @@ void LoadEnvironment(Environment* env) { static void PrintHelp() { // XXX: If you add an option here, please also add it to doc/node.1 and // doc/api/cli.md - printf("Usage: node [options] [ -e script | script.js ] [arguments]\n" + printf("Usage: node [options] [ -e script | script.js | - ] [arguments]\n" " node inspect script.js [arguments]\n" "\n" "Options:\n" @@ -3604,6 +3604,8 @@ static void PrintHelp() { " does not appear to be a terminal\n" " -r, --require module to preload (option can be " "repeated)\n" + " - script read from stdin (default; " + "interactive mode if a tty)" #if HAVE_INSPECTOR " --inspect[=[host:]port] activate inspector on host:port\n" " (default: 127.0.0.1:9229)\n" @@ -3913,6 +3915,8 @@ static void ParseArgs(int* argc, } else if (strcmp(arg, "--expose-internals") == 0 || strcmp(arg, "--expose_internals") == 0) { config_expose_internals = true; + } else if (strcmp(arg, "-") == 0) { + break; } else if (strcmp(arg, "--") == 0) { index += 1; break; diff --git a/test/parallel/test-stdin-script-child-option.js b/test/parallel/test-stdin-script-child-option.js new file mode 100644 index 00000000000..5526d66f3f2 --- /dev/null +++ b/test/parallel/test-stdin-script-child-option.js @@ -0,0 +1,17 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const expected = '--option-to-be-seen-on-child'; + +const { spawn } = require('child_process'); +const child = spawn(process.execPath, ['-', expected], { stdio: 'pipe' }); + +child.stdin.end('console.log(process.argv[2])'); + +let actual = ''; +child.stdout.setEncoding('utf8'); +child.stdout.on('data', (chunk) => actual += chunk); +child.stdout.on('end', common.mustCall(() => { + assert.strictEqual(actual.trim(), expected); +})); diff --git a/test/parallel/test-stdin-script-child.js b/test/parallel/test-stdin-script-child.js index 81eb3ea5b4a..d3d22a9a82f 100644 --- a/test/parallel/test-stdin-script-child.js +++ b/test/parallel/test-stdin-script-child.js @@ -2,31 +2,32 @@ const common = require('../common'); const assert = require('assert'); -const spawn = require('child_process').spawn; -const child = spawn(process.execPath, [], { - env: Object.assign(process.env, { - NODE_DEBUG: process.argv[2] - }) -}); -const wanted = `${child.pid}\n`; -let found = ''; +const { spawn } = require('child_process'); +for (const args of [[], ['-']]) { + const child = spawn(process.execPath, args, { + env: Object.assign(process.env, { + NODE_DEBUG: process.argv[2] + }) + }); + const wanted = `${child.pid}\n`; + let found = ''; -child.stdout.setEncoding('utf8'); -child.stdout.on('data', function(c) { - found += c; -}); + child.stdout.setEncoding('utf8'); + child.stdout.on('data', function(c) { + found += c; + }); -child.stderr.setEncoding('utf8'); -child.stderr.on('data', function(c) { - console.error(`> ${c.trim().split(/\n/).join('\n> ')}`); -}); + child.stderr.setEncoding('utf8'); + child.stderr.on('data', function(c) { + console.error(`> ${c.trim().split(/\n/).join('\n> ')}`); + }); -child.on('close', common.mustCall(function(c) { - assert.strictEqual(c, 0); - assert.strictEqual(found, wanted); - console.log('ok'); -})); + child.on('close', common.mustCall(function(c) { + assert.strictEqual(c, 0); + assert.strictEqual(found, wanted); + })); -setTimeout(function() { - child.stdin.end('console.log(process.pid)'); -}, 1); + setTimeout(function() { + child.stdin.end('console.log(process.pid)'); + }, 1); +} From 359ea2a0bd772beb25267478d4f8e3ed59021e19 Mon Sep 17 00:00:00 2001 From: Brian White Date: Fri, 19 May 2017 04:07:53 -0400 Subject: [PATCH 06/25] stream: improve readable push performance PR-URL: https://github.com/nodejs/node/pull/13113 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- benchmark/streams/readable-boundaryread.js | 12 +- lib/_stream_readable.js | 141 +++++++++++---------- 2 files changed, 81 insertions(+), 72 deletions(-) diff --git a/benchmark/streams/readable-boundaryread.js b/benchmark/streams/readable-boundaryread.js index 1a0b7eb7ac9..4834da0a2c5 100644 --- a/benchmark/streams/readable-boundaryread.js +++ b/benchmark/streams/readable-boundaryread.js @@ -4,20 +4,22 @@ const common = require('../common'); const Readable = require('stream').Readable; const bench = common.createBenchmark(main, { - n: [200e1] + n: [200e1], + type: ['string', 'buffer'] }); function main(conf) { const n = +conf.n; - const b = new Buffer(32); const s = new Readable(); - function noop() {} - s._read = noop; + var data = 'a'.repeat(32); + if (conf.type === 'buffer') + data = Buffer.from(data); + s._read = function() {}; bench.start(); for (var k = 0; k < n; ++k) { for (var i = 0; i < 1e4; ++i) - s.push(b); + s.push(data); while (s.read(32)); } bench.end(n); diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 8b0d45cc86c..9666c7d6fb2 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -177,81 +177,97 @@ Readable.prototype._destroy = function(err, cb) { // write() some more. Readable.prototype.push = function(chunk, encoding) { var state = this._readableState; - - if (!state.objectMode && typeof chunk === 'string') { - encoding = encoding || state.defaultEncoding; - if (encoding !== state.encoding) { - chunk = Buffer.from(chunk, encoding); - encoding = ''; + var skipChunkCheck; + + if (!state.objectMode) { + if (typeof chunk === 'string') { + encoding = encoding || state.defaultEncoding; + if (encoding !== state.encoding) { + chunk = Buffer.from(chunk, encoding); + encoding = ''; + } + skipChunkCheck = true; } + } else { + skipChunkCheck = true; } - return readableAddChunk(this, state, chunk, encoding, false); + return readableAddChunk(this, chunk, encoding, false, skipChunkCheck); }; // Unshift should *always* be something directly out of read() Readable.prototype.unshift = function(chunk) { - var state = this._readableState; - return readableAddChunk(this, state, chunk, '', true); -}; - -Readable.prototype.isPaused = function() { - return this._readableState.flowing === false; + return readableAddChunk(this, chunk, null, true, false); }; -function readableAddChunk(stream, state, chunk, encoding, addToFront) { - var er = chunkInvalid(state, chunk); - if (er) { - stream.emit('error', er); - } else if (chunk === null) { +function readableAddChunk(stream, chunk, encoding, addToFront, skipChunkCheck) { + var state = stream._readableState; + if (chunk === null) { state.reading = false; onEofChunk(stream, state); - } else if (state.objectMode || chunk && chunk.length > 0) { - if (state.ended && !addToFront) { - const e = new Error('stream.push() after EOF'); - stream.emit('error', e); - } else if (state.endEmitted && addToFront) { - const e = new Error('stream.unshift() after end event'); - stream.emit('error', e); - } else { - var skipAdd; - if (state.decoder && !addToFront && !encoding) { - chunk = state.decoder.write(chunk); - skipAdd = (!state.objectMode && chunk.length === 0); - } - - if (!addToFront) + } else { + var er; + if (!skipChunkCheck) + er = chunkInvalid(state, chunk); + if (er) { + stream.emit('error', er); + } else if (state.objectMode || chunk && chunk.length > 0) { + if (addToFront) { + if (state.endEmitted) + stream.emit('error', new Error('stream.unshift() after end event')); + else + addChunk(stream, state, chunk, true); + } else if (state.ended) { + stream.emit('error', new Error('stream.push() after EOF')); + } else { state.reading = false; - - // Don't add to the buffer if we've decoded to an empty string chunk and - // we're not in object mode - if (!skipAdd) { - // if we want the data now, just emit it. - if (state.flowing && state.length === 0 && !state.sync) { - stream.emit('data', chunk); - stream.read(0); - } else { - // update the buffer info. - state.length += state.objectMode ? 1 : chunk.length; - if (addToFront) - state.buffer.unshift(chunk); + if (state.decoder && !encoding) { + chunk = state.decoder.write(chunk); + if (state.objectMode || chunk.length !== 0) + addChunk(stream, state, chunk, false); else - state.buffer.push(chunk); - - if (state.needReadable) - emitReadable(stream); + maybeReadMore(stream, state); + } else { + addChunk(stream, state, chunk, false); } } - - maybeReadMore(stream, state); + } else if (!addToFront) { + state.reading = false; } - } else if (!addToFront) { - state.reading = false; } return needMoreData(state); } +function addChunk(stream, state, chunk, addToFront) { + if (state.flowing && state.length === 0 && !state.sync) { + stream.emit('data', chunk); + stream.read(0); + } else { + // update the buffer info. + state.length += state.objectMode ? 1 : chunk.length; + if (addToFront) + state.buffer.unshift(chunk); + else + state.buffer.push(chunk); + + if (state.needReadable) + emitReadable(stream); + } + maybeReadMore(stream, state); +} + +function chunkInvalid(state, chunk) { + var er; + if (!(chunk instanceof Buffer) && + typeof chunk !== 'string' && + chunk !== undefined && + !state.objectMode) { + er = new TypeError('Invalid non-string/buffer chunk'); + } + return er; +} + // if it's past the high water mark, we can push in some more. // Also, if we have no data yet, we can stand some @@ -267,6 +283,10 @@ function needMoreData(state) { state.length === 0); } +Readable.prototype.isPaused = function() { + return this._readableState.flowing === false; +}; + // backwards compatibility. Readable.prototype.setEncoding = function(enc) { if (!StringDecoder) @@ -438,19 +458,6 @@ Readable.prototype.read = function(n) { return ret; }; -function chunkInvalid(state, chunk) { - var er = null; - if (!(chunk instanceof Buffer) && - typeof chunk !== 'string' && - chunk !== null && - chunk !== undefined && - !state.objectMode) { - er = new TypeError('Invalid non-string/buffer chunk'); - } - return er; -} - - function onEofChunk(stream, state) { if (state.ended) return; if (state.decoder) { From 02bf16e5aef7758900cdee51f96273c2de2b5bdf Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 19 May 2017 18:55:17 +0200 Subject: [PATCH 07/25] doc: link to `common` docs in test writing guide PR-URL: https://github.com/nodejs/node/pull/13118 Reviewed-By: Refael Ackermann Reviewed-By: Gibson Fahnestock Reviewed-By: Michael Dawson Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: Luigi Pinca --- doc/guides/writing-tests.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md index 3cb87623ae8..4d15f73ae83 100644 --- a/doc/guides/writing-tests.md +++ b/doc/guides/writing-tests.md @@ -56,7 +56,7 @@ const common = require('../common'); The first line enables strict mode. All tests should be in strict mode unless the nature of the test requires that the test run without it. -The second line loads the `common` module. The `common` module is a helper +The second line loads the `common` module. The [`common` module][] is a helper module that provides useful tools for the tests. Even if a test uses no functions or other properties exported by `common`, @@ -340,3 +340,4 @@ will depend on what is being tested if this is required or not. [Google Test]: https://github.com/google/googletest [Test fixture]: https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#test-fixtures-using-the-same-data-configuration-for-multiple-tests +[`common` module]: https://github.com/nodejs/node/blob/master/test/common/README.md From a886b17bfb48147fa2007b9968e9dce706a408a2 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Fri, 19 May 2017 10:53:38 -0700 Subject: [PATCH 08/25] src: redirect-warnings to file, not path Use `file` as name of the argument, as the CLI documentation does. PR-URL: https://github.com/nodejs/node/pull/13120 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig Reviewed-By: Gibson Fahnestock Reviewed-By: Anna Henningsen --- src/node.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node.cc b/src/node.cc index efa090cf1f8..5eefed5bd4f 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3620,8 +3620,8 @@ static void PrintHelp() { " --no-warnings silence all process warnings\n" " --napi-modules load N-API modules\n" " --trace-warnings show stack traces on process warnings\n" - " --redirect-warnings=path\n" - " write warnings to path instead of\n" + " --redirect-warnings=file\n" + " write warnings to file instead of\n" " stderr\n" " --trace-sync-io show stack trace when use of sync IO\n" " is detected after the first tick\n" From 3c5bfba28bc9ed244daed7e8f8234b9a4529be85 Mon Sep 17 00:00:00 2001 From: Marcel Laverdet Date: Thu, 18 May 2017 16:53:20 -0500 Subject: [PATCH 09/25] vm: fix displayErrors in runIn.. functions This option has been broken for almost a year when used with any of the vm.runIn.. family of functions, except for syntax errors. PR-URL: https://github.com/nodejs/node/pull/13074 Reviewed-By: Anna Henningsen --- lib/repl.js | 2 +- src/node_contextify.cc | 18 +++++++----------- test/message/vm_display_runtime_error.js | 8 +++++++- test/message/vm_display_runtime_error.out | 15 +++++++++++++++ 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index cff49cea35a..670dbfbc871 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -231,7 +231,7 @@ function REPLServer(prompt, try { try { const scriptOptions = { - displayErrors: true, + displayErrors: false, breakOnSigint: self.breakEvalOnSigint }; diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 970455bb25c..9f2aa31dbb7 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -736,8 +736,10 @@ class ContextifyScript : public BaseObject { return; } - Local decorated_stack = String::Concat(arrow.As(), - stack.As()); + Local decorated_stack = String::Concat( + String::Concat(arrow.As(), + FIXED_ONE_BYTE_STRING(env->isolate(), "\n")), + stack.As()); err_obj->Set(env->stack_string(), decorated_stack); err_obj->SetPrivate( env->context(), @@ -984,6 +986,9 @@ class ContextifyScript : public BaseObject { env->ThrowError("Script execution timed out."); } else if (received_signal) { env->ThrowError("Script execution interrupted."); + } else if (display_errors) { + // We should decorate non-termination exceptions + DecorateErrorStack(env, *try_catch); } // If there was an exception thrown during script execution, re-throw it. @@ -996,15 +1001,6 @@ class ContextifyScript : public BaseObject { return false; } - if (result.IsEmpty()) { - // Error occurred during execution of the script. - if (display_errors) { - DecorateErrorStack(env, *try_catch); - } - try_catch->ReThrow(); - return false; - } - args.GetReturnValue().Set(result); return true; } diff --git a/test/message/vm_display_runtime_error.js b/test/message/vm_display_runtime_error.js index a7a49bba277..8a40a03a039 100644 --- a/test/message/vm_display_runtime_error.js +++ b/test/message/vm_display_runtime_error.js @@ -25,6 +25,12 @@ const vm = require('vm'); console.error('beginning'); -vm.runInThisContext('throw new Error("boo!")', { filename: 'test.vm' }); +try { + vm.runInThisContext('throw new Error("boo!")', { filename: 'test.vm'}); +} catch (err) { + console.error(err.stack); +} + +vm.runInThisContext('throw new Error("spooky!")', { filename: 'test.vm' }); console.error('end'); diff --git a/test/message/vm_display_runtime_error.out b/test/message/vm_display_runtime_error.out index 9035c1783c3..7e116ee8964 100644 --- a/test/message/vm_display_runtime_error.out +++ b/test/message/vm_display_runtime_error.out @@ -14,3 +14,18 @@ Error: boo! at tryModuleLoad (module.js:*:*) at Function.Module._load (module.js:*) at Function.Module.runMain (module.js:*) +test.vm:1 +throw new Error("spooky!") +^ + +Error: spooky! + at test.vm:1:7 + at ContextifyScript.Script.runInThisContext (vm.js:*) + at Object.runInThisContext (vm.js:*) + at Object. (*test*message*vm_display_runtime_error.js:*) + at Module._compile (module.js:*) + at Object.Module._extensions..js (module.js:*) + at Module.load (module.js:*) + at tryModuleLoad (module.js:*:*) + at Function.Module._load (module.js:*) + at Function.Module.runMain (module.js:*) From e018c331114c5a8e73e374cb493730e7dcc9bc91 Mon Sep 17 00:00:00 2001 From: Marcel Laverdet Date: Thu, 18 May 2017 17:12:41 -0500 Subject: [PATCH 10/25] vm: fix race condition with timeout param This fixes a race condition in the watchdog timer used for vm timeouts. The condition would terminate the main stack's execution instead of the code running under the sandbox. PR-URL: https://github.com/nodejs/node/pull/13074 Reviewed-By: Anna Henningsen --- src/node_contextify.cc | 24 ++++++++--------- src/node_watchdog.cc | 53 ++++++++----------------------------- src/node_watchdog.h | 26 ++++++------------ test/pummel/test-vm-race.js | 33 +++++++++++++++++++++++ 4 files changed, 63 insertions(+), 73 deletions(-) create mode 100644 test/pummel/test-vm-race.js diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 9f2aa31dbb7..792b1c4f800 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -958,27 +958,20 @@ class ContextifyScript : public BaseObject { bool timed_out = false; bool received_signal = false; if (break_on_sigint && timeout != -1) { - Watchdog wd(env->isolate(), timeout); - SigintWatchdog swd(env->isolate()); + Watchdog wd(env->isolate(), timeout, &timed_out); + SigintWatchdog swd(env->isolate(), &received_signal); result = script->Run(); - timed_out = wd.HasTimedOut(); - received_signal = swd.HasReceivedSignal(); } else if (break_on_sigint) { - SigintWatchdog swd(env->isolate()); + SigintWatchdog swd(env->isolate(), &received_signal); result = script->Run(); - received_signal = swd.HasReceivedSignal(); } else if (timeout != -1) { - Watchdog wd(env->isolate(), timeout); + Watchdog wd(env->isolate(), timeout, &timed_out); result = script->Run(); - timed_out = wd.HasTimedOut(); } else { result = script->Run(); } - if (try_catch->HasCaught()) { - if (try_catch->HasTerminated()) - env->isolate()->CancelTerminateExecution(); - + if (timed_out || received_signal) { // It is possible that execution was terminated by another timeout in // which this timeout is nested, so check whether one of the watchdogs // from this invocation is responsible for termination. @@ -986,7 +979,12 @@ class ContextifyScript : public BaseObject { env->ThrowError("Script execution timed out."); } else if (received_signal) { env->ThrowError("Script execution interrupted."); - } else if (display_errors) { + } + env->isolate()->CancelTerminateExecution(); + } + + if (try_catch->HasCaught()) { + if (!timed_out && !received_signal && display_errors) { // We should decorate non-termination exceptions DecorateErrorStack(env, *try_catch); } diff --git a/src/node_watchdog.cc b/src/node_watchdog.cc index 3065343ddef..f4020e56f7f 100644 --- a/src/node_watchdog.cc +++ b/src/node_watchdog.cc @@ -27,9 +27,9 @@ namespace node { -Watchdog::Watchdog(v8::Isolate* isolate, uint64_t ms) : isolate_(isolate), - timed_out_(false), - destroyed_(false) { +Watchdog::Watchdog(v8::Isolate* isolate, uint64_t ms, bool* timed_out) + : isolate_(isolate), timed_out_(timed_out) { + int rc; loop_ = new uv_loop_t; CHECK(loop_); @@ -54,20 +54,6 @@ Watchdog::Watchdog(v8::Isolate* isolate, uint64_t ms) : isolate_(isolate), Watchdog::~Watchdog() { - Destroy(); -} - - -void Watchdog::Dispose() { - Destroy(); -} - - -void Watchdog::Destroy() { - if (destroyed_) { - return; - } - uv_async_send(&async_); uv_thread_join(&thread_); @@ -80,8 +66,6 @@ void Watchdog::Destroy() { CHECK_EQ(0, rc); delete loop_; loop_ = nullptr; - - destroyed_ = true; } @@ -93,7 +77,7 @@ void Watchdog::Run(void* arg) { uv_run(wd->loop_, UV_RUN_DEFAULT); // Loop ref count reaches zero when both handles are closed. - // Close the timer handle on this side and let Destroy() close async_ + // Close the timer handle on this side and let ~Watchdog() close async_ uv_close(reinterpret_cast(&wd->timer_), nullptr); } @@ -106,24 +90,15 @@ void Watchdog::Async(uv_async_t* async) { void Watchdog::Timer(uv_timer_t* timer) { Watchdog* w = ContainerOf(&Watchdog::timer_, timer); - w->timed_out_ = true; - uv_stop(w->loop_); + *w->timed_out_ = true; w->isolate()->TerminateExecution(); + uv_stop(w->loop_); } -SigintWatchdog::~SigintWatchdog() { - Destroy(); -} - - -void SigintWatchdog::Dispose() { - Destroy(); -} - - -SigintWatchdog::SigintWatchdog(v8::Isolate* isolate) - : isolate_(isolate), received_signal_(false), destroyed_(false) { +SigintWatchdog::SigintWatchdog( + v8::Isolate* isolate, bool* received_signal) + : isolate_(isolate), received_signal_(received_signal) { // Register this watchdog with the global SIGINT/Ctrl+C listener. SigintWatchdogHelper::GetInstance()->Register(this); // Start the helper thread, if that has not already happened. @@ -131,20 +106,14 @@ SigintWatchdog::SigintWatchdog(v8::Isolate* isolate) } -void SigintWatchdog::Destroy() { - if (destroyed_) { - return; - } - - destroyed_ = true; - +SigintWatchdog::~SigintWatchdog() { SigintWatchdogHelper::GetInstance()->Unregister(this); SigintWatchdogHelper::GetInstance()->Stop(); } void SigintWatchdog::HandleSigint() { - received_signal_ = true; + *received_signal_ = true; isolate_->TerminateExecution(); } diff --git a/src/node_watchdog.h b/src/node_watchdog.h index 9dd5a03f816..2cc7eab837c 100644 --- a/src/node_watchdog.h +++ b/src/node_watchdog.h @@ -37,16 +37,13 @@ namespace node { class Watchdog { public: - explicit Watchdog(v8::Isolate* isolate, uint64_t ms); + explicit Watchdog(v8::Isolate* isolate, + uint64_t ms, + bool* timed_out = nullptr); ~Watchdog(); - - void Dispose(); - v8::Isolate* isolate() { return isolate_; } - bool HasTimedOut() { return timed_out_; } - private: - void Destroy(); + private: static void Run(void* arg); static void Async(uv_async_t* async); static void Timer(uv_timer_t* timer); @@ -56,27 +53,20 @@ class Watchdog { uv_loop_t* loop_; uv_async_t async_; uv_timer_t timer_; - bool timed_out_; - bool destroyed_; + bool* timed_out_; }; class SigintWatchdog { public: - explicit SigintWatchdog(v8::Isolate* isolate); + explicit SigintWatchdog(v8::Isolate* isolate, + bool* received_signal = nullptr); ~SigintWatchdog(); - - void Dispose(); - v8::Isolate* isolate() { return isolate_; } - bool HasReceivedSignal() { return received_signal_; } void HandleSigint(); private: - void Destroy(); - v8::Isolate* isolate_; - bool received_signal_; - bool destroyed_; + bool* received_signal_; }; class SigintWatchdogHelper { diff --git a/test/pummel/test-vm-race.js b/test/pummel/test-vm-race.js new file mode 100644 index 00000000000..7f9514b04e9 --- /dev/null +++ b/test/pummel/test-vm-race.js @@ -0,0 +1,33 @@ +'use strict'; +require('../common'); +const vm = require('vm'); + +// We're testing a race condition so we just have to spin this in a loop +// for a little while and see if it breaks. The condition being tested +// is an `isolate->TerminateExecution()` reaching the main JS stack from +// the timeout watchdog. +const sandbox = { timeout: 5 }; +const context = vm.createContext(sandbox); +const script = new vm.Script( + 'var d = Date.now() + timeout;while (d > Date.now());' +); +const immediate = setImmediate(function() { + throw new Error('Detected vm race condition!'); +}); + +// When this condition was first discovered this test would fail in 50ms +// or so. A better, but still incorrect implementation would fail after +// 100 seconds or so. If you're messing with vm timeouts you might +// consider increasing this timeout to hammer out races. +const giveUp = Date.now() + 5000; +do { + // The loop adjusts the timeout up or down trying to hit the race + try { + script.runInContext(context, { timeout: 5 }); + ++sandbox.timeout; + } catch (err) { + --sandbox.timeout; + } +} while (Date.now() < giveUp); + +clearImmediate(immediate); From f3ef9719b5c18775fcfdb12dd62aaeef8caeb94a Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Fri, 19 May 2017 17:34:11 -0400 Subject: [PATCH 11/25] doc: fix title/function name mismatch Fix mismatch in title for napi_get_value_string_utf16 Fixes: https://github.com/nodejs/abi-stable-node/issues/243 PR-URL: https://github.com/nodejs/node/pull/13123 Reviewed-By: Anna Henningsen Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Gibson Fahnestock Reviewed-By: Colin Ihrig --- doc/api/n-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 507df04cf96..6136929e4a9 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -1590,7 +1590,7 @@ x is passed in it returns `napi_string_expected`. This API returns the UTF8-encoded string corresponding the value passed in. -#### *napi_get_value_string_utf16_length* +#### *napi_get_value_string_utf16* From 47919b3b6f1592ce80ec3c07b0eaea64c1d7e4dc Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Fri, 19 May 2017 18:18:54 -0400 Subject: [PATCH 12/25] test: increase n-api constructor coverage Add tests to validate that properties marked as static are available through the class as opposed to instances PR-URL: https://github.com/nodejs/node/pull/13124 Reviewed-By: Jason Ginchereau Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Hitesh Kanwathirtha --- test/addons-napi/test_constructor/test.js | 5 +++++ .../test_constructor/test_constructor.c | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/test/addons-napi/test_constructor/test.js b/test/addons-napi/test_constructor/test.js index 26083db7a28..75c7b367830 100644 --- a/test/addons-napi/test_constructor/test.js +++ b/test/addons-napi/test_constructor/test.js @@ -40,3 +40,8 @@ test_object.readwriteAccessor2 = 2; assert.strictEqual(test_object.readwriteAccessor2, 2); assert.strictEqual(test_object.readonlyAccessor2, 2); assert.throws(() => { test_object.readonlyAccessor2 = 3; }, TypeError); + +// validate that static properties are on the class as opposed +// to the instance +assert.strictEqual(TestConstructor.staticReadonlyAccessor1, 10); +assert.strictEqual(test_object.staticReadonlyAccessor1, undefined); diff --git a/test/addons-napi/test_constructor/test_constructor.c b/test/addons-napi/test_constructor/test_constructor.c index 0a73010d72f..220d564753c 100644 --- a/test/addons-napi/test_constructor/test_constructor.c +++ b/test/addons-napi/test_constructor/test_constructor.c @@ -2,6 +2,7 @@ #include "../common.h" static double value_ = 1; +static double static_value_ = 10; napi_ref constructor_; napi_value GetValue(napi_env env, napi_callback_info info) { @@ -45,6 +46,19 @@ napi_value New(napi_env env, napi_callback_info info) { return _this; } +napi_value GetStaticValue(napi_env env, napi_callback_info info) { + size_t argc = 0; + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, NULL, NULL, NULL)); + + NAPI_ASSERT(env, argc == 0, "Wrong number of arguments"); + + napi_value number; + NAPI_CALL(env, napi_create_number(env, static_value_, &number)); + + return number; +} + + void Init(napi_env env, napi_value exports, napi_value module, void* priv) { napi_value number; NAPI_CALL_RETURN_VOID(env, napi_create_number(env, value_, &number)); @@ -58,6 +72,8 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) { { "readwriteAccessor2", 0, 0, GetValue, SetValue, 0, napi_writable, 0}, { "readonlyAccessor1", 0, 0, GetValue, NULL, 0, napi_default, 0}, { "readonlyAccessor2", 0, 0, GetValue, NULL, 0, napi_writable, 0}, + { "staticReadonlyAccessor1", 0, 0, GetStaticValue, NULL, 0, + napi_default | napi_static, 0}, }; napi_value cons; From b5b6f5d6a9a5402e55f5080928d740e12e7295a4 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 19 May 2017 22:14:21 -0700 Subject: [PATCH 13/25] test: check number of message events When a no-op message event handler is used in a test, make it clear what is expected by using `common.mustCall()` and `common.mustNotCall()`. PR-URL: https://github.com/nodejs/node/pull/13125 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: Gibson Fahnestock Reviewed-By: Refael Ackermann Reviewed-By: Luigi Pinca --- test/parallel/test-child-process-fork-ref2.js | 2 +- test/sequential/test-child-process-pass-fd.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-child-process-fork-ref2.js b/test/parallel/test-child-process-fork-ref2.js index f114170630d..f34d67ee17f 100644 --- a/test/parallel/test-child-process-fork-ref2.js +++ b/test/parallel/test-child-process-fork-ref2.js @@ -29,7 +29,7 @@ if (process.argv[2] === 'child') { setTimeout(function() { console.log('child -> will this keep it alive?'); - process.on('message', common.noop); + process.on('message', common.mustNotCall()); }, 400); } else { diff --git a/test/sequential/test-child-process-pass-fd.js b/test/sequential/test-child-process-pass-fd.js index 2d05407e67b..b2481270faf 100644 --- a/test/sequential/test-child-process-pass-fd.js +++ b/test/sequential/test-child-process-pass-fd.js @@ -45,7 +45,7 @@ if (process.argv[2] !== 'child') { // the only thing keeping this worker alive will be IPC. This is important, // because it means a worker with no parent will have no referenced handles, // thus no work to do, and will exit immediately, preventing process leaks. - process.on('message', common.noop); + process.on('message', common.mustCall()); const server = net.createServer((c) => { process.once('message', function(msg) { From 7c12d12ecf7fb4f0e22bc45690e123d827a0820d Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 20 May 2017 09:50:51 -0700 Subject: [PATCH 14/25] test: confirm callback is invoked in fs test Use common.mustCall() in test-fs-makeStatsCallback to confirm that the callback is invoked. PR-URL: https://github.com/nodejs/node/pull/13132 Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Michael Dawson Reviewed-By: Alexey Orlenko Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Luigi Pinca --- test/parallel/test-fs-makeStatsCallback.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fs-makeStatsCallback.js b/test/parallel/test-fs-makeStatsCallback.js index 84a46bb72e5..12120e97376 100644 --- a/test/parallel/test-fs-makeStatsCallback.js +++ b/test/parallel/test-fs-makeStatsCallback.js @@ -16,7 +16,7 @@ function testMakeStatsCallback(cb) { common.expectWarning('DeprecationWarning', warn); // Verify the case where a callback function is provided -assert.doesNotThrow(testMakeStatsCallback(common.noop)); +assert.doesNotThrow(testMakeStatsCallback(common.mustCall())); // Passing undefined/nothing calls rethrow() internally, which emits a warning assert.doesNotThrow(testMakeStatsCallback()); From ef56d47f509cfe727db3fa3c842e2c66b916489d Mon Sep 17 00:00:00 2001 From: Alexey Orlenko Date: Sun, 21 May 2017 01:02:00 +0300 Subject: [PATCH 15/25] doc: fix incorrect keyboard shortcut This commit fixes an incorrect keyboard shortcut in `doc/STYLE_GUIDE.md`: entering em-dashes is done via Alt+Shift+"-" on macOS, not via Ctrl+Alt+"-". Besides that, Option is more canonical name of Alt on Macs. PR-URL: https://github.com/nodejs/node/pull/13134 Reviewed-By: Rich Trott Reviewed-By: Colin Ihrig Reviewed-By: Daijiro Wachi Reviewed-By: Luigi Pinca Reviewed-By: Michael Dawson Reviewed-By: Gibson Fahnestock --- doc/STYLE_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/STYLE_GUIDE.md b/doc/STYLE_GUIDE.md index 0d3fc001073..f11c5c05231 100644 --- a/doc/STYLE_GUIDE.md +++ b/doc/STYLE_GUIDE.md @@ -39,7 +39,7 @@ * When documenting APIs, note the version the API was introduced in at the end of the section. If an API has been deprecated, also note the first version that the API appeared deprecated in. -* When using dashes, use emdashes ("—", Ctrl+Alt+"-" on macOS) surrounded by +* When using dashes, use emdashes ("—", Option+Shift+"-" on macOS) surrounded by spaces, per the New York Times usage. * Including assets: * If you wish to add an illustration or full program, add it to the From eebc2622781d62125cd37a1c5ad06f26ba0a117f Mon Sep 17 00:00:00 2001 From: Alexey Orlenko Date: Sun, 21 May 2017 02:12:15 +0300 Subject: [PATCH 16/25] doc: improve formatting of STYLE_GUIDE.md * Wrap text at 80 characters. * Use periods consistently. PR-URL: https://github.com/nodejs/node/pull/13135 Reviewed-By: Anna Henningsen Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Rich Trott Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: Gibson Fahnestock --- doc/STYLE_GUIDE.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/doc/STYLE_GUIDE.md b/doc/STYLE_GUIDE.md index f11c5c05231..b535d2babcd 100644 --- a/doc/STYLE_GUIDE.md +++ b/doc/STYLE_GUIDE.md @@ -1,16 +1,17 @@ # Style Guide * Documents are written in markdown files. -* Those files should be written in **`lowercase-with-dashes.md`.** +* Those files should be written in **`lowercase-with-dashes.md`**. * Underscores in filenames are allowed only when they are present in the - topic the document will describe (e.g., `child_process`.) + topic the document will describe (e.g., `child_process`). * Filenames should be **lowercase**. * Some files, such as top-level markdown files, are exceptions. * Older files may use the `.markdown` extension. These may be ported to `.md` at will. **Prefer `.md` for all new documents.** * Documents should be word-wrapped at 80 characters. * The formatting described in `.editorconfig` is preferred. - * A [plugin][] is available for some editors to automatically apply these rules. + * A [plugin][] is available for some editors to automatically apply these + rules. * Mechanical issues, like spelling and grammar, should be identified by tools, insofar as is possible. If not caught by a tool, they should be pointed out by human reviewers. @@ -18,12 +19,12 @@ "color" vs. "colour", etc. * Though controversial, the [Oxford comma][] is preferred for clarity's sake. * Generally avoid personal pronouns in reference documentation ("I", "you", - "we".) + "we"). * Pronouns are acceptable in more colloquial documentation, like guides. * Use **gender-neutral pronouns** and **mass nouns**. Non-comprehensive examples: * **OK**: "they", "their", "them", "folks", "people", "developers", "cats" - * **NOT OK**: "his", "hers", "him", "her", "guys", "dudes". + * **NOT OK**: "his", "hers", "him", "her", "guys", "dudes" * When combining wrapping elements (parentheses and quotes), terminal punctuation should be placed: * Inside the wrapping element if the wrapping element contains a complete @@ -54,10 +55,12 @@ your point, not as complete running programs. If a complete running program is necessary, include it as an asset in `assets/code-examples` and link to it. -* When using underscores, asterisks and backticks please use proper escaping (**\\\_**, **\\\*** and **\\\`** instead of **\_**, **\*** and **\`**) -* References to constructor functions should use PascalCase -* References to constructor instances should be camelCased -* References to methods should be used with parentheses: `socket.end()` instead of `socket.end` +* When using underscores, asterisks, and backticks, please use proper escaping + (**\\\_**, **\\\*** and **\\\`** instead of **\_**, **\*** and **\`**). +* References to constructor functions should use PascalCase. +* References to constructor instances should use camelCase. +* References to methods should be used with parentheses: for example, + `socket.end()` instead of `socket.end`. [plugin]: http://editorconfig.org/#download [Oxford comma]: https://en.wikipedia.org/wiki/Serial_comma From 9278ce202ac6d188faeff514b67f4646786efc58 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sun, 21 May 2017 13:18:16 +0300 Subject: [PATCH 17/25] doc: update code example for Windows in stream.md PR-URL: https://github.com/nodejs/node/pull/13138 Reviewed-By: Anna Henningsen Reviewed-By: Luigi Pinca Reviewed-By: Matteo Collina Reviewed-By: Gibson Fahnestock Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- doc/api/stream.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/api/stream.md b/doc/api/stream.md index a17a0c455d7..aeb6c930378 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -134,7 +134,7 @@ const server = http.createServer((req, res) => { res.write(typeof data); res.end(); } catch (er) { - // uh oh! bad json! + // uh oh! bad json! res.statusCode = 400; return res.end(`error: ${er.message}`); } @@ -143,12 +143,12 @@ const server = http.createServer((req, res) => { server.listen(1337); -// $ curl localhost:1337 -d '{}' +// $ curl localhost:1337 -d "{}" // object -// $ curl localhost:1337 -d '"foo"' +// $ curl localhost:1337 -d "\"foo\"" // string -// $ curl localhost:1337 -d 'not json' -// error: Unexpected token o +// $ curl localhost:1337 -d "not json" +// error: Unexpected token o in JSON at position 1 ``` [Writable][] streams (such as `res` in the example) expose methods such as From 6af72d4b037eba38d94395f57a03a498a2efef09 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sun, 21 May 2017 21:53:57 +0300 Subject: [PATCH 18/25] doc: don't use useless constructors in stream.md PR-URL: https://github.com/nodejs/node/pull/13145 Refs: http://eslint.org/docs/rules/no-useless-constructor Reviewed-By: Refael Ackermann Reviewed-By: Matteo Collina Reviewed-By: Daniel Bevenius Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- doc/api/stream.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/api/stream.md b/doc/api/stream.md index aeb6c930378..c2a257cb5a5 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -1157,6 +1157,7 @@ const Writable = require('stream').Writable; class MyWritable extends Writable { constructor(options) { super(options); + // ... } } ``` @@ -1288,6 +1289,7 @@ class MyWritable extends Writable { constructor(options) { // Calls the stream.Writable() constructor super(options); + // ... } } ``` @@ -1433,6 +1435,7 @@ const Writable = require('stream').Writable; class MyWritable extends Writable { constructor(options) { super(options); + // ... } _write(chunk, encoding, callback) { @@ -1477,6 +1480,7 @@ class MyReadable extends Readable { constructor(options) { // Calls the stream.Readable(options) constructor super(options); + // ... } } ``` @@ -1690,6 +1694,7 @@ const Duplex = require('stream').Duplex; class MyDuplex extends Duplex { constructor(options) { super(options); + // ... } } ``` @@ -1845,6 +1850,7 @@ const Transform = require('stream').Transform; class MyTransform extends Transform { constructor(options) { super(options); + // ... } } ``` From ccd3eadbd7dae3a23d43bf490fa9d3019324370e Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 22 May 2017 18:03:55 +0200 Subject: [PATCH 19/25] stream: fix destroy(err, cb) regression Fixed a regression that caused the callback passed to destroy() to not be called if the stream was already destroyed. This caused a regression on the ws module in CITGM introduced by https://github.com/nodejs/node/pull/12925. PR-URL: https://github.com/nodejs/node/pull/13156 Fixes: https://github.com/websockets/ws/issues/1118 Reviewed-By: Colin Ihrig Reviewed-By: Calvin Metcalf Reviewed-By: Luigi Pinca Reviewed-By: Refael Ackermann --- lib/internal/streams/destroy.js | 5 ++++- test/parallel/test-net-socket-destroy-send.js | 22 +++++++++++++++++++ test/parallel/test-stream-readable-destroy.js | 14 ++++++++++++ test/parallel/test-stream-writable-destroy.js | 15 +++++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-net-socket-destroy-send.js diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js index 18a2a839843..37e7d4bc364 100644 --- a/lib/internal/streams/destroy.js +++ b/lib/internal/streams/destroy.js @@ -8,7 +8,10 @@ function destroy(err, cb) { this._writableState.destroyed; if (readableDestroyed || writableDestroyed) { - if (err && (!this._writableState || !this._writableState.errorEmitted)) { + if (cb) { + cb(err); + } else if (err && + (!this._writableState || !this._writableState.errorEmitted)) { process.nextTick(emitErrorNT, this, err); } return; diff --git a/test/parallel/test-net-socket-destroy-send.js b/test/parallel/test-net-socket-destroy-send.js new file mode 100644 index 00000000000..309e41f6c94 --- /dev/null +++ b/test/parallel/test-net-socket-destroy-send.js @@ -0,0 +1,22 @@ +'use strict'; + +const common = require('../common'); +const net = require('net'); +const assert = require('assert'); + +const server = net.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + const conn = net.createConnection(port); + + conn.on('connect', common.mustCall(function() { + conn.destroy(); + conn.on('error', common.mustCall(function(err) { + assert.strictEqual(err.message, 'This socket is closed'); + })); + conn.write(Buffer.from('kaboom'), common.mustCall(function(err) { + assert.strictEqual(err.message, 'This socket is closed'); + })); + server.close(); + })); +})); diff --git a/test/parallel/test-stream-readable-destroy.js b/test/parallel/test-stream-readable-destroy.js index 800b6be0865..def20d26c34 100644 --- a/test/parallel/test-stream-readable-destroy.js +++ b/test/parallel/test-stream-readable-destroy.js @@ -160,3 +160,17 @@ const { inherits } = require('util'); new MyReadable(); } + +{ + // destroy and destroy callback + const read = new Readable({ + read() {} + }); + read.resume(); + + const expected = new Error('kaboom'); + + read.destroy(expected, common.mustCall(function(err) { + assert.strictEqual(expected, err); + })); +} diff --git a/test/parallel/test-stream-writable-destroy.js b/test/parallel/test-stream-writable-destroy.js index a91f148f9e5..87e55eccc3f 100644 --- a/test/parallel/test-stream-writable-destroy.js +++ b/test/parallel/test-stream-writable-destroy.js @@ -170,3 +170,18 @@ const { inherits } = require('util'); new MyWritable(); } + +{ + // destroy and destroy callback + const write = new Writable({ + write(chunk, enc, cb) { cb(); } + }); + + write.destroy(); + + const expected = new Error('kaboom'); + + write.destroy(expected, common.mustCall(function(err) { + assert.strictEqual(expected, err); + })); +} From bbd39c359f8a645687a001cf96f9d606240855b5 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 21 May 2017 14:36:12 -0700 Subject: [PATCH 20/25] test: check noop function invocations In test/pummel/test-stream2-basic.js, check that noop functions are called the expected number of times. PR-URL: https://github.com/nodejs/node/pull/13146 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- test/pummel/test-stream2-basic.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/pummel/test-stream2-basic.js b/test/pummel/test-stream2-basic.js index 717c11b07e4..8a1d940ce77 100644 --- a/test/pummel/test-stream2-basic.js +++ b/test/pummel/test-stream2-basic.js @@ -147,7 +147,7 @@ test('a most basic test', function(t) { 'xxxxxxxxxxxxxxxxxxxxx' ]; r.on('end', function() { - t.same(reads, expect); + assert.strict(reads, expect); t.end(); }); @@ -180,7 +180,7 @@ test('pipe', function(t) { const w = new TestWriter(); w.on('end', function(received) { - t.same(received, expect); + assert.strict(received, expect); t.end(); }); @@ -226,7 +226,7 @@ test('pipe', function(t) { t.equal(ended0, false); ended0 = true; ended++; - t.same(results, expect[0]); + assert.strict(results, expect[0]); }); w[1].on('end', function(results) { @@ -234,7 +234,7 @@ test('pipe', function(t) { ended1 = true; ended++; t.equal(ended, 2); - t.same(results, expect[1]); + assert.strict(results, expect[1]); t.end(); }); @@ -261,11 +261,11 @@ test('multipipe', function(t) { let c = 2; w[0].on('end', function(received) { - t.same(received, expect, 'first'); + assert.strict(received, expect, 'first'); if (--c === 0) t.end(); }); w[1].on('end', function(received) { - t.same(received, expect, 'second'); + assert.strict(received, expect, 'second'); if (--c === 0) t.end(); }); @@ -306,13 +306,13 @@ test('multipipe', function(t) { w[0].on('end', function(results) { ended++; - t.same(results, expect[0]); + assert.strict(results, expect[0]); }); w[1].on('end', function(results) { ended++; t.equal(ended, 2); - t.same(results, expect[1]); + assert.strict(results, expect[1]); t.end(); }); @@ -323,7 +323,7 @@ test('multipipe', function(t) { test('back pressure respected', function(t) { const r = new R({ objectMode: true }); - r._read = common.noop; + r._read = common.mustNotCall(); let counter = 0; r.push(['one']); r.push(['two']); @@ -341,7 +341,7 @@ test('back pressure respected', function(t) { r.pipe(w3); }); }; - w1.end = common.noop; + w1.end = common.mustNotCall(); r.pipe(w1); @@ -367,7 +367,7 @@ test('back pressure respected', function(t) { return false; }; - w2.end = common.noop; + w2.end = common.mustCall(); const w3 = new R(); w3.write = function(chunk) { @@ -400,7 +400,7 @@ test('read(0) for ended streams', function(t) { const r = new R(); let written = false; let ended = false; - r._read = common.noop; + r._read = common.mustNotCall(); r.push(Buffer.from('foo')); r.push(null); @@ -471,7 +471,7 @@ test('adding readable triggers data flow', function(t) { test('chainable', function(t) { const r = new R(); - r._read = common.noop; + r._read = common.mustCall(); const r2 = r.setEncoding('utf8').pause().resume().pause(); t.equal(r, r2); t.end(); From 7bb9fd203522cb37424629dad777f966a81dd03d Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 21 May 2017 14:40:17 -0700 Subject: [PATCH 21/25] test: simplify assert usage in test-stream2-basic PR-URL: https://github.com/nodejs/node/pull/13146 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- test/pummel/test-stream2-basic.js | 37 ++++++++++++++----------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/test/pummel/test-stream2-basic.js b/test/pummel/test-stream2-basic.js index 8a1d940ce77..cb017a17065 100644 --- a/test/pummel/test-stream2-basic.js +++ b/test/pummel/test-stream2-basic.js @@ -107,9 +107,6 @@ function run() { const fn = next[1]; console.log('# %s', name); fn({ - same: assert.deepStrictEqual, - ok: assert, - equal: assert.strictEqual, end: function() { count--; run(); @@ -147,7 +144,7 @@ test('a most basic test', function(t) { 'xxxxxxxxxxxxxxxxxxxxx' ]; r.on('end', function() { - assert.strict(reads, expect); + assert.deepStrictEqual(reads, expect); t.end(); }); @@ -180,7 +177,7 @@ test('pipe', function(t) { const w = new TestWriter(); w.on('end', function(received) { - assert.strict(received, expect); + assert.deepStrictEqual(received, expect); t.end(); }); @@ -211,10 +208,10 @@ test('pipe', function(t) { w[0].on('write', function() { if (--writes === 0) { r.unpipe(); - t.equal(r._readableState.pipes, null); + assert.strictEqual(r._readableState.pipes, null); w[0].end(); r.pipe(w[1]); - t.equal(r._readableState.pipes, w[1]); + assert.strictEqual(r._readableState.pipes, w[1]); } }); @@ -223,18 +220,18 @@ test('pipe', function(t) { let ended0 = false; let ended1 = false; w[0].on('end', function(results) { - t.equal(ended0, false); + assert.strictEqual(ended0, false); ended0 = true; ended++; - assert.strict(results, expect[0]); + assert.deepStrictEqual(results, expect[0]); }); w[1].on('end', function(results) { - t.equal(ended1, false); + assert.strictEqual(ended1, false); ended1 = true; ended++; - t.equal(ended, 2); - assert.strict(results, expect[1]); + assert.strictEqual(ended, 2); + assert.deepStrictEqual(results, expect[1]); t.end(); }); @@ -261,11 +258,11 @@ test('multipipe', function(t) { let c = 2; w[0].on('end', function(received) { - assert.strict(received, expect, 'first'); + assert.deepStrictEqual(received, expect, 'first'); if (--c === 0) t.end(); }); w[1].on('end', function(received) { - assert.strict(received, expect, 'second'); + assert.deepStrictEqual(received, expect, 'second'); if (--c === 0) t.end(); }); @@ -306,13 +303,13 @@ test('multipipe', function(t) { w[0].on('end', function(results) { ended++; - assert.strict(results, expect[0]); + assert.deepStrictEqual(results, expect[0]); }); w[1].on('end', function(results) { ended++; - t.equal(ended, 2); - assert.strict(results, expect[1]); + assert.strictEqual(ended, 2); + assert.deepStrictEqual(results, expect[1]); t.end(); }); @@ -463,8 +460,8 @@ test('adding readable triggers data flow', function(t) { }); r.on('end', function() { - t.equal(readCalled, 3); - t.ok(onReadable); + assert.strictEqual(readCalled, 3); + assert.ok(onReadable); t.end(); }); }); @@ -473,6 +470,6 @@ test('chainable', function(t) { const r = new R(); r._read = common.mustCall(); const r2 = r.setEncoding('utf8').pause().resume().pause(); - t.equal(r, r2); + assert.strictEqual(r, r2); t.end(); }); From 1aad4ba2841120d04661681cd7a60b7124912d42 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 21 May 2017 14:43:09 -0700 Subject: [PATCH 22/25] test: move stream2 test from pummel to parallel test-stream2-basic runs in a few seconds. It can be moved to parallel. PR-URL: https://github.com/nodejs/node/pull/13146 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- test/{pummel => parallel}/test-stream2-basic.js | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{pummel => parallel}/test-stream2-basic.js (100%) diff --git a/test/pummel/test-stream2-basic.js b/test/parallel/test-stream2-basic.js similarity index 100% rename from test/pummel/test-stream2-basic.js rename to test/parallel/test-stream2-basic.js From e912c67d24ba42eeb47431a2b163f7f8a8532c78 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 9 May 2017 14:18:35 -0400 Subject: [PATCH 23/25] dgram: convert to using internal/errors Covert lib/dgram.js over to using lib/internal/errors.js for generating Errors. See [using-internal-errors.md](https://github.com/nodejs/node/blob/master/doc/guides/using-internal-errors.md) for more details. I have not addressed the cases that use errnoException() and exceptionWithHostPort() helper methods as changing these would require fixing the tests across all of the different files that use them. In addition, these helpers already add a `code` to the Error and we'll have to discuss how that interacts with the `code` used by lib/internal/errors.js. I believe we should convert all users of errnoException and exceptionWithHostPort in a PR dedicated to that conversion. PR-URL: https://github.com/nodejs/node/pull/12926 Reviewed-By: James M Snell Reviewed-By: Franziska Hinkelmann Reviewed-By: Ruben Bridgewater --- doc/api/errors.md | 29 +++++++++ lib/dgram.js | 64 +++++++++++++------ lib/internal/errors.js | 6 ++ test/parallel/test-dgram-bind.js | 6 +- test/parallel/test-dgram-createSocket-type.js | 8 ++- .../test-dgram-implicit-bind-failure.js | 2 +- test/parallel/test-dgram-membership.js | 36 ++++++++--- test/parallel/test-dgram-multicast-setTTL.js | 6 +- .../parallel/test-dgram-send-address-types.js | 13 ++-- test/parallel/test-dgram-sendto.js | 38 +++++++++-- test/parallel/test-dgram-setTTL.js | 6 +- 11 files changed, 167 insertions(+), 47 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index ebafd5a05fb..40b8e7ab74c 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -703,6 +703,35 @@ in some cases may accept `func(undefined)` but not `func()`). In most native Node.js APIs, `func(undefined)` and `func()` are treated identically, and the [`ERR_INVALID_ARG_TYPE`][] error code may be used instead. + +### ERR_SOCKET_ALREADY_BOUND +An error using the `'ERR_SOCKET_ALREADY_BOUND'` code is thrown when an attempt +is made to bind a socket that has already been bound. + + +### ERR_SOCKET_BAD_PORT + +An error using the `'ERR_SOCKET_BAD_PORT'` code is thrown when an API +function expecting a port > 0 and < 65536 receives an invalid value. + + +### ERR_SOCKET_BAD_TYPE + +An error using the `'ERR_SOCKET_BAD_TYPE'` code is thrown when an API +function expecting a socket type (`udp4` or `udp6`) receives an invalid value. + + +### ERR_SOCKET_CANNOT_SEND + +An error using the `'ERR_SOCKET_CANNOT_SEND'` code is thrown when data +cannot be sent on a socket. + + +### ERR_SOCKET_DGRAM_NOT_RUNNING + +An error using the `'ERR_SOCKET_DGRAM_NOT_RUNNING'` code is thrown +when a call is made and the UDP subsystem is not running. + ### ERR_STDERR_CLOSE diff --git a/lib/dgram.js b/lib/dgram.js index 4d9f7eb29fd..0d93ca28748 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -22,6 +22,7 @@ 'use strict'; const assert = require('assert'); +const errors = require('internal/errors'); const Buffer = require('buffer').Buffer; const util = require('util'); const EventEmitter = require('events'); @@ -78,7 +79,7 @@ function newHandle(type) { return handle; } - throw new Error('Bad socket type specified. Valid types are: udp4, udp6'); + throw new errors.Error('ERR_SOCKET_BAD_TYPE'); } @@ -162,7 +163,7 @@ Socket.prototype.bind = function(port_, address_ /*, callback*/) { this._healthCheck(); if (this._bindState !== BIND_STATE_UNBOUND) - throw new Error('Socket is already bound'); + throw new errors.Error('ERR_SOCKET_ALREADY_BOUND'); this._bindState = BIND_STATE_BINDING; @@ -260,11 +261,21 @@ Socket.prototype.sendto = function(buffer, port, address, callback) { - if (typeof offset !== 'number' || typeof length !== 'number') - throw new Error('Send takes "offset" and "length" as args 2 and 3'); + if (typeof offset !== 'number') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'offset', 'number'); + } + + if (typeof length !== 'number') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'length', 'number'); + } - if (typeof address !== 'string') - throw new Error(this.type + ' sockets must send to port, address'); + if (typeof port !== 'number') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'port', 'number'); + } + + if (typeof address !== 'string') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'address', 'string'); + } this.send(buffer, offset, length, port, address, callback); }; @@ -274,8 +285,9 @@ function sliceBuffer(buffer, offset, length) { if (typeof buffer === 'string') { buffer = Buffer.from(buffer); } else if (!isUint8Array(buffer)) { - throw new TypeError('First argument must be a Buffer, ' + - 'Uint8Array or string'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'buffer', + ['Buffer', 'Uint8Array', 'string']); } offset = offset >>> 0; @@ -323,7 +335,7 @@ function onListenSuccess() { function onListenError(err) { this.removeListener('listening', onListenSuccess); this._queue = undefined; - this.emit('error', new Error('Unable to send data')); + this.emit('error', new errors.Error('ERR_SOCKET_CANNOT_SEND')); } @@ -366,18 +378,21 @@ Socket.prototype.send = function(buffer, if (typeof buffer === 'string') { list = [ Buffer.from(buffer) ]; } else if (!isUint8Array(buffer)) { - throw new TypeError('First argument must be a Buffer, ' + - 'Uint8Array or string'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'buffer', + ['Buffer', 'Uint8Array', 'string']); } else { list = [ buffer ]; } } else if (!(list = fixBufferList(buffer))) { - throw new TypeError('Buffer list arguments must be buffers or strings'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'buffer list arguments', + ['Buffer', 'string']); } port = port >>> 0; if (port === 0 || port > 65535) - throw new RangeError('Port should be > 0 and < 65536'); + throw new errors.RangeError('ERR_SOCKET_BAD_PORT'); // Normalize callback so it's either a function or undefined but not anything // else. @@ -388,8 +403,9 @@ Socket.prototype.send = function(buffer, callback = address; address = undefined; } else if (address && typeof address !== 'string') { - throw new TypeError('Invalid arguments: address must be a nonempty ' + - 'string or falsy'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'address', + ['string', 'falsy']); } this._healthCheck(); @@ -510,7 +526,9 @@ Socket.prototype.setBroadcast = function(arg) { Socket.prototype.setTTL = function(arg) { if (typeof arg !== 'number') { - throw new TypeError('Argument must be a number'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'arg', + 'number'); } var err = this._handle.setTTL(arg); @@ -524,7 +542,9 @@ Socket.prototype.setTTL = function(arg) { Socket.prototype.setMulticastTTL = function(arg) { if (typeof arg !== 'number') { - throw new TypeError('Argument must be a number'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'arg', + 'number'); } var err = this._handle.setMulticastTTL(arg); @@ -551,7 +571,7 @@ Socket.prototype.addMembership = function(multicastAddress, this._healthCheck(); if (!multicastAddress) { - throw new Error('multicast address must be specified'); + throw new errors.TypeError('ERR_MISSING_ARGS', 'multicastAddress'); } var err = this._handle.addMembership(multicastAddress, interfaceAddress); @@ -566,7 +586,7 @@ Socket.prototype.dropMembership = function(multicastAddress, this._healthCheck(); if (!multicastAddress) { - throw new Error('multicast address must be specified'); + throw new errors.TypeError('ERR_MISSING_ARGS', 'multicastAddress'); } var err = this._handle.dropMembership(multicastAddress, interfaceAddress); @@ -577,8 +597,10 @@ Socket.prototype.dropMembership = function(multicastAddress, Socket.prototype._healthCheck = function() { - if (!this._handle) - throw new Error('Not running'); // error message from dgram_legacy.js + if (!this._handle) { + // Error message from dgram_legacy.js. + throw new errors.Error('ERR_SOCKET_DGRAM_NOT_RUNNING'); + } }; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 50d2c20325f..1ca0f59d793 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -142,6 +142,12 @@ E('ERR_UNKNOWN_BUILTIN_MODULE', (id) => `No such built-in module: ${id}`); E('ERR_UNKNOWN_SIGNAL', (signal) => `Unknown signal: ${signal}`); E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type'); E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type'); +E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound'); +E('ERR_SOCKET_BAD_TYPE', + 'Bad socket type specified. Valid types are: udp4, udp6'); +E('ERR_SOCKET_CANNOT_SEND', 'Unable to send data'); +E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536'); +E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running'); // Add new errors from here... function invalidArgType(name, expected, actual) { diff --git a/test/parallel/test-dgram-bind.js b/test/parallel/test-dgram-bind.js index 25fd7408ffd..4e293a8021c 100644 --- a/test/parallel/test-dgram-bind.js +++ b/test/parallel/test-dgram-bind.js @@ -29,7 +29,11 @@ const socket = dgram.createSocket('udp4'); socket.on('listening', common.mustCall(() => { assert.throws(() => { socket.bind(); - }, /^Error: Socket is already bound$/); + }, common.expectsError({ + code: 'ERR_SOCKET_ALREADY_BOUND', + type: Error, + message: /^Socket is already bound$/ + })); socket.close(); })); diff --git a/test/parallel/test-dgram-createSocket-type.js b/test/parallel/test-dgram-createSocket-type.js index 1d269ddeda2..e6b17251f82 100644 --- a/test/parallel/test-dgram-createSocket-type.js +++ b/test/parallel/test-dgram-createSocket-type.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const dgram = require('dgram'); const invalidTypes = [ @@ -24,7 +24,11 @@ const validTypes = [ invalidTypes.forEach((invalidType) => { assert.throws(() => { dgram.createSocket(invalidType); - }, /Bad socket type specified/); + }, common.expectsError({ + code: 'ERR_SOCKET_BAD_TYPE', + type: Error, + message: /^Bad socket type specified\. Valid types are: udp4, udp6$/ + })); }); // Error must not be thrown with valid types diff --git a/test/parallel/test-dgram-implicit-bind-failure.js b/test/parallel/test-dgram-implicit-bind-failure.js index 626af418e53..2944c9aae72 100644 --- a/test/parallel/test-dgram-implicit-bind-failure.js +++ b/test/parallel/test-dgram-implicit-bind-failure.js @@ -31,7 +31,7 @@ socket.on('error', (err) => { return; } - if (/^Error: Unable to send data$/.test(err)) { + if (err.code === 'ERR_SOCKET_CANNOT_SEND') { // On error, the queue should be destroyed and this function should be // the only listener. sendFailures++; diff --git a/test/parallel/test-dgram-membership.js b/test/parallel/test-dgram-membership.js index 1543b9043f7..586db5f3cfe 100644 --- a/test/parallel/test-dgram-membership.js +++ b/test/parallel/test-dgram-membership.js @@ -11,8 +11,13 @@ const setup = dgram.createSocket.bind(dgram, {type: 'udp4', reuseAddr: true}); { const socket = setup(); socket.close(common.mustCall(() => { - assert.throws(() => { socket.addMembership(multicastAddress); }, - /^Error: Not running$/); + assert.throws(() => { + socket.addMembership(multicastAddress); + }, common.expectsError({ + code: 'ERR_SOCKET_DGRAM_NOT_RUNNING', + type: Error, + message: /^Not running$/ + })); })); } @@ -20,24 +25,39 @@ const setup = dgram.createSocket.bind(dgram, {type: 'udp4', reuseAddr: true}); { const socket = setup(); socket.close(common.mustCall(() => { - assert.throws(() => { socket.dropMembership(multicastAddress); }, - /^Error: Not running$/); + assert.throws(() => { + socket.dropMembership(multicastAddress); + }, common.expectsError({ + code: 'ERR_SOCKET_DGRAM_NOT_RUNNING', + type: Error, + message: /^Not running$/ + })); })); } // addMembership() with no argument should throw { const socket = setup(); - assert.throws(() => { socket.addMembership(); }, - /^Error: multicast address must be specified$/); + assert.throws(() => { + socket.addMembership(); + }, common.expectsError({ + code: 'ERR_MISSING_ARGS', + type: TypeError, + message: /^The "multicastAddress" argument must be specified$/ + })); socket.close(); } // dropMembership() with no argument should throw { const socket = setup(); - assert.throws(() => { socket.dropMembership(); }, - /^Error: multicast address must be specified$/); + assert.throws(() => { + socket.dropMembership(); + }, common.expectsError({ + code: 'ERR_MISSING_ARGS', + type: TypeError, + message: /^The "multicastAddress" argument must be specified$/ + })); socket.close(); } diff --git a/test/parallel/test-dgram-multicast-setTTL.js b/test/parallel/test-dgram-multicast-setTTL.js index 37a2d158631..b7d1e01b321 100644 --- a/test/parallel/test-dgram-multicast-setTTL.js +++ b/test/parallel/test-dgram-multicast-setTTL.js @@ -37,7 +37,11 @@ socket.on('listening', common.mustCall(() => { assert.throws(() => { socket.setMulticastTTL('foo'); - }, /^TypeError: Argument must be a number$/); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "arg" argument must be of type number$/ + })); //close the socket socket.close(); diff --git a/test/parallel/test-dgram-send-address-types.js b/test/parallel/test-dgram-send-address-types.js index 6c5bf20e00d..6b26c23a266 100644 --- a/test/parallel/test-dgram-send-address-types.js +++ b/test/parallel/test-dgram-send-address-types.js @@ -10,8 +10,11 @@ const onMessage = common.mustCall((err, bytes) => { assert.strictEqual(bytes, buf.length); }, 6); -const expectedError = - /^TypeError: Invalid arguments: address must be a nonempty string or falsy$/; +const expectedError = { code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: + /^The "address" argument must be one of type string or falsy$/ +}; const client = dgram.createSocket('udp4').bind(0, () => { const port = client.address().port; @@ -37,17 +40,17 @@ const client = dgram.createSocket('udp4').bind(0, () => { // invalid address: object assert.throws(() => { client.send(buf, port, []); - }, expectedError); + }, common.expectsError(expectedError)); // invalid address: nonzero number assert.throws(() => { client.send(buf, port, 1); - }, expectedError); + }, common.expectsError(expectedError)); // invalid address: true assert.throws(() => { client.send(buf, port, true); - }, expectedError); + }, common.expectsError(expectedError)); }); client.unref(); diff --git a/test/parallel/test-dgram-sendto.js b/test/parallel/test-dgram-sendto.js index 350f488f12a..c922dc1039e 100644 --- a/test/parallel/test-dgram-sendto.js +++ b/test/parallel/test-dgram-sendto.js @@ -1,24 +1,48 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const dgram = require('dgram'); const socket = dgram.createSocket('udp4'); -const errorMessage = - /^Error: Send takes "offset" and "length" as args 2 and 3$/; +const errorMessageOffset = + /^The "offset" argument must be of type number$/; assert.throws(() => { socket.sendto(); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: errorMessageOffset +})); assert.throws(() => { socket.sendto('buffer', 1, 'offset', 'port', 'address', 'cb'); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "length" argument must be of type number$/ +})); assert.throws(() => { socket.sendto('buffer', 'offset', 1, 'port', 'address', 'cb'); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: errorMessageOffset +})); assert.throws(() => { socket.sendto('buffer', 1, 1, 10, false, 'cb'); -}, /^Error: udp4 sockets must send to port, address$/); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "address" argument must be of type string$/ +})); + +assert.throws(() => { + socket.sendto('buffer', 1, 1, false, 'address', 'cb'); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "port" argument must be of type number$/ +})); diff --git a/test/parallel/test-dgram-setTTL.js b/test/parallel/test-dgram-setTTL.js index 7da3975ad4c..c061fbc1870 100644 --- a/test/parallel/test-dgram-setTTL.js +++ b/test/parallel/test-dgram-setTTL.js @@ -11,7 +11,11 @@ socket.on('listening', common.mustCall(() => { assert.throws(() => { socket.setTTL('foo'); - }, /^TypeError: Argument must be a number$/); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "arg" argument must be of type number$/ + })); // TTL must be a number from > 0 to < 256 assert.throws(() => { From ba513d140c22d373ec8828e18ee85e93a7954dc8 Mon Sep 17 00:00:00 2001 From: Calvin Metcalf Date: Thu, 4 May 2017 15:33:14 +0200 Subject: [PATCH 24/25] stream: add final method Adds the ability to for write streams to have an _final method which acts similarly to the _flush method that transform streams have but is called before the finish event is emitted and if asynchronous delays the stream from finishing. The `final` option may also be passed in order to set it. PR-URL: https://github.com/nodejs/node/pull/12828 Reviewed-By: James M Snell Reviewed-By: Matteo Collina Reviewed-By: Refael Ackermann --- doc/api/stream.md | 28 ++++- lib/_stream_writable.js | 35 ++++-- ...stream-readable-constructor-set-methods.js | 14 +-- ...tream-transform-constructor-set-methods.js | 24 ++--- .../test-stream-transform-final-sync.js | 100 +++++++++++++++++ test/parallel/test-stream-transform-final.js | 102 ++++++++++++++++++ test/parallel/test-stream-write-final.js | 24 +++++ test/parallel/test-stream2-writable.js | 22 ++++ 8 files changed, 317 insertions(+), 32 deletions(-) create mode 100644 test/parallel/test-stream-transform-final-sync.js create mode 100644 test/parallel/test-stream-transform-final.js create mode 100644 test/parallel/test-stream-write-final.js diff --git a/doc/api/stream.md b/doc/api/stream.md index c2a257cb5a5..d5c25517749 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -1198,7 +1198,8 @@ on the type of stream being created, as detailed in the chart below:

[Writable](#stream_class_stream_writable)

-

[_write][stream-_write], [_writev][stream-_writev]

+

[_write][stream-_write], [_writev][stream-_writev], + [_final][stream-_final]

@@ -1209,7 +1210,8 @@ on the type of stream being created, as detailed in the chart below:

[Duplex](#stream_class_stream_duplex)

-

[_read][stream-_read], [_write][stream-_write], [_writev][stream-_writev]

+

[_read][stream-_read], [_write][stream-_write], [_writev][stream-_writev], + [_final][stream-_final]

@@ -1220,7 +1222,8 @@ on the type of stream being created, as detailed in the chart below:

[Transform](#stream_class_stream_transform)

-

[_transform][stream-_transform], [_flush][stream-_flush]

+

[_transform][stream-_transform], [_flush][stream-_flush], + [_final][stream-_final]

@@ -1279,6 +1282,8 @@ constructor and implement the `writable._write()` method. The [`stream._writev()`][stream-_writev] method. * `destroy` {Function} Implementation for the [`stream._destroy()`][writable-_destroy] method. + * `final` {Function} Implementation for the + [`stream._final()`][stream-_final] method. For example: @@ -1398,6 +1403,22 @@ added: REPLACEME * `callback` {Function} A callback function that takes an optional error argument which is invoked when the writable is destroyed. +#### writable.\_final(callback) + + +* `callback` {Function} Call this function (optionally with an error + argument) when you are done writing any remaining data. + +Note: `_final()` **must not** be called directly. It MAY be implemented +by child classes, and if so, will be called by the internal Writable +class methods only. + +This optional function will be called before the stream closes, delaying the +`finish` event until `callback` is called. This is useful to close resources +or write buffered data before a stream ends. + #### Errors While Writing It is recommended that errors occurring during the processing of the @@ -2115,6 +2136,7 @@ readable buffer so there is nothing for a user to consume. [stream-_transform]: #stream_transform_transform_chunk_encoding_callback [stream-_write]: #stream_writable_write_chunk_encoding_callback_1 [stream-_writev]: #stream_writable_writev_chunks_callback +[stream-_final]: #stream_writable_final_callback [stream-end]: #stream_writable_end_chunk_encoding_callback [stream-pause]: #stream_readable_pause [stream-push]: #stream_readable_push_chunk_encoding diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 4e2a19f12c8..8540d180c75 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -58,6 +58,12 @@ function WritableState(options, stream) { // cast to ints. this.highWaterMark = Math.floor(this.highWaterMark); + // if _final has been called + this.finalCalled = false; + + // if _final has been called + this.finalCalled = false; + // drain event flag. this.needDrain = false; // at the start of calling end() @@ -199,6 +205,9 @@ function Writable(options) { if (typeof options.destroy === 'function') this._destroy = options.destroy; + + if (typeof options.final === 'function') + this._final = options.final; } Stream.call(this); @@ -520,23 +529,37 @@ function needFinish(state) { !state.finished && !state.writing); } - -function prefinish(stream, state) { - if (!state.prefinished) { +function callFinal(stream, state) { + stream._final((err) => { + state.pendingcb--; + if (err) { + stream.emit('error', err); + } state.prefinished = true; stream.emit('prefinish'); + finishMaybe(stream, state); + }); +} +function prefinish(stream, state) { + if (!state.prefinished && !state.finalCalled) { + if (typeof stream._final === 'function') { + state.pendingcb++; + state.finalCalled = true; + process.nextTick(callFinal, stream, state); + } else { + state.prefinished = true; + stream.emit('prefinish'); + } } } function finishMaybe(stream, state) { var need = needFinish(state); if (need) { + prefinish(stream, state); if (state.pendingcb === 0) { - prefinish(stream, state); state.finished = true; stream.emit('finish'); - } else { - prefinish(stream, state); } } return need; diff --git a/test/parallel/test-stream-readable-constructor-set-methods.js b/test/parallel/test-stream-readable-constructor-set-methods.js index e5e3114de45..1b9f0496b99 100644 --- a/test/parallel/test-stream-readable-constructor-set-methods.js +++ b/test/parallel/test-stream-readable-constructor-set-methods.js @@ -1,19 +1,11 @@ 'use strict'; -require('../common'); -const assert = require('assert'); +const common = require('../common'); const Readable = require('stream').Readable; -let _readCalled = false; -function _read(n) { - _readCalled = true; +const _read = common.mustCall(function _read(n) { this.push(null); -} +}); const r = new Readable({ read: _read }); r.resume(); - -process.on('exit', function() { - assert.strictEqual(r._read, _read); - assert(_readCalled); -}); diff --git a/test/parallel/test-stream-transform-constructor-set-methods.js b/test/parallel/test-stream-transform-constructor-set-methods.js index 1423f4de109..3e1325c0fd1 100644 --- a/test/parallel/test-stream-transform-constructor-set-methods.js +++ b/test/parallel/test-stream-transform-constructor-set-methods.js @@ -1,24 +1,25 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const Transform = require('stream').Transform; -let _transformCalled = false; -function _transform(d, e, n) { - _transformCalled = true; +const _transform = common.mustCall(function _transform(d, e, n) { n(); -} +}); -let _flushCalled = false; -function _flush(n) { - _flushCalled = true; +const _final = common.mustCall(function _final(n) { n(); -} +}); + +const _flush = common.mustCall(function _flush(n) { + n(); +}); const t = new Transform({ transform: _transform, - flush: _flush + flush: _flush, + final: _final }); const t2 = new Transform({}); @@ -34,6 +35,5 @@ assert.throws(() => { process.on('exit', () => { assert.strictEqual(t._transform, _transform); assert.strictEqual(t._flush, _flush); - assert.strictEqual(_transformCalled, true); - assert.strictEqual(_flushCalled, true); + assert.strictEqual(t._final, _final); }); diff --git a/test/parallel/test-stream-transform-final-sync.js b/test/parallel/test-stream-transform-final-sync.js new file mode 100644 index 00000000000..de3f0904885 --- /dev/null +++ b/test/parallel/test-stream-transform-final-sync.js @@ -0,0 +1,100 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const stream = require('stream'); +let state = 0; + +/* +What you do +var stream = new tream.Transform({ + transform: function transformCallback(chunk, _, next) { + // part 1 + this.push(chunk); + //part 2 + next(); + }, + final: function endCallback(done) { + // part 1 + process.nextTick(function () { + // part 2 + done(); + }); + }, + flush: function flushCallback(done) { + // part 1 + process.nextTick(function () { + // part 2 + done(); + }); + } +}); +t.on('data', dataListener); +t.on('end', endListener); +t.on('finish', finishListener); +t.write(1); +t.write(4); +t.end(7, endMethodCallback); + +The order things are called + +1. transformCallback part 1 +2. dataListener +3. transformCallback part 2 +4. transformCallback part 1 +5. dataListener +6. transformCallback part 2 +7. transformCallback part 1 +8. dataListener +9. transformCallback part 2 +10. finalCallback part 1 +11. finalCallback part 2 +12. flushCallback part 1 +13. finishListener +14. endMethodCallback +15. flushCallback part 2 +16. endListener +*/ + +const t = new stream.Transform({ + objectMode: true, + transform: common.mustCall(function(chunk, _, next) { + assert.strictEqual(++state, chunk, 'transformCallback part 1'); + this.push(state); + assert.strictEqual(++state, chunk + 2, 'transformCallback part 2'); + process.nextTick(next); + }, 3), + final: common.mustCall(function(done) { + state++; + assert.strictEqual(state, 10, 'finalCallback part 1'); + state++; + assert.strictEqual(state, 11, 'finalCallback part 2'); + done(); + }, 1), + flush: common.mustCall(function(done) { + state++; + assert.strictEqual(state, 12, 'flushCallback part 1'); + process.nextTick(function() { + state++; + assert.strictEqual(state, 15, 'flushCallback part 2'); + done(); + }); + }, 1) +}); +t.on('finish', common.mustCall(function() { + state++; + assert.strictEqual(state, 13, 'finishListener'); +}, 1)); +t.on('end', common.mustCall(function() { + state++; + assert.strictEqual(state, 16, 'end event'); +}, 1)); +t.on('data', common.mustCall(function(d) { + assert.strictEqual(++state, d + 1, 'dataListener'); +}, 3)); +t.write(1); +t.write(4); +t.end(7, common.mustCall(function() { + state++; + assert.strictEqual(state, 14, 'endMethodCallback'); +}, 1)); diff --git a/test/parallel/test-stream-transform-final.js b/test/parallel/test-stream-transform-final.js new file mode 100644 index 00000000000..56566152e69 --- /dev/null +++ b/test/parallel/test-stream-transform-final.js @@ -0,0 +1,102 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const stream = require('stream'); +let state = 0; + +/* +What you do +var stream = new tream.Transform({ + transform: function transformCallback(chunk, _, next) { + // part 1 + this.push(chunk); + //part 2 + next(); + }, + final: function endCallback(done) { + // part 1 + process.nextTick(function () { + // part 2 + done(); + }); + }, + flush: function flushCallback(done) { + // part 1 + process.nextTick(function () { + // part 2 + done(); + }); + } +}); +t.on('data', dataListener); +t.on('end', endListener); +t.on('finish', finishListener); +t.write(1); +t.write(4); +t.end(7, endMethodCallback); + +The order things are called + +1. transformCallback part 1 +2. dataListener +3. transformCallback part 2 +4. transformCallback part 1 +5. dataListener +6. transformCallback part 2 +7. transformCallback part 1 +8. dataListener +9. transformCallback part 2 +10. finalCallback part 1 +11. finalCallback part 2 +12. flushCallback part 1 +13. finishListener +14. endMethodCallback +15. flushCallback part 2 +16. endListener +*/ + +const t = new stream.Transform({ + objectMode: true, + transform: common.mustCall(function(chunk, _, next) { + assert.strictEqual(++state, chunk, 'transformCallback part 1'); + this.push(state); + assert.strictEqual(++state, chunk + 2, 'transformCallback part 2'); + process.nextTick(next); + }, 3), + final: common.mustCall(function(done) { + state++; + assert.strictEqual(state, 10, 'finalCallback part 1'); + setTimeout(function() { + state++; + assert.strictEqual(state, 11, 'finalCallback part 2'); + done(); + }, 100); + }, 1), + flush: common.mustCall(function(done) { + state++; + assert.strictEqual(state, 12, 'flushCallback part 1'); + process.nextTick(function() { + state++; + assert.strictEqual(state, 15, 'flushCallback part 2'); + done(); + }); + }, 1) +}); +t.on('finish', common.mustCall(function() { + state++; + assert.strictEqual(state, 13, 'finishListener'); +}, 1)); +t.on('end', common.mustCall(function() { + state++; + assert.strictEqual(state, 16, 'end event'); +}, 1)); +t.on('data', common.mustCall(function(d) { + assert.strictEqual(++state, d + 1, 'dataListener'); +}, 3)); +t.write(1); +t.write(4); +t.end(7, common.mustCall(function() { + state++; + assert.strictEqual(state, 14, 'endMethodCallback'); +}, 1)); diff --git a/test/parallel/test-stream-write-final.js b/test/parallel/test-stream-write-final.js new file mode 100644 index 00000000000..56537bd7fae --- /dev/null +++ b/test/parallel/test-stream-write-final.js @@ -0,0 +1,24 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const stream = require('stream'); +let shutdown = false; + +const w = new stream.Writable({ + final: common.mustCall(function(cb) { + assert.strictEqual(this, w); + setTimeout(function() { + shutdown = true; + cb(); + }, 100); + }), + write: function(chunk, e, cb) { + process.nextTick(cb); + } +}); +w.on('finish', common.mustCall(function() { + assert(shutdown); +})); +w.write(Buffer.allocUnsafe(1)); +w.end(Buffer.allocUnsafe(0)); diff --git a/test/parallel/test-stream2-writable.js b/test/parallel/test-stream2-writable.js index c046194d6bf..8e1ea9a6034 100644 --- a/test/parallel/test-stream2-writable.js +++ b/test/parallel/test-stream2-writable.js @@ -408,3 +408,25 @@ test('finish is emitted if last chunk is empty', function(t) { w.write(Buffer.allocUnsafe(1)); w.end(Buffer.alloc(0)); }); + +test('finish is emitted after shutdown', function(t) { + const w = new W(); + let shutdown = false; + + w._final = common.mustCall(function(cb) { + assert.strictEqual(this, w); + setTimeout(function() { + shutdown = true; + cb(); + }, 100); + }); + w._write = function(chunk, e, cb) { + process.nextTick(cb); + }; + w.on('finish', common.mustCall(function() { + assert(shutdown); + t.end(); + })); + w.write(Buffer.allocUnsafe(1)); + w.end(Buffer.allocUnsafe(0)); +}); From 260cd411d4ec3ff0305852ea9dbdd561d015e2c0 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 24 May 2017 12:49:07 +0200 Subject: [PATCH 25/25] src: correct endif comment SRC_NODE_API_H__ Really minor but I could not find an open PR for anything n-api where this could be changed, so creating this so that it is not forgotten. PR-URL: https://github.com/nodejs/node/pull/13190 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: James M Snell --- src/node_api.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_api.h b/src/node_api.h index 29c5c513f72..5a95c030a35 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -480,4 +480,4 @@ NAPI_EXTERN napi_status napi_cancel_async_work(napi_env env, EXTERN_C_END -#endif // SRC_NODE_API_H__ +#endif // SRC_NODE_API_H_