diff --git a/GOVERNANCE.md b/GOVERNANCE.md index d7d60671ac1..27c0372ac3e 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -9,9 +9,21 @@ The [nodejs/node](https://github.com/nodejs/node) GitHub repository is maintained by Collaborators who are added by the CTC on an ongoing basis. Individuals identified by the CTC as making significant and valuable -contributions are made Collaborators and given commit access to the project. If -you make a significant contribution and are not considered for commit access, -log an issue or contact a CTC member directly. +contributions across any Node.js repository may be made Collaborators and given +commit access to the project. Activities taken into consideration include (but +are not limited to) the quality of: + +* code commits and pull requests +* documentation commits and pull requests +* comments on issues and pull requests +* contributions to the Node.js website +* assistance provided to end users and novice contributors +* participation in Working Groups +* other participation in the wider Node.js community + +If individuals making valuable contributions do not believe they have been +considered for commit access, they may log an issue or contact a CTC member +directly. Modifications of the contents of the nodejs/node repository are made on a collaborative basis. Anybody with a GitHub account may propose a diff --git a/deps/v8/src/trap-handler/handler-shared.cc b/deps/v8/src/trap-handler/handler-shared.cc index 7b399f5eeac..d1b549a1701 100644 --- a/deps/v8/src/trap-handler/handler-shared.cc +++ b/deps/v8/src/trap-handler/handler-shared.cc @@ -23,7 +23,14 @@ namespace v8 { namespace internal { namespace trap_handler { -THREAD_LOCAL bool g_thread_in_wasm_code = false; +// We declare this as int rather than bool as a workaround for a glibc bug, in +// which the dynamic loader cannot handle executables whose TLS area is only +// 1 byte in size; see https://sourceware.org/bugzilla/show_bug.cgi?id=14898. +THREAD_LOCAL int g_thread_in_wasm_code = false; + +static_assert(sizeof(g_thread_in_wasm_code) > 1, + "sizeof(thread_local_var) must be > 1, see " + "https://sourceware.org/bugzilla/show_bug.cgi?id=14898"); size_t gNumCodeObjects = 0; CodeProtectionInfoListEntry* gCodeObjects = nullptr; diff --git a/deps/v8/src/trap-handler/trap-handler.h b/deps/v8/src/trap-handler/trap-handler.h index 5494c5fdb31..ed9459918b7 100644 --- a/deps/v8/src/trap-handler/trap-handler.h +++ b/deps/v8/src/trap-handler/trap-handler.h @@ -65,7 +65,7 @@ inline bool UseTrapHandler() { return FLAG_wasm_trap_handler && V8_TRAP_HANDLER_SUPPORTED; } -extern THREAD_LOCAL bool g_thread_in_wasm_code; +extern THREAD_LOCAL int g_thread_in_wasm_code; inline bool IsThreadInWasm() { return g_thread_in_wasm_code; } diff --git a/doc/api/cli.md b/doc/api/cli.md index a8f91fa7827..beceebec779 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -352,15 +352,15 @@ added: v6.11.0 --> Use OpenSSL's default CA store or use bundled Mozilla CA store as supplied by -current NodeJS version. The default store is selectable at build-time. +current Node.js version. The default store is selectable at build-time. Using OpenSSL store allows for external modifications of the store. For most Linux and BSD distributions, this store is maintained by the distribution maintainers and system administrators. OpenSSL CA store location is dependent on configuration of the OpenSSL library but this can be altered at runtime using -environmental variables. +environment variables. -The bundled CA store, as supplied by NodeJS, is a snapshot of Mozilla CA store +The bundled CA store, as supplied by Node.js, is a snapshot of Mozilla CA store that is fixed at release time. It is identical on all supported platforms. See `SSL_CERT_DIR` and `SSL_CERT_FILE`. @@ -568,6 +568,30 @@ appended to if it does. If an error occurs while attempting to write the warning to the file, the warning will be written to stderr instead. This is equivalent to using the `--redirect-warnings=file` command-line flag. +### `UV_THREADPOOL_SIZE=size` + +Set the number of threads used in libuv's threadpool to `size` threads. + +Asynchronous system APIs are used by Node.js whenever possible, but where they +do not exist, libuv's threadpool is used to create asynchronous node APIs based +on synchronous system APIs. Node.js APIs that use the threadpool are: + +- all `fs` APIs, other than the file watcher APIs and those that are explicitly + synchronous +- `crypto.pbkdf2()` +- `crypto.randomBytes()`, unless it is used without a callback +- `crypto.randomFill()` +- `dns.lookup()` +- all `zlib` APIs, other than those that are explicitly synchronous + +Because libuv's threadpool has a fixed size, it means that if for whatever +reason any of these APIs takes a long time, other (seemingly unrelated) APIs +that run in libuv's threadpool will experience degraded performance. In order to +mitigate this issue, one potential solution is to increase the size of libuv's +threadpool by setting the `'UV_THREADPOOL_SIZE'` environment variable to a value +greater than `4` (its current default value). For more information, see the +[libuv threadpool documentation][]. + [`--openssl-config`]: #cli_openssl_config_file [Buffer]: buffer.html#buffer_buffer [Chrome Debugging Protocol]: https://chromedevtools.github.io/debugger-protocol-viewer @@ -575,3 +599,4 @@ equivalent to using the `--redirect-warnings=file` command-line flag. [SlowBuffer]: buffer.html#buffer_class_slowbuffer [debugger]: debugger.html [emit_warning]: process.html#process_process_emitwarning_warning_type_code_ctor +[libuv threadpool documentation]: http://docs.libuv.org/en/latest/threadpool.html diff --git a/doc/api/crypto.md b/doc/api/crypto.md index 56c196ed23e..05e02450867 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -1559,6 +1559,10 @@ crypto.pbkdf2('secret', 'salt', 100000, 512, 'sha512', (err, derivedKey) => { An array of supported digest functions can be retrieved using [`crypto.getHashes()`][]. +Note that this API uses libuv's threadpool, which can have surprising and +negative performance implications for some applications, see the +[`UV_THREADPOOL_SIZE`][] documentation for more information. + ### crypto.pbkdf2Sync(password, salt, iterations, keylen, digest) +## Promises + +N-API provides facilities for creating `Promise` objects as described in +[Section 25.4][] of the ECMA specification. It implements promises as a pair of +objects. When a promise is created by `napi_create_promise()`, a "deferred" +object is created and returned alongside the `Promise`. The deferred object is +bound to the created `Promise` and is the only means to resolve or reject the +`Promise` using `napi_resolve_deferred()` or `napi_reject_deferred()`. The +deferred object that is created by `napi_create_promise()` is freed by +`napi_resolve_deferred()` or `napi_reject_deferred()`. The `Promise` object may +be returned to JavaScript where it can be used in the usual fashion. + +For example, to create a promise and pass it to an asynchronous worker: +```c +napi_deferred deferred; +napi_value promise; +napi_status status; + +// Create the promise. +status = napi_create_promise(env, &deferred, &promise); +if (status != napi_ok) return NULL; + +// Pass the deferred to a function that performs an asynchronous action. +do_something_asynchronous(deferred); + +// Return the promise to JS +return promise; +``` + +The above function `do_something_asynchronous()` would perform its asynchronous +action and then it would resolve or reject the deferred, thereby concluding the +promise and freeing the deferred: +```c +napi_deferred deferred; +napi_value undefined; +napi_status status; + +// Create a value with which to conclude the deferred. +status = napi_get_undefined(env, &undefined); +if (status != napi_ok) return NULL; + +// Resolve or reject the promise associated with the deferred depending on +// whether the asynchronous action succeeded. +if (asynchronous_action_succeeded) { + status = napi_resolve_deferred(env, deferred, undefined); +} else { + status = napi_reject_deferred(env, deferred, undefined); +} +if (status != napi_ok) return NULL; + +// At this point the deferred has been freed, so we should assign NULL to it. +deferred = NULL; +``` + +### napi_create_promise + +```C +NAPI_EXTERN napi_status napi_create_promise(napi_env env, + napi_deferred* deferred, + napi_value* promise); +``` + +- `[in] env`: The environment that the API is invoked under. +- `[out] deferred`: A newly created deferred object which can later be passed to +`napi_resolve_deferred()` or `napi_reject_deferred()` to resolve resp. reject +the associated promise. +- `[out] promise`: The JavaScript promise associated with the deferred object. + +Returns `napi_ok` if the API succeeded. + +This API creates a deferred object and a JavaScript promise. + +### napi_resolve_deferred + +```C +NAPI_EXTERN napi_status napi_resolve_deferred(napi_env env, + napi_deferred deferred, + napi_value resolution); +``` + +- `[in] env`: The environment that the API is invoked under. +- `[in] deferred`: The deferred object whose associated promise to resolve. +- `[in] resolution`: The value with which to resolve the promise. + +This API resolves a JavaScript promise by way of the deferred object +with which it is associated. Thus, it can only be used to resolve JavaScript +promises for which the corresponding deferred object is available. This +effectively means that the promise must have been created using +`napi_create_promise()` and the deferred object returned from that call must +have been retained in order to be passed to this API. + +The deferred object is freed upon successful completion. + +### napi_reject_deferred + +```C +NAPI_EXTERN napi_status napi_reject_deferred(napi_env env, + napi_deferred deferred, + napi_value rejection); +``` + +- `[in] env`: The environment that the API is invoked under. +- `[in] deferred`: The deferred object whose associated promise to resolve. +- `[in] rejection`: The value with which to reject the promise. + +This API rejects a JavaScript promise by way of the deferred object +with which it is associated. Thus, it can only be used to reject JavaScript +promises for which the corresponding deferred object is available. This +effectively means that the promise must have been created using +`napi_create_promise()` and the deferred object returned from that call must +have been retained in order to be passed to this API. + +The deferred object is freed upon successful completion. + +### napi_is_promise + +```C +NAPI_EXTERN napi_status napi_is_promise(napi_env env, + napi_value promise, + bool* is_promise); +``` + +- `[in] env`: The environment that the API is invoked under. +- `[in] promise`: The promise to examine +- `[out] is_promise`: Flag indicating whether `promise` is a native promise +object - that is, a promise object created by the underlying engine. + +[Promises]: #n_api_promises [Asynchronous Operations]: #n_api_asynchronous_operations [Basic N-API Data Types]: #n_api_basic_n_api_data_types [ECMAScript Language Specification]: https://tc39.github.io/ecma262/ @@ -3406,6 +3543,7 @@ support it: [Section 9.1.6]: https://tc39.github.io/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-defineownproperty-p-desc [Section 12.5.5]: https://tc39.github.io/ecma262/#sec-typeof-operator [Section 24.3]: https://tc39.github.io/ecma262/#sec-dataview-objects +[Section 25.4]: https://tc39.github.io/ecma262/#sec-promise-objects [Working with JavaScript Functions]: #n_api_working_with_javascript_functions [Working with JavaScript Properties]: #n_api_working_with_javascript_properties [Working with JavaScript Values]: #n_api_working_with_javascript_values diff --git a/doc/api/repl.md b/doc/api/repl.md index 4ed95f61116..618744f6e2d 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -523,7 +523,7 @@ by the `NODE_REPL_HISTORY` variable, as documented in the ### Using the Node.js REPL with advanced line-editors -For advanced line-editors, start Node.js with the environmental variable +For advanced line-editors, start Node.js with the environment variable `NODE_NO_READLINE=1`. This will start the main and debugger REPL in canonical terminal settings, which will allow use with `rlwrap`. diff --git a/doc/api/zlib.md b/doc/api/zlib.md index 8b5653a72ce..0f589799491 100644 --- a/doc/api/zlib.md +++ b/doc/api/zlib.md @@ -43,6 +43,13 @@ zlib.unzip(buffer, (err, buffer) => { }); ``` +## Threadpool Usage + +Note that all zlib APIs except those that are explicitly synchronous use libuv's +threadpool, which can have surprising and negative performance implications for +some applications, see the [`UV_THREADPOOL_SIZE`][] documentation for more +information. + ## Compressing HTTP requests and responses The `zlib` module can be used to implement support for the `gzip` and `deflate` diff --git a/doc/node.1 b/doc/node.1 index cf79ce33f92..36c44d6b2cf 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -223,15 +223,15 @@ used to enable FIPS-compliant crypto if Node.js is built with .TP .BR \-\-use\-openssl\-ca,\-\-use\-bundled\-ca Use OpenSSL's default CA store or use bundled Mozilla CA store as supplied by -current NodeJS version. The default store is selectable at build-time. +current Node.js version. The default store is selectable at build-time. Using OpenSSL store allows for external modifications of the store. For most Linux and BSD distributions, this store is maintained by the distribution maintainers and system administrators. OpenSSL CA store location is dependent on configuration of the OpenSSL library but this can be altered at runtime using -environmental variables. +environment variables. -The bundled CA store, as supplied by NodeJS, is a snapshot of Mozilla CA store +The bundled CA store, as supplied by Node.js, is a snapshot of Mozilla CA store that is fixed at release time. It is identical on all supported platforms. See \fBSSL_CERT_DIR\fR and \fBSSL_CERT_FILE\fR. diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 6e0eaf45b54..7de77958d56 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -179,6 +179,8 @@ if (typeof Symbol === 'function' && Symbol.hasInstance) { value: function(object) { if (realHasInstance.call(this, object)) return true; + if (this !== Writable) + return false; return object && object._writableState instanceof WritableState; } diff --git a/lib/buffer.js b/lib/buffer.js index 003c352bc72..6c83aafee03 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -234,7 +234,8 @@ Buffer.from = function from(value, encodingOrOffset, length) { throw new errors.TypeError( 'ERR_INVALID_ARG_TYPE', 'first argument', - ['string', 'buffer', 'arrayBuffer', 'array', 'array-like object'] + ['string', 'buffer', 'arrayBuffer', 'array', 'array-like object'], + value ); }; @@ -519,7 +520,8 @@ function byteLength(string, encoding) { } throw new errors.TypeError( - 'ERR_INVALID_ARG_TYPE', 'string', ['string', 'buffer', 'arrayBuffer'] + 'ERR_INVALID_ARG_TYPE', 'string', + ['string', 'buffer', 'arrayBuffer'], string ); } @@ -680,7 +682,8 @@ Buffer.prototype.toString = function toString(encoding, start, end) { Buffer.prototype.equals = function equals(b) { if (!isUint8Array(b)) { throw new errors.TypeError( - 'ERR_INVALID_ARG_TYPE', 'otherBuffer', ['buffer', 'uint8Array'] + 'ERR_INVALID_ARG_TYPE', 'otherBuffer', + ['buffer', 'uint8Array'], b ); } if (this === b) @@ -708,7 +711,8 @@ Buffer.prototype.compare = function compare(target, thisEnd) { if (!isUint8Array(target)) { throw new errors.TypeError( - 'ERR_INVALID_ARG_TYPE', 'target', ['buffer', 'uint8Array'] + 'ERR_INVALID_ARG_TYPE', 'target', + ['buffer', 'uint8Array'], target ); } if (arguments.length === 1) @@ -790,7 +794,8 @@ function bidirectionalIndexOf(buffer, val, byteOffset, encoding, dir) { } throw new errors.TypeError( - 'ERR_INVALID_ARG_TYPE', 'val', ['string', 'buffer', 'uint8Array'] + 'ERR_INVALID_ARG_TYPE', 'value', + ['string', 'buffer', 'uint8Array'], val ); } @@ -859,7 +864,8 @@ Buffer.prototype.fill = function fill(val, start, end, encoding) { } if (encoding !== undefined && typeof encoding !== 'string') { - throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'encoding', 'string'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'encoding', + 'string', encoding); } var normalizedEncoding = normalizeEncoding(encoding); if (normalizedEncoding === undefined) { diff --git a/src/node_api.cc b/src/node_api.cc index c952d8d3bce..12204c4ba0f 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -216,6 +216,14 @@ V8EscapableHandleScopeFromJsEscapableHandleScope( static_assert(sizeof(v8::Local) == sizeof(napi_value), "Cannot convert between v8::Local and napi_value"); +napi_deferred JsDeferredFromV8Persistent(v8::Persistent* local) { + return reinterpret_cast(local); +} + +v8::Persistent* V8PersistentFromJsDeferred(napi_deferred local) { + return reinterpret_cast*>(local); +} + napi_value JsValueFromV8LocalValue(v8::Local local) { return reinterpret_cast(*local); } @@ -772,6 +780,33 @@ napi_status Unwrap(napi_env env, return napi_ok; } +napi_status ConcludeDeferred(napi_env env, + napi_deferred deferred, + napi_value result, + bool is_resolved) { + NAPI_PREAMBLE(env); + CHECK_ARG(env, result); + + v8::Local context = env->isolate->GetCurrentContext(); + v8::Persistent* deferred_ref = + V8PersistentFromJsDeferred(deferred); + v8::Local v8_deferred = + v8::Local::New(env->isolate, *deferred_ref); + + auto v8_resolver = v8::Local::Cast(v8_deferred); + + v8::Maybe success = is_resolved ? + v8_resolver->Resolve(context, v8impl::V8LocalValueFromJsValue(result)) : + v8_resolver->Reject(context, v8impl::V8LocalValueFromJsValue(result)); + + deferred_ref->Reset(); + delete deferred_ref; + + RETURN_STATUS_IF_FALSE(env, success.FromMaybe(false), napi_generic_failure); + + return GET_RETURN_STATUS(env); +} + } // end of namespace v8impl // Intercepts the Node-V8 module registration callback. Converts parameters @@ -3330,3 +3365,46 @@ napi_status napi_cancel_async_work(napi_env env, napi_async_work work) { return napi_clear_last_error(env); } + +NAPI_EXTERN napi_status napi_create_promise(napi_env env, + napi_deferred* deferred, + napi_value* promise) { + NAPI_PREAMBLE(env); + CHECK_ARG(env, deferred); + CHECK_ARG(env, promise); + + auto maybe = v8::Promise::Resolver::New(env->isolate->GetCurrentContext()); + CHECK_MAYBE_EMPTY(env, maybe, napi_generic_failure); + + auto v8_resolver = maybe.ToLocalChecked(); + auto v8_deferred = new v8::Persistent(); + v8_deferred->Reset(env->isolate, v8_resolver); + + *deferred = v8impl::JsDeferredFromV8Persistent(v8_deferred); + *promise = v8impl::JsValueFromV8LocalValue(v8_resolver->GetPromise()); + return GET_RETURN_STATUS(env); +} + +NAPI_EXTERN napi_status napi_resolve_deferred(napi_env env, + napi_deferred deferred, + napi_value resolution) { + return v8impl::ConcludeDeferred(env, deferred, resolution, true); +} + +NAPI_EXTERN napi_status napi_reject_deferred(napi_env env, + napi_deferred deferred, + napi_value resolution) { + return v8impl::ConcludeDeferred(env, deferred, resolution, false); +} + +NAPI_EXTERN napi_status napi_is_promise(napi_env env, + napi_value promise, + bool* is_promise) { + CHECK_ENV(env); + CHECK_ARG(env, promise); + CHECK_ARG(env, is_promise); + + *is_promise = v8impl::V8LocalValueFromJsValue(promise)->IsPromise(); + + return napi_clear_last_error(env); +} diff --git a/src/node_api.h b/src/node_api.h index 6297d44f3e6..bf0cf27e296 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -545,6 +545,20 @@ NAPI_EXTERN napi_status napi_get_node_version(napi_env env, const napi_node_version** version); +// Promises +NAPI_EXTERN napi_status napi_create_promise(napi_env env, + napi_deferred* deferred, + napi_value* promise); +NAPI_EXTERN napi_status napi_resolve_deferred(napi_env env, + napi_deferred deferred, + napi_value resolution); +NAPI_EXTERN napi_status napi_reject_deferred(napi_env env, + napi_deferred deferred, + napi_value rejection); +NAPI_EXTERN napi_status napi_is_promise(napi_env env, + napi_value promise, + bool* is_promise); + EXTERN_C_END #endif // SRC_NODE_API_H_ diff --git a/src/node_api_types.h b/src/node_api_types.h index 0bdc377c8fc..ac8482bf9dc 100644 --- a/src/node_api_types.h +++ b/src/node_api_types.h @@ -17,6 +17,7 @@ typedef struct napi_handle_scope__ *napi_handle_scope; typedef struct napi_escapable_handle_scope__ *napi_escapable_handle_scope; typedef struct napi_callback_info__ *napi_callback_info; typedef struct napi_async_work__ *napi_async_work; +typedef struct napi_deferred__ *napi_deferred; typedef enum { napi_default = 0, diff --git a/src/node_file.cc b/src/node_file.cc index c01589f5480..9a80c613a69 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -244,13 +244,7 @@ void After(uv_fs_t *req) { req_wrap->encoding_, &error); if (link.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - argv[0] = UVException(env->isolate(), - UV_EINVAL, - req_wrap->syscall(), - "Invalid character encoding for filename", - req->path, - req_wrap->data()); + argv[0] = error; } else { argv[1] = link.ToLocalChecked(); } @@ -263,13 +257,7 @@ void After(uv_fs_t *req) { req_wrap->encoding_, &error); if (link.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - argv[0] = UVException(env->isolate(), - UV_EINVAL, - req_wrap->syscall(), - "Invalid character encoding for link", - req->path, - req_wrap->data()); + argv[0] = error; } else { argv[1] = link.ToLocalChecked(); } @@ -281,13 +269,7 @@ void After(uv_fs_t *req) { req_wrap->encoding_, &error); if (link.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - argv[0] = UVException(env->isolate(), - UV_EINVAL, - req_wrap->syscall(), - "Invalid character encoding for link", - req->path, - req_wrap->data()); + argv[0] = error; } else { argv[1] = link.ToLocalChecked(); } @@ -337,13 +319,7 @@ void After(uv_fs_t *req) { req_wrap->encoding_, &error); if (filename.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - argv[0] = UVException(env->isolate(), - UV_EINVAL, - req_wrap->syscall(), - "Invalid character encoding for filename", - req->path, - req_wrap->data()); + argv[0] = error; break; } name_argv[name_idx++] = filename.ToLocalChecked(); @@ -735,11 +711,8 @@ static void ReadLink(const FunctionCallbackInfo& args) { encoding, &error); if (rc.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - return env->ThrowUVException(UV_EINVAL, - "readlink", - "Invalid character encoding for link", - *path); + env->isolate()->ThrowException(error); + return; } args.GetReturnValue().Set(rc.ToLocalChecked()); } @@ -910,11 +883,8 @@ static void RealPath(const FunctionCallbackInfo& args) { encoding, &error); if (rc.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - return env->ThrowUVException(UV_EINVAL, - "realpath", - "Invalid character encoding for path", - *path); + env->isolate()->ThrowException(error); + return; } args.GetReturnValue().Set(rc.ToLocalChecked()); } @@ -964,11 +934,8 @@ static void ReadDir(const FunctionCallbackInfo& args) { encoding, &error); if (filename.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - return env->ThrowUVException(UV_EINVAL, - "readdir", - "Invalid character encoding for filename", - *path); + env->isolate()->ThrowException(error); + return; } name_v[name_idx++] = filename.ToLocalChecked(); @@ -1439,11 +1406,8 @@ static void Mkdtemp(const FunctionCallbackInfo& args) { MaybeLocal rc = StringBytes::Encode(env->isolate(), path, encoding, &error); if (rc.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - return env->ThrowUVException(UV_EINVAL, - "mkdtemp", - "Invalid character encoding for filename", - *tmpl); + env->isolate()->ThrowException(error); + return; } args.GetReturnValue().Set(rc.ToLocalChecked()); } diff --git a/src/node_os.cc b/src/node_os.cc index 8f950c55b11..2ea3db21286 100644 --- a/src/node_os.cc +++ b/src/node_os.cc @@ -389,27 +389,11 @@ static void GetUserInfo(const FunctionCallbackInfo& args) { else shell = StringBytes::Encode(env->isolate(), pwd.shell, encoding, &error); - uv_os_free_passwd(&pwd); - - if (username.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - return env->ThrowUVException(UV_EINVAL, - "uv_os_get_passwd", - "Invalid character encoding for username"); - } - - if (homedir.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - return env->ThrowUVException(UV_EINVAL, - "uv_os_get_passwd", - "Invalid character encoding for homedir"); - } - - if (shell.IsEmpty()) { - // TODO(addaleax): Use `error` itself here. - return env->ThrowUVException(UV_EINVAL, - "uv_os_get_passwd", - "Invalid character encoding for shell"); + if (username.IsEmpty() || homedir.IsEmpty() || shell.IsEmpty()) { + CHECK(!error.IsEmpty()); + uv_os_free_passwd(&pwd); + env->isolate()->ThrowException(error); + return; } Local entry = Object::New(env->isolate()); diff --git a/src/string_bytes.cc b/src/string_bytes.cc index d3c040859e6..8f5bdaf56e9 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -690,7 +690,6 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, CHECK_NE(encoding, UCS2); CHECK_BUFLEN_IN_RANGE(buflen); - *error = Local(); if (!buflen && encoding != BUFFER) { return String::Empty(isolate); } @@ -776,7 +775,6 @@ MaybeLocal StringBytes::Encode(Isolate* isolate, size_t buflen, Local* error) { CHECK_BUFLEN_IN_RANGE(buflen); - *error = Local(); // Node's "ucs2" encoding expects LE character data inside a // Buffer, so we need to reorder on BE platforms. See diff --git a/test/addons-napi/test_promise/binding.gyp b/test/addons-napi/test_promise/binding.gyp new file mode 100644 index 00000000000..bf266f93db7 --- /dev/null +++ b/test/addons-napi/test_promise/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "test_promise", + "sources": [ "test_promise.c" ] + } + ] +} diff --git a/test/addons-napi/test_promise/test.js b/test/addons-napi/test_promise/test.js new file mode 100644 index 00000000000..4c2a2e5e76c --- /dev/null +++ b/test/addons-napi/test_promise/test.js @@ -0,0 +1,60 @@ +'use strict'; + +const common = require('../../common'); +const test_promise = require(`./build/${common.buildType}/test_promise`); +const assert = require('assert'); + +let expected_result, promise; + +// A resolution +expected_result = 42; +promise = test_promise.createPromise(); +promise.then( + common.mustCall(function(result) { + assert.strictEqual(result, expected_result, + 'promise resolved as expected'); + }), + common.mustNotCall()); +test_promise.concludeCurrentPromise(expected_result, true); + +// A rejection +expected_result = 'It\'s not you, it\'s me.'; +promise = test_promise.createPromise(); +promise.then( + common.mustNotCall(), + common.mustCall(function(result) { + assert.strictEqual(result, expected_result, + 'promise rejected as expected'); + })); +test_promise.concludeCurrentPromise(expected_result, false); + +// Chaining +promise = test_promise.createPromise(); +promise.then( + common.mustCall(function(result) { + assert.strictEqual(result, 'chained answer', + 'resolving with a promise chains properly'); + }), + common.mustNotCall()); +test_promise.concludeCurrentPromise(Promise.resolve('chained answer'), true); + +assert.strictEqual(test_promise.isPromise(promise), true, + 'natively created promise is recognized as a promise'); + +assert.strictEqual(test_promise.isPromise(Promise.reject(-1)), true, + 'Promise created with JS is recognized as a promise'); + +assert.strictEqual(test_promise.isPromise(2.4), false, + 'Number is recognized as not a promise'); + +assert.strictEqual(test_promise.isPromise('I promise!'), false, + 'String is recognized as not a promise'); + +assert.strictEqual(test_promise.isPromise(undefined), false, + 'undefined is recognized as not a promise'); + +assert.strictEqual(test_promise.isPromise(null), false, + 'null is recognized as not a promise'); + +assert.strictEqual(test_promise.isPromise({}), false, + 'an object is recognized as not a promise'); diff --git a/test/addons-napi/test_promise/test_promise.c b/test/addons-napi/test_promise/test_promise.c new file mode 100644 index 00000000000..dc617f4592e --- /dev/null +++ b/test/addons-napi/test_promise/test_promise.c @@ -0,0 +1,60 @@ +#include +#include "../common.h" + +napi_deferred deferred = NULL; + +napi_value createPromise(napi_env env, napi_callback_info info) { + napi_value promise; + + // We do not overwrite an existing deferred. + if (deferred != NULL) { + return NULL; + } + + NAPI_CALL(env, napi_create_promise(env, &deferred, &promise)); + + return promise; +} + +napi_value concludeCurrentPromise(napi_env env, napi_callback_info info) { + napi_value argv[2]; + size_t argc = 2; + bool resolution; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); + NAPI_CALL(env, napi_get_value_bool(env, argv[1], &resolution)); + if (resolution) { + NAPI_CALL(env, napi_resolve_deferred(env, deferred, argv[0])); + } else { + NAPI_CALL(env, napi_reject_deferred(env, deferred, argv[0])); + } + + deferred = NULL; + + return NULL; +} + +napi_value isPromise(napi_env env, napi_callback_info info) { + napi_value promise, result; + size_t argc = 1; + bool is_promise; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &promise, NULL, NULL)); + NAPI_CALL(env, napi_is_promise(env, promise, &is_promise)); + NAPI_CALL(env, napi_get_boolean(env, is_promise, &result)); + + return result; +} + +void Init(napi_env env, napi_value exports, napi_value module, void* priv) { + napi_property_descriptor descriptors[] = { + DECLARE_NAPI_PROPERTY("createPromise", createPromise), + DECLARE_NAPI_PROPERTY("concludeCurrentPromise", concludeCurrentPromise), + DECLARE_NAPI_PROPERTY("isPromise", isPromise), + }; + + NAPI_CALL_RETURN_VOID(env, napi_define_properties( + env, exports, sizeof(descriptors) / sizeof(*descriptors), descriptors)); +} + +NAPI_MODULE(addon, Init) diff --git a/test/addons/async-hello-world/binding.cc b/test/addons/async-hello-world/binding.cc index da2bd417cd9..42291c49011 100644 --- a/test/addons/async-hello-world/binding.cc +++ b/test/addons/async-hello-world/binding.cc @@ -28,6 +28,7 @@ void DoAsync(uv_work_t* r) { req->output = req->input * 2; } +template void AfterAsync(uv_work_t* r) { async_req* req = reinterpret_cast(r->data); v8::Isolate* isolate = req->isolate; @@ -40,9 +41,18 @@ void AfterAsync(uv_work_t* r) { v8::TryCatch try_catch(isolate); + v8::Local global = isolate->GetCurrentContext()->Global(); v8::Local callback = v8::Local::New(isolate, req->callback); - callback->Call(isolate->GetCurrentContext()->Global(), 2, argv); + + if (use_makecallback) { + v8::Local ret = + node::MakeCallback(isolate, global, callback, 2, argv); + // This should be changed to an empty handle. + assert(!ret.IsEmpty()); + } else { + callback->Call(global, 2, argv); + } // cleanup req->callback.Reset(); @@ -53,6 +63,7 @@ void AfterAsync(uv_work_t* r) { } } +template void Method(const v8::FunctionCallbackInfo& args) { v8::Isolate* isolate = args.GetIsolate(); @@ -69,11 +80,12 @@ void Method(const v8::FunctionCallbackInfo& args) { uv_queue_work(uv_default_loop(), &req->req, DoAsync, - (uv_after_work_cb)AfterAsync); + (uv_after_work_cb)AfterAsync); } void init(v8::Local exports, v8::Local module) { - NODE_SET_METHOD(module, "exports", Method); + NODE_SET_METHOD(exports, "runCall", Method); + NODE_SET_METHOD(exports, "runMakeCallback", Method); } NODE_MODULE(binding, init) diff --git a/test/addons/async-hello-world/test-makecallback-uncaught.js b/test/addons/async-hello-world/test-makecallback-uncaught.js new file mode 100644 index 00000000000..c207f535bee --- /dev/null +++ b/test/addons/async-hello-world/test-makecallback-uncaught.js @@ -0,0 +1,9 @@ +'use strict'; +const common = require('../../common'); +const { runMakeCallback } = require(`./build/${common.buildType}/binding`); + +process.on('uncaughtException', common.mustCall()); + +runMakeCallback(5, common.mustCall(() => { + throw new Error('foo'); +})); diff --git a/test/addons/async-hello-world/test-makecallback.js b/test/addons/async-hello-world/test-makecallback.js new file mode 100644 index 00000000000..0edf052e8c3 --- /dev/null +++ b/test/addons/async-hello-world/test-makecallback.js @@ -0,0 +1,10 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const { runMakeCallback } = require(`./build/${common.buildType}/binding`); + +runMakeCallback(5, common.mustCall(function(err, val) { + assert.strictEqual(err, null); + assert.strictEqual(val, 10); + process.nextTick(common.mustCall()); +})); diff --git a/test/addons/async-hello-world/test.js b/test/addons/async-hello-world/test.js index fbd0d7eeb7e..f24071087c0 100644 --- a/test/addons/async-hello-world/test.js +++ b/test/addons/async-hello-world/test.js @@ -1,9 +1,9 @@ 'use strict'; const common = require('../../common'); const assert = require('assert'); -const binding = require(`./build/${common.buildType}/binding`); +const { runCall } = require(`./build/${common.buildType}/binding`); -binding(5, common.mustCall(function(err, val) { +runCall(5, common.mustCall(function(err, val) { assert.strictEqual(err, null); assert.strictEqual(val, 10); process.nextTick(common.mustCall()); diff --git a/test/async-hooks/test-disable-in-init.js b/test/async-hooks/test-disable-in-init.js index 21588bf5046..b7ab31e6d97 100644 --- a/test/async-hooks/test-disable-in-init.js +++ b/test/async-hooks/test-disable-in-init.js @@ -7,7 +7,7 @@ const fs = require('fs'); let nestedCall = false; async_hooks.createHook({ - init: common.mustCall(function(id, type) { + init: common.mustCall(function() { nestedHook.disable(); if (!nestedCall) { nestedCall = true; diff --git a/test/parallel/test-buffer-bytelength.js b/test/parallel/test-buffer-bytelength.js index c4bc90a4ac7..3d4ca1d1845 100644 --- a/test/parallel/test-buffer-bytelength.js +++ b/test/parallel/test-buffer-bytelength.js @@ -5,17 +5,22 @@ const assert = require('assert'); const SlowBuffer = require('buffer').SlowBuffer; const vm = require('vm'); -// coerce values to string -const errMsg = common.expectsError({ - code: 'ERR_INVALID_ARG_TYPE', - type: TypeError, - message: 'The "string" argument must be one of type string, ' + - 'buffer, or arrayBuffer' -}, 4); -assert.throws(() => { Buffer.byteLength(32, 'latin1'); }, errMsg); -assert.throws(() => { Buffer.byteLength(NaN, 'utf8'); }, errMsg); -assert.throws(() => { Buffer.byteLength({}, 'latin1'); }, errMsg); -assert.throws(() => { Buffer.byteLength(); }, errMsg); +[ + [32, 'latin1'], + [NaN, 'utf8'], + [{}, 'latin1'], + [] +].forEach((args) => { + common.expectsError( + () => Buffer.byteLength(...args), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "string" argument must be one of type string, ' + + `buffer, or arrayBuffer. Received type ${typeof args[0]}` + } + ); +}); assert.strictEqual(Buffer.byteLength('', undefined, true), -1); diff --git a/test/parallel/test-buffer-compare-offset.js b/test/parallel/test-buffer-compare-offset.js index 080f7012614..8458ae08fe8 100644 --- a/test/parallel/test-buffer-compare-offset.js +++ b/test/parallel/test-buffer-compare-offset.js @@ -69,5 +69,6 @@ assert.throws(() => a.compare(b, -Infinity, Infinity), oor); assert.throws(() => a.compare(), common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError, - message: 'The "target" argument must be one of type buffer or uint8Array' + message: 'The "target" argument must be one of ' + + 'type buffer or uint8Array. Received type undefined' })); diff --git a/test/parallel/test-buffer-compare.js b/test/parallel/test-buffer-compare.js index 8435e9950ee..e764f494602 100644 --- a/test/parallel/test-buffer-compare.js +++ b/test/parallel/test-buffer-compare.js @@ -41,5 +41,6 @@ assert.throws(() => Buffer.compare('abc', Buffer.alloc(1)), errMsg); assert.throws(() => Buffer.alloc(1).compare('abc'), common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError, - message: 'The "target" argument must be one of type buffer or uint8Array' + message: 'The "target" argument must be one of ' + + 'type buffer or uint8Array. Received type string' })); diff --git a/test/parallel/test-buffer-equals.js b/test/parallel/test-buffer-equals.js index 3f1869cfab7..9296b3f0633 100644 --- a/test/parallel/test-buffer-equals.js +++ b/test/parallel/test-buffer-equals.js @@ -14,10 +14,12 @@ assert.ok(!d.equals(e)); assert.ok(d.equals(d)); assert.ok(d.equals(new Uint8Array([0x61, 0x62, 0x63, 0x64, 0x65]))); -assert.throws(() => Buffer.alloc(1).equals('abc'), - common.expectsError({ - code: 'ERR_INVALID_ARG_TYPE', - type: TypeError, - message: 'The "otherBuffer" argument must be one of type ' + - 'buffer or uint8Array' - })); +common.expectsError( + () => Buffer.alloc(1).equals('abc'), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "otherBuffer" argument must be one of type ' + + 'buffer or uint8Array. Received type string' + } +); diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js index 1732c88a787..7cf3736aec8 100644 --- a/test/parallel/test-buffer-fill.js +++ b/test/parallel/test-buffer-fill.js @@ -192,45 +192,49 @@ deepStrictEqualValues(genBuffer(4, [hexBufFill, 1, -1]), [0, 0, 0, 0]); // Check exceptions -assert.throws( - () => buf1.fill(0, -1), - common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' })); -assert.throws( - () => buf1.fill(0, 0, buf1.length + 1), - common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' })); -assert.throws( - () => buf1.fill('', -1), - common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' })); -assert.throws( - () => buf1.fill('', 0, buf1.length + 1), - common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' })); -assert.throws( +[ + [0, -1], + [0, 0, buf1.length + 1], + ['', -1], + ['', 0, buf1.length + 1] +].forEach((args) => { + common.expectsError( + () => buf1.fill(...args), + { code: 'ERR_INDEX_OUT_OF_RANGE' } + ); +}); + +common.expectsError( () => buf1.fill('a', 0, buf1.length, 'node rocks!'), - common.expectsError({ + { code: 'ERR_UNKNOWN_ENCODING', type: TypeError, message: 'Unknown encoding: node rocks!' - })); -assert.throws( - () => buf1.fill('a', 0, 0, NaN), - common.expectsError({ - code: 'ERR_INVALID_ARG_TYPE', - message: 'The "encoding" argument must be of type string' - })); -assert.throws( - () => buf1.fill('a', 0, 0, null), - common.expectsError({ - code: 'ERR_INVALID_ARG_TYPE', - message: 'The "encoding" argument must be of type string' - })); -assert.throws( + } +); + +[ + ['a', 0, 0, NaN], + ['a', 0, 0, null] +].forEach((args) => { + common.expectsError( + () => buf1.fill(...args), + { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "encoding" argument must be of type ' + + `string. Received type ${args[3] === null ? 'null' : typeof args[3]}` + } + ); +}); + +common.expectsError( () => buf1.fill('a', 0, 0, 'foo'), - common.expectsError({ + { code: 'ERR_UNKNOWN_ENCODING', type: TypeError, message: 'Unknown encoding: foo' - })); - + } +); function genBuffer(size, args) { const b = Buffer.allocUnsafe(size); diff --git a/test/parallel/test-buffer-from.js b/test/parallel/test-buffer-from.js index d5d1a73b48f..c4d5e606ddb 100644 --- a/test/parallel/test-buffer-from.js +++ b/test/parallel/test-buffer-from.js @@ -36,18 +36,19 @@ deepStrictEqual( ); [ - {}, - new Boolean(true), - { valueOf() { return null; } }, - { valueOf() { return undefined; } }, - { valueOf: null }, - Object.create(null) -].forEach((input) => { + [{}, 'object'], + [new Boolean(true), 'boolean'], + [{ valueOf() { return null; } }, 'object'], + [{ valueOf() { return undefined; } }, 'object'], + [{ valueOf: null }, 'object'], + [Object.create(null), 'object'] +].forEach(([input, actualType]) => { const err = common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: 'The first argument must be one of type string, buffer, ' + - 'arrayBuffer, array, or array-like object' + 'arrayBuffer, array, or array-like object. Received ' + + `type ${actualType}` }); throws(() => Buffer.from(input), err); }); diff --git a/test/parallel/test-buffer-includes.js b/test/parallel/test-buffer-includes.js index a1aaa99c73b..2d699315a6d 100644 --- a/test/parallel/test-buffer-includes.js +++ b/test/parallel/test-buffer-includes.js @@ -148,7 +148,7 @@ assert(twoByteString.includes( assert(!twoByteString.includes('\u03a3', -2, 'ucs2')); const mixedByteStringUcs2 = - Buffer.from('\u039a\u0391abc\u03a3\u03a3\u0395', 'ucs2'); + Buffer.from('\u039a\u0391abc\u03a3\u03a3\u0395', 'ucs2'); assert(mixedByteStringUcs2.includes('bc', 0, 'ucs2')); assert(mixedByteStringUcs2.includes('\u03a3', 0, 'ucs2')); assert(!mixedByteStringUcs2.includes('\u0396', 0, 'ucs2')); @@ -261,7 +261,7 @@ for (let lengthIndex = 0; lengthIndex < lengths.length; lengthIndex++) { const length = lengths[lengthIndex]; const patternBufferUcs2 = - allCharsBufferUcs2.slice(index, index + length); + allCharsBufferUcs2.slice(index, index + length); assert.ok( allCharsBufferUcs2.includes(patternBufferUcs2, 0, 'ucs2')); @@ -271,21 +271,21 @@ for (let lengthIndex = 0; lengthIndex < lengths.length; lengthIndex++) { } } -const expectedError = common.expectsError({ - code: 'ERR_INVALID_ARG_TYPE', - type: TypeError, - message: 'The "val" argument must be one of type ' + - 'string, buffer, or uint8Array' -}, 3); -assert.throws(() => { - b.includes(() => {}); -}, expectedError); -assert.throws(() => { - b.includes({}); -}, expectedError); -assert.throws(() => { - b.includes([]); -}, expectedError); +[ + () => { }, + {}, + [] +].forEach((val) => { + common.expectsError( + () => b.includes(val), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "value" argument must be one of type string, ' + + `buffer, or uint8Array. Received type ${typeof val}` + } + ); +}); // test truncation of Number arguments to uint8 { diff --git a/test/parallel/test-buffer-indexof.js b/test/parallel/test-buffer-indexof.js index f5a15de2c5e..6e24b61478a 100644 --- a/test/parallel/test-buffer-indexof.js +++ b/test/parallel/test-buffer-indexof.js @@ -344,24 +344,21 @@ assert.strictEqual(Buffer.from('aaaaa').indexOf('b', 'ucs2'), -1); } } -const argumentExpected = common.expectsError({ - code: 'ERR_INVALID_ARG_TYPE', - type: TypeError, - message: 'The "val" argument must be one of type ' + - 'string, buffer, or uint8Array' -}, 3); - -assert.throws(() => { - b.indexOf(() => { }); -}, argumentExpected); - -assert.throws(() => { - b.indexOf({}); -}, argumentExpected); - -assert.throws(() => { - b.indexOf([]); -}, argumentExpected); +[ + () => {}, + {}, + [] +].forEach((val) => { + common.expectsError( + () => b.indexOf(val), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "value" argument must be one of type string, ' + + `buffer, or uint8Array. Received type ${typeof val}` + } + ); +}); // Test weird offset arguments. // The following offsets coerce to NaN or 0, searching the whole Buffer diff --git a/test/parallel/test-stream-inheritance.js b/test/parallel/test-stream-inheritance.js index 2d9a1e83ef7..a687ea9da05 100644 --- a/test/parallel/test-stream-inheritance.js +++ b/test/parallel/test-stream-inheritance.js @@ -56,3 +56,8 @@ common.expectsError( message: 'undefined does not inherit from CustomWritable' } ); + +class OtherCustomWritable extends Writable {} + +assert(!(new OtherCustomWritable() instanceof CustomWritable)); +assert(!(new CustomWritable() instanceof OtherCustomWritable)); diff --git a/test/parallel/test-tls-client-default-ciphers.js b/test/parallel/test-tls-client-default-ciphers.js index 0630fe947e7..1596c8e981f 100644 --- a/test/parallel/test-tls-client-default-ciphers.js +++ b/test/parallel/test-tls-client-default-ciphers.js @@ -37,11 +37,8 @@ function test1() { throw new Done(); }; - try { - tls.connect(common.PORT); - } catch (e) { - assert(e instanceof Done); - } + assert.throws(tls.connect, Done); + assert.strictEqual(ciphers, tls.DEFAULT_CIPHERS); } test1();