Skip to content

Conversation

@MylesBorins
Copy link
Contributor

@MylesBorinsMylesBorins commented Mar 6, 2018

2018-03-07, Version 9.8.0 (Current), @MylesBorins

Notable Changes

  • crypto:
    • add cert.fingerprint256 as SHA256 fingerprint (Hannes Magnusson) #17690
  • http2:
    • Fixed issues with aborted connections in the HTTP/2 implementation (Anna Henningsen) #18987#19002
  • loader:
    • --inspect-brk now works properly for esmodules (Gus Caplan) #18949
  • src:
    • make process.dlopen() load well-known symbol (Ben Noordhuis) #18934
  • trace_events:
    • add file pattern cli option (Andreas Madsen) #18480
  • Added new collaborators

Commits

danbevand others added 30 commits March 5, 2018 09:43
Currently, when configured --without-ssl test-repl-tab-complete fails with the following error: assert.js:43 throw new errors.AssertionError(obj); ^ AssertionError [ERR_ASSERTION]: [ [], 'lexicalL' ] deepStrictEqual [] at testRepl.complete.common.mustCall (node/test/parallel/test-repl-tab-complete.js:549:14) at /node/test/common/index.js:530:15 at completionGroupsLoaded (repl.js:1204:5) at REPLServer.complete (repl.js:1090:11) at REPLServer.completer (repl.js:450:14) at REPLServer.complete (repl.js:919:18) at __dirname.forEach (parallel/test-repl-tab-complete.js:548:14) at Array.forEach (<anonymous>) at Object.<anonymous> (parallel/test-repl-tab-complete.js:545:29) at Module._compile (module.js:660:30) This commit attempts to fix this test but I'm not sure if this is a proper fix as I'm not familiar with the repl code base yet. PR-URL: #17867 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #18875 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #17690 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #18895 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #18981Fixes: #18978 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #18928 Reviewed-By: Claudio Rodriguez <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
There are two minor issues in the AsyncHook constructor, if the object passed in has an after and/or destroy property that are not functions the errors thrown will still be: TypeError [ERR_ASYNC_CALLBACK]: before must be a function This commit updates the code and adds a unit test. PR-URL: #19000 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
PR-URL: #18958Fixes: #18938 Ref: nodejs/build#1145 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]>
PR-URL: #18949Fixes: #18948 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]>
When socket is closed on a response for a request that is being piped to a stream there is a condition where aborted event will be fired to http client when socket is closing and the incomingMessage stream is still set to readable. We need a check for request being complete and to only raise the 'aborted' event on the http client if we have not yet completed reading the response from the server. Fixes: #18756 PR-URL: #18999 Reviewed-By: Shingo Inoue <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
Tests in progress to reproduce issue consistently. Fixes: #18756 PR-URL: #18999 Reviewed-By: Shingo Inoue <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #18999 Reviewed-By: Shingo Inoue <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
Directory symlinks in Windows require the 'dir' flag to be passed to create the symlink correctly. PR-URL: #19049 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #19052 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #19053 Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Send a human-readable HTTP/1 response in case of an unexpected ALPN protocol. This helps with debugging this condition, since previously the only result of it would be a closed socket. PR-URL: #18986 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Make the test for pending deprecations work when the env var is set during the whole test suite run. PR-URL: #18991 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Previously, if `session.destroy()` was called with an error object, the information contained in it would be discarded and a generic `ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams. Instead, make the information from the original error object available. PR-URL: #18988 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
`async` and `bytes` are only interesting when the write is coming from JS, and unnecessary otherwise. Also, make all of the stream `Write*()` bindings use the same code for setting these, and upgrade to the non-deprecated versions. PR-URL: #18963 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Passing a pointer to a static integer is sufficient for the test. PR-URL: #19039 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Hitesh Kanwathirtha <[email protected]>
PR-URL: #18080 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
libuv and zlib symbols are also purposefully re-exported by Node.js for use in Addons. Refs: #17444 PR-URL: #19013 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Use common.platformTimeout() to give longer durations to Raspberry Pi devices to make test more reliable. PR-URL: #18126Fixes: #16772 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
The test was flaky because it relied on a specific order of asynchronous operation that were fired paralellely. This was true on most platform and conditions, but not all the time. See: #18986 PR-URL: #19093 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #19109 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Documentation for N-API Custom Asynchronous Operations incorrectly stated that async execution happens on the main event loop. Added details to napi_create_async_work about which threads are used to invoke the execute and complete callbacks. Changed 'async' to 'asynchronous' in the documentation for Custom Asynchronous Operations. Changed "executes in parallel" to "can execute in parallel" for the documentation of napi_create_async_work execute parameter. PR-URL: #19073Fixes: #19071 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #19126 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
The default message will be printed if the assertion fires. Use block scope for related variables and tests. PR-URL: #19054 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
TimothyGuand others added 12 commits March 7, 2018 09:30
Fixes: #17892Fixes: #17893Fixes: #18992 PR-URL: #18993 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: #19162 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Replace v8::Persistent with node::Persistent, a specialization that resets the persistent handle on destruction. Prevents accidental resource leaks when forgetting to call .Reset() manually. I'm fairly confident this commit fixes a number of resource leaks that have gone undiagnosed so far. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
The previous commit made persistent handles auto-reset on destruction. This commit removes the Reset() calls that are now no longer necessary. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Don't try to update the internal field pointer of the JS object in the destructor. The garbage collector invokes the destructor when the object is collected and is not necessarily in a valid state anymore. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
This fixes a crash that occurred when a `Http2Stream` write is completed after it is already destroyed. Instead, don’t destroy the stream in that case and wait for GC to take over. Backport-PR-URL: #19185 PR-URL: #19002Fixes: #18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
This adds support for ensuring that the top-level main into Node is supported loading when it has no extension for backwards-compat with NodeJS bin workflows. In addition package.json caching is implemented in the module lookup process. Backport-PR-URL: #18923 PR-URL: #18728 Reviewed-By: Benjamin Gruenbaum <[email protected]>
Backport-PR-URL: #18923 PR-URL: #18788 Refs: #18728 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: #19180 PR-URL: #18925 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
We have two notes in API docs about Android support: the first has no links, the second links to the table of supported OSs where Android is not mentioned which may be confusing. This PR makes both notes link to dedicated Android part of BUILDING.md. Backport-PR-URL: #19183 PR-URL: #19004 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>
Backport-PR-URL: #19194 PR-URL: #18607 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins added a commit that referenced this pull request Mar 7, 2018
Notable Changes: * crypto: - add cert.fingerprint256 as SHA256 fingerprint (Hannes Magnusson) #17690 * lib: - v8_prof_processor works again 🎉 (Anna Henningsen) #19059 * loader: - --inspect-brk now works properly for esmodules (Gus Caplan) #18949 * src: - handle exceptions in env-\>SetImmediates (James M Snell) #18297 - make process.dlopen() load well-known symbol (Ben Noordhuis) #18934 * trace_events: - add file pattern cli option (Andreas Madsen) #18480 PR-URL: #19181
@MylesBorins
Copy link
ContributorAuthor

