Skip to content

[v9.x] backport 17338, 18186, 18358, 18546 - #18916

Closed
joyeecheung wants to merge 5 commits into
nodejs:v9.x-stagingfrom
joyeecheung:backport-sys-errors
Closed

[v9.x] backport 17338, 18186, 18358, 18546#18916
joyeecheung wants to merge 5 commits into
nodejs:v9.x-stagingfrom
joyeecheung:backport-sys-errors

Conversation

@joyeecheung

Copy link
Copy Markdown
Member

Only two commits from #18546 are backported. The remaining one is related to the fs error migration and should be backported differently, if worth backporting at all.

src: expose uv.errmap to binding
Refs: #17338

util: implement util.getSystemErrorName()
Refs: #18186

errors: improve the description of ERR_INVALID_ARG_VALUE
Refs: #18358

util: skip type checks in internal getSystemErrorName
errors: move error creation helpers to errors.js
Refs: #18546

@nodejs-github-botnodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v9.x labels Feb 21, 2018
@joyeecheung

Copy link
Copy Markdown
MemberAuthor

cc @MylesBorins

@joyeecheung

Copy link
Copy Markdown
MemberAuthor

Add a errno -> [error code, uv error message] map to the uv binding
so the error message can be assembled in the JS layer.
PR-URL: nodejs#17338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reimplement uv.errname() as internal/util.getSystemErrorName() to
avoid the memory leaks caused by unknown error codes
and avoid calling into C++ for the error names. Also
expose it as a public API for external use.
PR-URL: nodejs#18186
Refs: http://docs.libuv.org/en/v1.x/errors.html#c.uv_err_name
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
- Allow user to customize why the argument is invalid
- Display the argument with util.inspect so null bytes can be
displayed properly.
PR-URL: nodejs#18358
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
PR-URL: nodejs#18546
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit moves error creation helpers scattered around
under lib/ into lib/internal/errors.js in the hope of being clearer
about the differences of errors that we throw into the user land.
- Move util._errnoException and util._exceptionWithHostPort
into internal/errors.js and simplify their logic so it's
clearer what the properties these helpers create.
- Move the errnoException helper in dns.js to internal/errors.js
into internal/errors.js and rename it to dnsException. Simplify
it's logic so it no longer calls errnoException and skips
the unnecessary argument checks.
PR-URL: nodejs#18546
Reviewed-By: James M Snell <jasnell@gmail.com>
@joyeecheung

Copy link
Copy Markdown
MemberAuthor

There was a force update in the base branch. Rebased and new CI: https://ci.nodejs.org/job/node-test-pull-request/13326/

@joyeecheung

joyeecheung commented Feb 22, 2018

Copy link
Copy Markdown
MemberAuthor

URL: https://ci.nodejs.org/job/node-test-commit-linux/nodes=centos7-64/16611/console
Reason:

 not ok 1958 sequential/test-fs-readfile-tostring-fail
---
duration_ms: 5.941
severity: crashed
stack: |-
oh no!
exit code: CRASHED (Signal: 9)
...

Only a known flake on the CI..

@joyeecheung

Copy link
Copy Markdown
MemberAuthor

By the way, #18186 here is semver-minor.

@addaleaxaddaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 26, 2018
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
Add a errno -> [error code, uv error message] map to the uv binding
so the error message can be assembled in the JS layer.
Backport-PR-URL: #18916
PR-URL: #17338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
Reimplement uv.errname() as internal/util.getSystemErrorName() to
avoid the memory leaks caused by unknown error codes
and avoid calling into C++ for the error names. Also
expose it as a public API for external use.
Backport-PR-URL: #18916
PR-URL: #18186
Refs: http://docs.libuv.org/en/v1.x/errors.html#c.uv_err_name
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
- Allow user to customize why the argument is invalid
- Display the argument with util.inspect so null bytes can be
displayed properly.
Backport-PR-URL: #18916
PR-URL: #18358
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
Backport-PR-URL: #18916
PR-URL: #18546
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
This commit moves error creation helpers scattered around
under lib/ into lib/internal/errors.js in the hope of being clearer
about the differences of errors that we throw into the user land.
- Move util._errnoException and util._exceptionWithHostPort
into internal/errors.js and simplify their logic so it's
clearer what the properties these helpers create.
- Move the errnoException helper in dns.js to internal/errors.js
into internal/errors.js and rename it to dnsException. Simplify
it's logic so it no longer calls errnoException and skips
the unnecessary argument checks.
Backport-PR-URL: #18916
PR-URL: #18546
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins

Copy link
Copy Markdown
Contributor

landed in 562fdb5...19c46aa

All failures in CI seem to be related to other flakes.

MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
Add a errno -> [error code, uv error message] map to the uv binding
so the error message can be assembled in the JS layer.
Backport-PR-URL: #18916
PR-URL: #17338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
Reimplement uv.errname() as internal/util.getSystemErrorName() to
avoid the memory leaks caused by unknown error codes
and avoid calling into C++ for the error names. Also
expose it as a public API for external use.
Backport-PR-URL: #18916
PR-URL: #18186
Refs: http://docs.libuv.org/en/v1.x/errors.html#c.uv_err_name
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
- Allow user to customize why the argument is invalid
- Display the argument with util.inspect so null bytes can be
displayed properly.
Backport-PR-URL: #18916
PR-URL: #18358
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
Backport-PR-URL: #18916
PR-URL: #18546
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
This commit moves error creation helpers scattered around
under lib/ into lib/internal/errors.js in the hope of being clearer
about the differences of errors that we throw into the user land.
- Move util._errnoException and util._exceptionWithHostPort
into internal/errors.js and simplify their logic so it's
clearer what the properties these helpers create.
- Move the errnoException helper in dns.js to internal/errors.js
into internal/errors.js and rename it to dnsException. Simplify
it's logic so it no longer calls errnoException and skips
the unnecessary argument checks.
Backport-PR-URL: #18916
PR-URL: #18546
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / srcIssues and PRs related to general changes in the lib or src directory.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@joyeecheung@MylesBorins@addaleax@nodejs-github-bot