Merge nodejs/master

doc/api/stream.md

@@ -2177,6 +2177,7 @@ readable buffer so there is nothing for a user to consume.
 [fs read streams]: fs.html#fs_class_fs_readstream
 [fs write streams]: fs.html#fs_class_fs_writestream
 [http-incoming-message]: http.html#http_class_http_incomingmessage
+[zlib]: zlib.html
 [stream-_flush]: #stream_transform_flush_callback
 [stream-_read]: #stream_readable_read_size_1
 [stream-_transform]: #stream_transform_transform_chunk_encoding_callback

doc/guides/using-internal-errors.md

@@ -27,7 +27,7 @@ are intended to replace existing `Error` objects within the Node.js source.
 For instance, an existing `Error` such as:
 
 ```js
-  const err = new TypeError('Expected string received ' + type);
+const err = new TypeError(`Expected string received ${type}`);
 ```
 
 Can be replaced by first adding a new error key into the `internal/errors.js`
@@ -40,9 +40,9 @@ E('FOO', 'Expected string received %s');
 Then replacing the existing `new TypeError` in the code:
 
 ```js
-  const errors = require('internal/errors');
-  // ...
-  const err = new errors.TypeError('FOO', type);
+const errors = require('internal/errors');
+// ...
+const err = new errors.TypeError('FOO', type);
 ```
 
 ## Adding new errors
@@ -90,7 +90,7 @@ The specific error message for the `myError` instance will depend on the
 associated value of `KEY` (see "Adding new errors").
 
 The `myError` object will have a `code` property equal to the `key` and a
-`name` property equal to `Error[${key}]`.
+`name` property equal to `` `Error [${key}]` ``.
 
 ### Class: errors.TypeError(key[, args...])
 
@@ -110,7 +110,7 @@ The specific error message for the `myError` instance will depend on the
 associated value of `KEY` (see "Adding new errors").
 
 The `myError` object will have a `code` property equal to the `key` and a
-`name` property equal to `TypeError[${key}]`.
+`name` property equal to `` `TypeError [${key}]` ``.
 
 ### Class: errors.RangeError(key[, args...])
 
@@ -130,7 +130,7 @@ The specific error message for the `myError` instance will depend on the
 associated value of `KEY` (see "Adding new errors").
 
 The `myError` object will have a `code` property equal to the `key` and a
-`name` property equal to `RangeError[${key}]`.
+`name` property equal to `` `RangeError [${key}]` ``.
 
 ### Method: errors.message(key, args)
 

doc/releases.md

@@ -253,7 +253,7 @@ PR-URL: <full URL to your release proposal PR>
 
 This sets up the branch so that nightly builds are produced with the next version number _and_ a pre-release tag.
 
-Merge your release branch into the stable branch that you are releasing from (not master).
+Merge your release proposal branch into the stable branch that you are releasing from (e.g. `v8.x`), and rebase the corresponding staging branch (`v8.x-staging`) on top of that.
 
 Cherry-pick the release commit to `master`. After cherry-picking, edit `src/node_version.h` to ensure the version macros contain whatever values were previously on `master`. `NODE_VERSION_IS_RELEASE` should be `0`.
 

lib/_stream_readable.js

