v11.11.0 proposal - #26322
Conversation
Sorry, something went wrong.
|
@BridgeAR sadly an error occured when I tried to trigger a build :( |
Sorry, something went wrong.
|
@nodejs/releasers it was originally planned to release a new current version today. However, the CI is still locked due to the upcoming security release. Should someone else trigger the CI and release this before the security release or shall we wait until that's out of the way? |
Sorry, something went wrong.
|
I'd suggest delaying to avoid potential confusion. v10.15.2 was also due today but has been delayed. |
Sorry, something went wrong.
PR-URL: #26052 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Moving myself temporarily to TSC emertus status. Expecting to take a two month hiatus from TSC duties while I focus on some NearForm internal business. PR-URL: #26106 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The Trace Events JS API isn't functional if none of --trace-events-enabled or --trace-event-categories is passed as a CLI argument. This commit fixes that. In addition, we currently don't test the trace_events JS API in the casewhere no CLI args are provided. This commit adds that test. Fixes #24944 PR-URL: #24945 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
PR-URL: #26101 Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add a test for long strings and assert.notDeepEqual() to cover code that truncates output when it is longer than 1024 characters. PR-URL: #26065 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
libuv returns values for `blksize` and `blocks` on stat calls so do not coerce them into `undefined` on Windows. PR-URL: #26056 Fixes: #25913 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
clang doesn't seem to support 'Wno-old-style-declaration', this is a work-around. Fixes: #25550 Refs: nodejs/node-v0.x-archive#4186 PR-URL: #25954 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Instead of passing the payload for Workers directly to `.onmessage`, perform something more similar to what the browser API provides, namely create an event object with a `.data` property. This does not make `MessagePort` implement the `EventTarget` API, nor does it implement the full `MessageEvent` API, but it would make such extensions non-breaking changes if we desire them at some point in the future. (This would be a breaking change if Workers were not experimental. Currently, this method is also undocumented and only exists with the idea of enabling some degree of Web compatibility.) PR-URL: #26082 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com>
PR-URL: #26083 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #26046 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Currently, this test generated the following compiler warning:
../binding.cc:33:30: warning:
'TLSv1_2_server_method' is deprecated [-Wdeprecated-declarations]
const SSL_METHOD* method = TLSv1_2_server_method();
^
/node/deps/openssl/openssl/include/openssl/ssl.h:1877:1:
note: 'TLSv1_2_server_method' has been explicitly marked deprecated here
DEPRECATEDIN_1_1_0(__owur const SSL_METHOD *TLSv1_2_server_method(void))
^
1 warning generated.
This commit adds -Wno-deprecated-declarations to silence this warning
for this test.
PR-URL: #26067
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: #26091 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
`Environment::RunCleanup` is invoked twice in a row, remove one. PR-URL: #26104 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
PR-URL: #26129 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
It is not obvious that timer handles are cleaned up properly when the current `Environment` exits, nor that the `Environment` knows to keep track of the closing handles. This change may not be necessary, because timer handles close without non-trivial delay (i.e. at the end of the current event loop term), and JS-based inspector sessions (which are the only ones we can easily test) are destroyed when cleaning up, closing the timers as a result. I don’t know what happens for other kinds of inspector sessions, though. PR-URL: #26088 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Since bootstrap/node.js performs the setup synchronously, the process exception handlers do not have to setup so early in the bootstrap process - any fatal errors thrown before user code execution should simply crash the process, and we do not care about any clean up at that point. We don't care about emitting any events if the process crash upon bootstrap either. PR-URL: #26061 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Gus Caplan <me@gus.host>
PR-URL: #26114 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Consolidate the N-API material in the Collaborator Guide to be succinct and direct. PR-URL: #26094 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com>
PR-URL: #26073 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Do not put the `.stop()` and `.drain()` methods on the `MessagePort` prototype if we are going to remove them later on anyway. PR-URL: #26109 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
An empty `Local<>` already indicates that an exception is pending, so there is no need to throw an exception. In the case of Workers, this could override a `.terminate()` call. PR-URL: #26112 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com>
Do not assume that entering JS cannot fail. PR-URL: #26113 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: James M Snell <jasnell@gmail.com>
These checks were useful while investigating other issues; using empty `Local<>`s can be very un-debuggable, because that typically does not lead to assertions with debugging information but rather crashes based on accessing invalid memory. PR-URL: #26125 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
I backed out the mentioned PR that caused the build failures. It won't be possible to push the release today anymore, so I also updated the release date. @addaleax I updated the notable changes as suggested. PTAL. CI https://ci.nodejs.org/job/node-test-pull-request/21233/ Earlier I went through the CITGM results and could not find anything related to this release. There are a couple of failures that pop up frequently at the moment and I opened some PRs to fix them. |
Sorry, something went wrong.
|
@BridgeAR Fwiw, the first commit in that PR seemed to be okay. The notable changes LGTM 👍 |
Sorry, something went wrong.
|
@addaleax AFAIK we always include / exclude whole PRs and not individual commits from a PR. |
Sorry, something went wrong.
|
Resumed CI https://ci.nodejs.org/job/node-test-commit/26425/ @nodejs/benchmarking-admins the CI failed at least twice on the job e.g. https://ci.nodejs.org/job/node-test-commit-v8-linux/2138/nodes=benchmark,v8test=v8test/console |
Sorry, something went wrong.
|
@nodejs/releasers @nodejs/build any opinion on the benchmark failures from the V8-CI? Should this block the release? It seems reproducible on each run. |
Sorry, something went wrong.
|
Run against master to compare: https://ci.nodejs.org/job/node-test-commit-v8-linux/2142/ |
Sorry, something went wrong.
|
So I guess it's an infra issue and we can go ahead and ignore it? |
Sorry, something went wrong.
|
Once more resuming the CI https://ci.nodejs.org/job/node-test-commit/26434/ |
Sorry, something went wrong.
|
Seems like there is one legitimate test failure for the --without-ssl builds. I am looking into it right now. |
Sorry, something went wrong.
|
It seems a deprecation warning is emitted twice for such builds while requireing 'node-inspect/lib/internal/inspect_client'. https://ci.nodejs.org/job/node-test-commit-linux-containered/11111/nodes=ubuntu1604_sharedlibs_withoutssl_x64/console I would like to ignore that test for now as it does not seem critical to block the release. Any other opinions? @nodejs/releasers @nodejs/tsc |
Sorry, something went wrong.
|
I am not able to reproduce the issue locally by the way. |
Sorry, something went wrong.
|
Rebuild linux-containered https://ci.nodejs.org/job/node-test-commit-linux-containered/11113/ (same error). |
Sorry, something went wrong.
|
@bcoe we plan on releasing a new version in a week again and I would rather get this release out as it is as long as nothing is blocking it. |
Sorry, something went wrong.
|
sounds good @BridgeAR 👍 just want to make sure some of these backports are on our radar. I'm going to also see if we could port any of these patches back to 8.x. |
Sorry, something went wrong.
We have only just enabled the --without-ssl builds in the last week: nodejs/build#1574 The test in question doesn't exist in master (removed in #25138 which is semver-major). I kicked off https://ci.nodejs.org/job/node-test-commit-linux-containered/11117/ against v11.x (i.e. 11.10.1) and the test failed there so it's not a new regression in this proposed release so I'm okay with ignoring this for now but we do need to address it so the builds pass for the next 11.x release. |
Sorry, something went wrong.
PR-URL: #26322
|
a bit off-topic, but, there's lint in the v11.x-staging branch - https://travis-ci.com/nodejs/node/builds/104258827 |
Sorry, something went wrong.
|
@refack this has nothing to do with that release. I am working currently working on the staging branch to have a working branch to open a new release proposal but there were lots and lots of conflicts this time and I had to bisect a few times to find the commit that caused issues. So I am still going to force-push all issues out. |
Sorry, something went wrong.
|
@BridgeAR with V11.11.0 something changed in regards to how require handles failed attempts: try {
require('./non-existant');
} catch (e) {}
require('fs').writeFileSync('./non-existant', '{}');
require('./non-existant');The second require attempt does not throw but instead resolves to the file. I have found the following three commits that touch the module system, which might be causing this behaviour: I'm currently trying to compile each of these commits to find out which one introduced this behaviour and will open a proper issue once I have found out what actually happens here. |
Sorry, something went wrong.
|
@ZauberNerd thanks for reporting that. It should indeed resolve and I am pretty sure it's d94f4c2 that broke this. Please open a separate issue for this though. Thanks! |
Sorry, something went wrong.
Notable Changes
Commits