-
Notifications
You must be signed in to change notification settings - Fork 341
Conversation
This reverts commit 571882c. Removing the process.nextTick() call can prevent the consumer from being able to catch error events. PR-URL: nodejs/node#12854 Fixes: nodejs/node#12841 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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: nodejs/node#12854 Fixes: nodejs/node#12841 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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: nodejs/node#12993 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fixes: nodejs/node#12833 PR-URL: nodejs/node#12957 Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Michael Dawson <mhdawson@ibm.com>
PR-URL: nodejs/node#13012 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
PR-URL: nodejs/node#13113 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs/node#13118 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Michael Dawson <mhdawson@ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Use `file` as name of the argument, as the CLI documentation does. PR-URL: nodejs/node#13120 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
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: nodejs/node#13074 Reviewed-By: Anna Henningsen <anna@addaleax.net>
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: nodejs/node#13074 Reviewed-By: Anna Henningsen <anna@addaleax.net>
Fix mismatch in title for napi_get_value_string_utf16 Fixes: nodejs/abi-stable-node#243 PR-URL: nodejs/node#13123 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add tests to validate that properties marked as static are available through the class as opposed to instances PR-URL: nodejs/node#13124 Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Hitesh Kanwathirtha <digitalinfinity@gmail.com>
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: nodejs/node#13125 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <mhdawson@ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Use common.mustCall() in test-fs-makeStatsCallback to confirm that the callback is invoked. PR-URL: nodejs/node#13132 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <mhdawson@ibm.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
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: nodejs/node#13134 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <mhdawson@ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
* Wrap text at 80 characters. * Use periods consistently. PR-URL: nodejs/node#13135 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <mhdawson@ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
PR-URL: nodejs/node#13138 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#13145 Refs: http://eslint.org/docs/rules/no-useless-constructor Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
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 nodejs/node#12925. PR-URL: nodejs/node#13156 Fixes: websockets/ws#1118 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
In test/pummel/test-stream2-basic.js, check that noop functions are called the expected number of times. PR-URL: nodejs/node#13146 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs/node#13146 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
test-stream2-basic runs in a few seconds. It can be moved to parallel. PR-URL: nodejs/node#13146 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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: nodejs/node#12926 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Ruben Bridgewater <ruben.bridgewater@fintura.de>
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: nodejs/node#12828 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
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: nodejs/node#13190 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <mhdawson@ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
In a969132, can you explain "test was failing because of over specifiying a not condition"? And is this failure from a new test that was added with the merge, or a test that was already existing (and previously passing) in Node-ChakraCore? |
@jasongin - The failure came when new coverage was added in nodejs/node@47919b3 . The bug was in this line. The condition should be just |
OK thanks. That looks fine then. Though for clarity, the component prefix on that commit message should probably be |
Yes, i think i will just separate out that change in a different commit when i merge. |
Should a969132 also have "src" in the tags? |
Merge commit 260cd41 as of 2017-05-24 PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* Napi test was failing because of over specifiying a not condition. Fixed that and also restructured the code to not iterate over list of properties twice. * Reintroduced a condition for debugger that got removed in previous merge. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* test-vm-timeout-rethrow: This started failing after nodejs/node#13074/ changes upstream. The change is to not do `GetAndClearException` but allocate a string (`JsCreateString`) which throws because engine is in exception state. We need to figure out better way to handle these situation. Marked this as flaky for now. * test-bindings: Marked this as flaky for now. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* test-vm-timeout-rethrow: This started failing after nodejs/node#13074/ changes upstream. The change is to not do `GetAndClearException` but allocate a string (`JsCreateString`) which throws because engine is in exception state. We need to figure out better way to handle these situation. Marked below test as flaky. * parallel/test-vm-sigint : Marked this as flaky for now. * parallel/test-vm-sigint-existing-handler : Marked this as flaky for now. * parallel/test-repl-sigint : Marked this as flaky for now. * parallel/test-bindings: Marked this as flaky for now. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* Napi test was failing because of over specifiying a not condition. Fixed that and also restructured the code to not iterate over list of properties twice. * Reintroduced a condition for debugger that got removed in previous merge. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* test-vm-timeout-rethrow: This started failing after nodejs/node#13074/ changes upstream. The change is to not do `GetAndClearException` but allocate a string (`JsCreateString`) which throws because engine is in exception state. We need to figure out better way to handle these situation. Marked below test as flaky. * parallel/test-vm-sigint : Marked this as flaky for now. * parallel/test-vm-sigint-existing-handler : Marked this as flaky for now. * parallel/test-repl-sigint : Marked this as flaky for now. * parallel/test-bindings: Marked this as flaky for now. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* Napi test was failing because of over specifiying a not condition. Fixed that and also restructured the code to not iterate over list of properties twice. * Reintroduced a condition for debugger that got removed in previous merge. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* test-vm-timeout-rethrow: This started failing after nodejs/node#13074/ changes upstream. The change is to not do `GetAndClearException` but allocate a string (`JsCreateString`) which throws because engine is in exception state. We need to figure out better way to handle these situation. Marked below test as flaky. * parallel/test-vm-sigint : Marked this as flaky for now. * parallel/test-vm-sigint-existing-handler : Marked this as flaky for now. * parallel/test-repl-sigint : Marked this as flaky for now. * parallel/test-bindings: Marked this as flaky for now. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* Napi test was failing because of over specifiying a not condition. Fixed that and also restructured the code to not iterate over list of properties twice. * Reintroduced a condition for debugger that got removed in previous merge. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* test-vm-timeout-rethrow: This started failing after nodejs/node#13074/ changes upstream. The change is to not do `GetAndClearException` but allocate a string (`JsCreateString`) which throws because engine is in exception state. We need to figure out better way to handle these situation. Marked below test as flaky. * parallel/test-vm-sigint : Marked this as flaky for now. * parallel/test-vm-sigint-existing-handler : Marked this as flaky for now. * parallel/test-repl-sigint : Marked this as flaky for now. * parallel/test-bindings: Marked this as flaky for now. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* Napi test was failing because of over specifiying a not condition. Fixed that and also restructured the code to not iterate over list of properties twice. * Reintroduced a condition for debugger that got removed in previous merge. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
* test-vm-timeout-rethrow: This started failing after nodejs/node#13074/ changes upstream. The change is to not do `GetAndClearException` but allocate a string (`JsCreateString`) which throws because engine is in exception state. We need to figure out better way to handle these situation. Marked below test as flaky. * parallel/test-vm-sigint : Marked this as flaky for now. * parallel/test-vm-sigint-existing-handler : Marked this as flaky for now. * parallel/test-repl-sigint : Marked this as flaky for now. * parallel/test-bindings: Marked this as flaky for now. PR-URL: nodejs#264 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Merge commit 260cd41 as of 2017-05-24
Interested commit to review:
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
core, test