@@ -215,8 +215,8 @@ function readableAddChunk(stream, chunk, encoding, addToFront, skipChunkCheck) {
       stream.emit('error', er);
     } else if (state.objectMode || chunk && chunk.length > 0) {
       if (typeof chunk !== 'string' &&
-          Object.getPrototypeOf(chunk) !== Buffer.prototype &&
-          !state.objectMode) {
+          !state.objectMode &&
+          Object.getPrototypeOf(chunk) !== Buffer.prototype) {
         chunk = Stream._uint8ArrayToBuffer(chunk);
       }
 

lib/internal/errors.js

@@ -192,7 +192,8 @@ E('ERR_V8BREAKITERATOR', 'full ICU data not installed. ' +
 function invalidArgType(name, expected, actual) {
   const assert = lazyAssert();
   assert(name, 'name is required');
-  var msg = `The "${name}" argument must be ${oneOf(expected, 'type')}`;
+  const type = name.includes('.') ? 'property' : 'argument';
+  var msg = `The "${name}" ${type} must be ${oneOf(expected, 'type')}`;
   if (arguments.length >= 3) {
     msg += `. Received type ${actual !== null ? typeof actual : 'null'}`;
   }

src/node_api.cc

@@ -17,6 +17,7 @@
 #include <vector>
 #include "uv.h"
 #include "node_api.h"
+#include "node_internals.h"
 
 static
 napi_status napi_set_last_error(napi_env env, napi_status error_code,
@@ -154,14 +155,20 @@ class HandleScopeWrapper {
 // across different versions.
 class EscapableHandleScopeWrapper {
  public:
-  explicit EscapableHandleScopeWrapper(v8::Isolate* isolate) : scope(isolate) {}
+  explicit EscapableHandleScopeWrapper(v8::Isolate* isolate)
+      : scope(isolate), escape_called_(false) {}
+  bool escape_called() const {
+    return escape_called_;
+  }
   template <typename T>
   v8::Local<T> Escape(v8::Local<T> handle) {
+    escape_called_ = true;
     return scope.Escape(handle);
   }
 
  private:
   v8::EscapableHandleScope scope;
+  bool escape_called_;
 };
 
 napi_handle_scope JsHandleScopeFromV8HandleScope(HandleScopeWrapper* s) {
@@ -716,7 +723,8 @@ const char* error_messages[] = {nullptr,
                                 "An array was expected",
                                 "Unknown failure",
                                 "An exception is pending",
-                                "The async work item was cancelled"};
+                                "The async work item was cancelled",
+                                "napi_escape_handle already called on scope"};
 
 static napi_status napi_clear_last_error(napi_env env) {
   CHECK_ENV(env);
@@ -744,10 +752,14 @@ napi_status napi_get_last_error_info(napi_env env,
   CHECK_ENV(env);
   CHECK_ARG(env, result);
 
+  // you must update this assert to reference the last message
+  // in the napi_status enum each time a new error message is added.
+  // We don't have a napi_status_last as this would result in an ABI
+  // change each time a message was added.
   static_assert(
-      (sizeof (error_messages) / sizeof (*error_messages)) == napi_status_last,
+      node::arraysize(error_messages) == napi_escape_called_twice + 1,
       "Count of error messages must match count of error values");
-  assert(env->last_error.error_code < napi_status_last);
+  assert(env->last_error.error_code <= napi_escape_called_twice);
 
   // Wait until someone requests the last error information to fetch the error
   // message string
@@ -2209,9 +2221,12 @@ napi_status napi_escape_handle(napi_env env,
 
   v8impl::EscapableHandleScopeWrapper* s =
       v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope);
-  *result = v8impl::JsValueFromV8LocalValue(
-      s->Escape(v8impl::V8LocalValueFromJsValue(escapee)));
-  return napi_clear_last_error(env);
+  if (!s->escape_called()) {
+    *result = v8impl::JsValueFromV8LocalValue(
+        s->Escape(v8impl::V8LocalValueFromJsValue(escapee)));
+    return napi_clear_last_error(env);
+  }
+  return napi_set_last_error(env, napi_escape_called_twice);
 }
 
 napi_status napi_new_instance(napi_env env,

src/node_api_jsrt.cc

@@ -348,16 +348,17 @@ const char* error_messages[] = {
   "Unknown failure",
   "An exception is pending",
   "The async work item was cancelled",
+  "napi_escape_handle already called on scope"
 };
 
 napi_status napi_get_last_error_info(napi_env env,
                                      const napi_extended_error_info** result) {
   CHECK_ARG(result);
 
   static_assert(
-    sizeof(error_messages) / sizeof(*error_messages) == napi_status_last,
+    node::arraysize(error_messages) == napi_escape_called_twice + 1,
     "Count of error messages must match count of error values");
-  assert(static_last_error.error_code < napi_status_last);
+  assert(static_last_error.error_code <= napi_escape_called_twice);
 
   // Wait until someone requests the last error information to fetch the error
   // message string

src/node_api_types.h

@@ -67,7 +67,7 @@ typedef enum {
   napi_generic_failure,
   napi_pending_exception,
   napi_cancelled,
-  napi_status_last
+  napi_escape_called_twice
 } napi_status;
 
 typedef napi_value (*napi_callback)(napi_env env,

test/addons-napi/test_handle_scope/test.js

@@ -10,6 +10,14 @@ testHandleScope.NewScope();
 
 assert.ok(testHandleScope.NewScopeEscape() instanceof Object);
 
+if (!common.isChakraEngine) {
+  assert.throws(
+    () => {
+      testHandleScope.NewScopeEscapeTwice();
+    },
+    Error);
+}
+
 assert.throws(
   () => {
     testHandleScope.NewScopeWithException(() => { throw new RangeError(); });

test/addons-napi/test_handle_scope/test_handle_scope.c

@@ -29,6 +29,19 @@ napi_value NewScopeEscape(napi_env env, napi_callback_info info) {
   return escapee;
 }
 
+napi_value NewScopeEscapeTwice(napi_env env, napi_callback_info info) {
+  napi_escapable_handle_scope scope;
+  napi_value output = NULL;
+  napi_value escapee = NULL;
+
+  NAPI_CALL(env, napi_open_escapable_handle_scope(env, &scope));
+  NAPI_CALL(env, napi_create_object(env, &output));
+  NAPI_CALL(env, napi_escape_handle(env, scope, output, &escapee));
+  NAPI_CALL(env, napi_escape_handle(env, scope, output, &escapee));
+  NAPI_CALL(env, napi_close_escapable_handle_scope(env, scope));
+  return escapee;
+}
+
 napi_value NewScopeWithException(napi_env env, napi_callback_info info) {
   napi_handle_scope scope;
   size_t argc;
@@ -57,6 +70,7 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
   napi_property_descriptor properties[] = {
     DECLARE_NAPI_PROPERTY("NewScope", NewScope),
     DECLARE_NAPI_PROPERTY("NewScopeEscape", NewScopeEscape),
+    DECLARE_NAPI_PROPERTY("NewScopeEscapeTwice", NewScopeEscapeTwice),
     DECLARE_NAPI_PROPERTY("NewScopeWithException", NewScopeWithException),
   };
 

test/cctest/test_inspector_socket_server.cc

@@ -172,12 +172,14 @@ class SocketWrapper {
     contents_.clear();
     uv_tcp_init(loop_, &socket_);
     union {sockaddr generic; sockaddr_in v4; sockaddr_in6 v6;} addr;
+    int err = 0;
     if (v6) {
-      uv_ip6_addr(host.c_str(), port, &addr.v6);
+      err = uv_ip6_addr(host.c_str(), port, &addr.v6);
     } else {
-      uv_ip4_addr(host.c_str(), port, &addr.v4);
+      err = uv_ip4_addr(host.c_str(), port, &addr.v4);
     }
-    int err = uv_tcp_connect(&connect_, &socket_, &addr.generic, Connected_);
+    ASSERT_EQ(0, err);
+    err = uv_tcp_connect(&connect_, &socket_, &addr.generic, Connected_);
     ASSERT_EQ(0, err);
     SPIN_WHILE(!connected_)
     uv_read_start(reinterpret_cast<uv_stream_t*>(&socket_), AllocCallback,
@@ -618,7 +620,7 @@ TEST_F(InspectorSocketServerTest, BindsToIpV6) {
   ASSERT_TRUE(server->Start());
 
   SocketWrapper socket1(&loop);
-  socket1.Connect("[::]", server.port(), true);
+  socket1.Connect("::", server.port(), true);
   socket1.Write(WsHandshakeRequest(MAIN_TARGET_ID));
   socket1.Expect(WS_HANDSHAKE_RESPONSE);
   server->Stop(ServerHolder::CloseCallback);

test/parallel/test-process-cpuUsage.js

@@ -34,13 +34,13 @@ for (let i = 0; i < 10; i++) {
 const invalidUserArgument = common.expectsError({
   code: 'ERR_INVALID_ARG_TYPE',
   type: TypeError,
-  message: 'The "preValue.user" argument must be of type Number'
+  message: 'The "preValue.user" property must be of type Number'
 });
 
 const invalidSystemArgument = common.expectsError({
   code: 'ERR_INVALID_ARG_TYPE',
   type: TypeError,
-  message: 'The "preValue.system" argument must be of type Number'
+  message: 'The "preValue.system" property must be of type Number'
 });
 
 

test/parallel/test-stream-objectmode-undefined.js

@@ -0,0 +1,44 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { Readable, Writable, Transform } = require('stream');
+
+{
+  const stream = new Readable({
+    objectMode: true,
+    read: common.mustCall(() => {
+      stream.push(undefined);
+      stream.push(null);
+    })
+  });
+
+  stream.on('data', common.mustCall((chunk) => {
+    assert.strictEqual(chunk, undefined);
+  }));
+}
+
+{
+  const stream = new Writable({
+    objectMode: true,
+    write: common.mustCall((chunk) => {
+      assert.strictEqual(chunk, undefined);
+    })
+  });
+
+  stream.write(undefined);
+}
+
+{
+  const stream = new Transform({
+    objectMode: true,
+    transform: common.mustCall((chunk) => {
+      stream.push(chunk);
+    })
+  });
+
+  stream.on('data', common.mustCall((chunk) => {
+    assert.strictEqual(chunk, undefined);
+  }));
+
+  stream.write(undefined);
+}

test/parallel/test-util-inherits.js

@@ -85,7 +85,7 @@ assert.throws(function() {
 }, common.expectsError({
     code: 'ERR_INVALID_ARG_TYPE',
     type: TypeError,
-    message: 'The "superCtor.prototype" argument must be of type function'
+    message: 'The "superCtor.prototype" property must be of type function'
 })
 );
 assert.throws(function() {