- Feb 10, 2020
-
-
antsmartian authored
PR-URL: https://github.com/nodejs/node/pull/31187 Refs: https://github.com/nodejs/node/pull/31144 Reviewed-By:
Luigi Pinca <luigipinca@gmail.com> Reviewed-By:
Denys Otrishko <shishugi@gmail.com> Reviewed-By:
Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By:
Rich Trott <rtrott@gmail.com> Reviewed-By:
Robert Nagy <ronagy@icloud.com> Reviewed-By:
Anna Henningsen <anna@addaleax.net>
-
Anna Henningsen authored
Since for all affected conversions there is a preceding type check, it’s safe to use `.FromJust()` instead. This fixes the following compile warnings: ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2242:43: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = long int]’, declared with attribute warn_unused_result [-Wunused-result] 2242 | args[1]->IntegerValue(env->context()).To(&start); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../src/node_url.cc: In function ‘node::url::url_data node::url::{anonymous}::HarvestBase(node::Environment*, v8::Local<v8::Object>)’: ../src/node_url.cc:1206:34: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1206 | flags->Int32Value(context).To(&base.flags); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ ../src/node_url.cc:1211:33: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1211 | port->Int32Value(context).To(&base.port); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2245:15: warning: ‘start’ may be used uninitialized in this function [-Wmaybe-uninitialized] 2245 | for (size_t i = start; i < value.length(); i++) { | ^ PR-URL: https://github.com/nodejs/node/pull/31689 Reviewed-By:James M Snell <jasnell@gmail.com> Reviewed-By:
Michaël Zasso <targos@protonmail.com> Reviewed-By:
David Carlier <devnexen@gmail.com> Reviewed-By:
Colin Ihrig <cjihrig@gmail.com>
-
Anna Henningsen authored
Sometimes, the expected warnings were not emitted. To harden the test: - Instead of relying on a timeout that raced against the file being opened, wait until the file is opened and then schedule GC after that. - Explicitly keep the event loop alive for one turn to make sure the warning is being seen. PR-URL: https://github.com/nodejs/node/pull/31687 Reviewed-By:
-
Rich Trott authored
Minor edits to improve introductory test for util module documentation. PR-URL: https://github.com/nodejs/node/pull/31685 Reviewed-By:
-
Rich Trott authored
Reword the paragraph on Unix domain paths. Hopefully, it is a little bit more clear and easier to read now. PR-URL: https://github.com/nodejs/node/pull/31684 Reviewed-By:
Richard Lau <riclau@uk.ibm.com> Reviewed-By:
-
Anna Henningsen authored
Previously, the test could fail on slow machines because the child process was still in the process of starting up after one second, and not yet idle. To resolve this: - Wait for a message from the child process indicating that it had started. - Wait some time after that, but make it platform-dependent to account for timing differences. - Remove the timer in the child process. PR-URL: https://github.com/nodejs/node/pull/31645 Reviewed-By:
-
Rich Trott authored
Add test coverage for dns.promises.resolve() handling an exception from c-ares. Refs: https://coverage.nodejs.org/coverage-d213f21c72f77da6/lib/internal/dns/promises.js.html#L198 PR-URL: https://github.com/nodejs/node/pull/31678 Reviewed-By:
-
cjihrig authored
PR-URL: https://github.com/nodejs/node/pull/31674 Refs: https://github.com/nodejs/node/pull/28396 Reviewed-By:
-
cjihrig authored
Refs: https://github.com/nodejs/node/pull/31169 PR-URL: https://github.com/nodejs/node/pull/31675 Reviewed-By:
-
cjihrig authored
Refs: https://github.com/nodejs/node/pull/31169 PR-URL: https://github.com/nodejs/node/pull/31675 Reviewed-By:
-
cjihrig authored
Refs: https://github.com/nodejs/node/pull/31167 PR-URL: https://github.com/nodejs/node/pull/31675 Reviewed-By:
-
cjihrig authored
This commit removes an extrea intermediate variable. This makes the call consistent with other uses of validateUint32() in the codebase. PR-URL: https://github.com/nodejs/node/pull/31676 Reviewed-By:
-
- Feb 09, 2020
-
-
Ruben Bridgewater authored
It was possible that this warning ends up in an infinite recursion. The reason is that printing the warning triggered a color check and that triggered another warning. Limiting it to a single warning prevents this. PR-URL: https://github.com/nodejs/node/pull/31429 Reviewed-By:
-
Ruben Bridgewater authored
Make sure the assertion is actually triggered by using `assert.throws()` instead of `try/catch`. PR-URL: https://github.com/nodejs/node/pull/31429 Reviewed-By:
-
Ruben Bridgewater authored
This makes sure color codes are not taken into account in case util.inspect's default value was changed. PR-URL: https://github.com/nodejs/node/pull/31429 Reviewed-By:
-
Ruben Bridgewater authored
To indicate which lines are test lines and which from Node.js core, it's good to rely on `util.inspect()` while inspecting errors. The stack was accessed directly instead in multiple cases and logging that does not provide as much information as using `util.inspect()`. PR-URL: https://github.com/nodejs/node/pull/31425 Reviewed-By:
Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By:
Yihong Wang <yh.wang@ibm.com>
-
Ruben Bridgewater authored
It was necessary to have fallbacks to run the original tests. This is obsolete with the new test mode. PR-URL: https://github.com/nodejs/node/pull/31396 Reviewed-By:
Sam Roberts <vieuxtech@gmail.com> Reviewed-By:
-
Ruben Bridgewater authored
PR-URL: https://github.com/nodejs/node/pull/31396 Reviewed-By:
-
Ruben Bridgewater authored
This adds a new `test` option. Using it automatically uses a single minimal option matrix to verify the benchmark works as expected. Using the new `all` option makes sure all test suites are run. On top of that the benchmarks will from now on report properly what category might have a typo, if any. The http duration was also refactored to use a option instead of relying on a configuration setting. The fixture folder is ignored as test suite from now on. PR-URL: https://github.com/nodejs/node/pull/31396 Fixes: https://github.com/nodejs/node/issues/31083 Reviewed-By:
-
Ruben Bridgewater authored
This is the basis to refactor the helper to use modern class language features such as private fields. It also refactors the exports to use module.exports. That way it's immediately clear what parts are exported. PR-URL: https://github.com/nodejs/node/pull/31396 Reviewed-By:
-
Brian White authored
PR-URL: https://github.com/nodejs/node/pull/31615 Reviewed-By:
-
Brian White authored
PR-URL: https://github.com/nodejs/node/pull/31624 Reviewed-By:
Matteo Collina <matteo.collina@gmail.com> Reviewed-By:
-
Brian White authored
PR-URL: https://github.com/nodejs/node/pull/31624 Reviewed-By:
-
Rich Trott authored
PR-URL: https://github.com/nodejs/node/pull/31667 Reviewed-By:
-
Robert Nagy authored
Previously finished(stream, cb) would not invoke the callback for streams that have already finished, ended or errored before being passed to finished(stream, cb). PR-URL: https://github.com/nodejs/node/pull/31509 Refs: https://github.com/nodejs/node/pull/31508 Reviewed-By:
-
Robert Nagy authored
This makes readable and writable automatically computed based on the stream state. Effectivly deprecating/discouraging manual management of this. Makes the properties more consistent and easier to reason about. Fixes: https://github.com/nodejs/node/issues/29377 PR-URL: https://github.com/nodejs/node/pull/31197 Refs: https://github.com/nodejs/node/issues/29377 Reviewed-By:
-
Yuhanun Citgez authored
PR-URL: https://github.com/nodejs/node/pull/31654 Reviewed-By:
Gus Caplan <me@gus.host> Reviewed-By:
Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By:
Tiancheng "Timothy" Gu <timothygu99@gmail.com>
-
Anna Henningsen authored
Building Node.js with `--debug` takes a long time and generates binaries that can be hard to work with due to their size, in particular because debug builds of V8 are large. Sometimes, it’s sufficient to build the Node.js parts with native debugging options enabled. Add an option for that. PR-URL: https://github.com/nodejs/node/pull/31644 Reviewed-By:
Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By:
-
Rich Trott authored
Explain the purpose of async_hooks simply in the overview. PR-URL: https://github.com/nodejs/node/pull/31660 Reviewed-By:
-
- Feb 08, 2020
-
-
Gerhard Stoebich authored
Remove unneeded condition as the first line in function already ensures that recv can't be empty. PR-URL: https://github.com/nodejs/node/pull/31622 Reviewed-By:
Chengzhong Wu <legendecas@gmail.com> Reviewed-By:
Tobias Nießen <tniessen@tnie.de> Reviewed-By:
-
Anna Henningsen authored
It turns out that using `v8::Locker` undoes the effects of passing an explicit stack limit as part of the `Isolate`’s resource constraints. Therefore, reset the stack limit manually after entering a Locker. Refs: https://github.com/nodejs/node/pull/26049#issuecomment-580668530 PR-URL: https://github.com/nodejs/node/pull/31593 Reviewed-By:
Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By:
-
Rich Trott authored
fe397578 added a setting to a benchmark which needs to be reflected in the test. PR-URL: https://github.com/nodejs/node/pull/31686 Reviewed-By:
-
Anna Henningsen authored
Previously, the test waited for a (any) message from the workers, and then attached another event listener to a specific kind of message. However, it was possible that the second listener was attached after the Worker had already exited, thus never receiving the message it was supposed to receive. (This is the race condition here – usually, the Worker thread would exit *after* the second listener was attached.) Solve this by keeping a single `'message'` event listener attached to the worker instance during its entire lifetime. Fixes: https://github.com/nodejs/node/issues/31226 PR-URL: https://github.com/nodejs/node/pull/31637 Reviewed-By:
Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By:
-
Rich Trott authored
process.emitWarning() "now" option is undocumented and a Boolean trap. Remove it before people start adopting it. We only need it in one place internally. Replace it with an internal-only emitWarningSync() function. PR-URL: https://github.com/nodejs/node/pull/31643 Reviewed-By:
Myles Borins <myles.borins@gmail.com>
-
Anna Henningsen authored
It looks like it’s virtually impossible at this point to create “fake” backing stores for objects that don’t fully own their memory allocations, like the sub-field `js_fields_` of `Http2Session`. In particular, it turns out that an `ArrayBuffer` cannot always be easily separated from its backing store in that situation through by detaching it. This commit gives the JS-exposed parts of the class its own memory allocation and its own backing store, simplifying the code a bit and fixing flakiness coming from it, at the cost of one additional layer of indirection when accessing the data. Refs: https://github.com/nodejs/node/pull/30782 Fixes: https://github.com/nodejs/node/issues/31107 PR-URL: https://github.com/nodejs/node/pull/31648 Reviewed-By:
-
Ben Noordhuis authored
Remove the version of GetHumanReadableProcessName() that operates on a fixed-size buffer. The only remaining caller is Assert() which might get called in contexts where dynamically allocating memory isn't possible but as Assert() calls printf(), which also allocates memory when necessary, this commit is unlikely to make matters much worse. PR-URL: https://github.com/nodejs/node/pull/31633 Fixes: https://github.com/nodejs/node/issues/31631 Reviewed-By:
-
Ben Noordhuis authored
The getter passed a stack-allocated, fixed-size buffer to uv_get_process_title() but neglected to check the return value. When the total length of the command line arguments exceeds the size of the buffer, libuv returns UV_ENOBUFS and doesn't modify the contents of the buffer. The getter then proceeded to return whatever garbage was on the stack at the time of the call, quite possibly reading beyond the end of the buffer. Add a GetProcessTitle() helper that reads the process title into a dynamically allocated buffer that is resized when necessary. Fixes: https://github.com/nodejs/node/issues/31631 PR-URL: https://github.com/nodejs/node/pull/31633 Reviewed-By:
-
Anna Henningsen authored
Motivated by the fact that getting this wrong has led to flaky tests in our test suite. Refs: https://github.com/nodejs/node/pull/31637 PR-URL: https://github.com/nodejs/node/pull/31642 Reviewed-By:
Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By:
-
James M Snell authored
Went hunting for possible performance improvements. Didn't find anything significant but did manage to make a number of style improvements that bring more in line with style guidelines and good pratice. PR-URL: https://github.com/nodejs/node/pull/31651 Reviewed-By:
-
Tobias Nießen authored
PR-URL: https://github.com/nodejs/node/pull/31652 Reviewed-By:
Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By:
-