@addaleax
Copy link
Member

v8_prof_processor works again

@MylesBorins I wouldn’t put it in the notable changes section. It doesn’t actually work yet, it’s been broken for quite a while for some other reason that we still have to get into. What my patch did was only fixing part of that, and I think it’s not usable again (yet).

@addaleax
Copy link
Member

handle exceptions in env->SetImmediates

Also not really a notable change, tbh … before the other commits from the same PR, there were no such exceptions happening at all, and we’re not backporting the rest of the PR.

It’s really only backported to v9.x to avoid merge conflicts. :)

(We can also remove the SEMVER-MINOR tag from the changelog here, I guess. It’s just a drawback of the Backport-PR-URL: approach that we can’t tag the backport separately…)

doc: add MoonBall to collaborators

I think some of the others in @nodejs/release always include these as notable changes?

http2: no stream destroy while its data is on the wire

tls,http2: handle writes after SSL destroy more gracefully

I think you can call these out as notable changes. Something like “Fixed issues with aborted connections in the HTTP/2 implementation.” should be fine?

@addaleax
Copy link
Member

v8_prof_processor works again

@MylesBorins I wouldn’t put it in the notable changes section. It doesn’t actually work yet, it’s been broken for quite a while for some other reason that we still have to get into. What my patch did was only fixing part of that, and I think it’s not usable again (yet).

Correcting myself here: Okay, it appears to be partially working, but the tests still fail on master, and the output doesn't quite seem correct...... Sigh.

Notable Changes: * crypto: - add cert.fingerprint256 as SHA256 fingerprint (Hannes Magnusson) #17690 * http2: - Fixed issues with aborted connections in the HTTP/2 implementation (Anna Henningsen) #18987#19002 * loader: - --inspect-brk now works properly for esmodules (Gus Caplan) #18949 * src: - make process.dlopen() load well-known symbol (Ben Noordhuis) #18934 * trace_events: - add file pattern cli option (Andreas Madsen) #18480 * Added new collaborators: - Chen Gang (MoonBall) https://github.com/MoonBall PR-URL: #19181
@MylesBorins
Copy link
ContributorAuthor

Updated the changelog as requested. Also fixed minor linting error (doc related). CITGM looks good just waiting for last arm to finish running

@MylesBorinsMylesBorins merged commit 27ba6e2 into v9.xMar 8, 2018
MylesBorins added a commit that referenced this pull request Mar 8, 2018
MylesBorins added a commit that referenced this pull request Mar 8, 2018
Notable Changes: * crypto: - add cert.fingerprint256 as SHA256 fingerprint (Hannes Magnusson) #17690 * http2: - Fixed issues with aborted connections in the HTTP/2 implementation (Anna Henningsen) #18987#19002 * loader: - --inspect-brk now works properly for esmodules (Gus Caplan) #18949 * src: - make process.dlopen() load well-known symbol (Ben Noordhuis) #18934 * trace_events: - add file pattern cli option (Andreas Madsen) #18480 * Added new collaborators: - Chen Gang (MoonBall) https://github.com/MoonBall PR-URL: #19181
@gibfahngibfahn deleted the v9.8.0-proposal branch March 8, 2018 19:28
@targostargos mentioned this pull request Mar 18, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Notable Changes: * crypto: - add cert.fingerprint256 as SHA256 fingerprint (Hannes Magnusson) nodejs#17690 * http2: - Fixed issues with aborted connections in the HTTP/2 implementation (Anna Henningsen) nodejs#18987nodejs#19002 * loader: - --inspect-brk now works properly for esmodules (Gus Caplan) nodejs#18949 * src: - make process.dlopen() load well-known symbol (Ben Noordhuis) nodejs#18934 * trace_events: - add file pattern cli option (Andreas Madsen) nodejs#18480 * Added new collaborators: - Chen Gang (MoonBall) https://github.com/MoonBall PR-URL: nodejs#19181
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.docIssues and PRs related to the documentations.metaIssues and PRs related to the general management of the project.opensslIssues and PRs related to the OpenSSL dependency.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

20 participants

@MylesBorins@addaleax@gibfahn@nodejs-github-bot@danbev@vsemozhetbyt@bjori@mcollina@joyeecheung@killagu@devsnek@billywhizz@kfarnung@XadillaX@GeorgeSapkin@richardlau@Trott@MoonBall@ebickle@ryzokuken