FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

net: fix net.Server.prototype.listen error message by joyeecheung · Pull Request #11693 · nodejs/node · GitHub

/ node Public

net: fix net.Server.prototype.listen error message - #11693

Closed
joyeecheung wants to merge 2 commits into
nodejs:masterfrom
joyeecheung:fix-net-listen-error
Closed

net: fix net.Server.prototype.listen error message#11693
joyeecheung wants to merge 2 commits into
nodejs:masterfrom
joyeecheung:fix-net-listen-error

Conversation

joyeecheung commented Mar 5, 2017
edited
Loading

Copy link
Copy Markdown
Member

The first commit uses util.inspect to make the error messages thrown by server.listen more useful.

Before:

net.createServer().listen(null)
// Error: Invalid listen argument: [object Object]

After:

net.createServer().listen(null)
// Error: Invalid listen argument: { port: null }

The second commit refactors the listen option test, adding precise error message validation and a few more test cases.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

net, test

nodejs-github-bot added the net Issues and PRs related to the net subsystem. label Mar 5, 2017
joyeecheung added the test Issues and PRs related to the tests. label Mar 5, 2017
mscdex added semver-major PRs that contain breaking changes and should be released in the next major version. test Issues and PRs related to the tests. and removed test Issues and PRs related to the tests. labels Mar 5, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Duplicate?

Previously the error messages are mostly `[object Object]`
after the options get normalized. Use util.inspect to make
it more useful.
Refactor the listen option test, add precise
error message validation and a few more test cases.
joyeecheung force-pushed the fix-net-listen-error branch from 8ae4ffc to dbcba0d Compare March 8, 2017 07:45

Copy link
Copy Markdown
Member Author

Got a conflict after #11667 landed. Rebased.

New CI: https://ci.nodejs.org/job/node-test-pull-request/6742/

fhinkel commented Mar 9, 2017

Copy link
Copy Markdown
Contributor

Thanks. Squashed and landed in 4775942

fhinkel closed this Mar 9, 2017
fhinkel pushed a commit that referenced this pull request Mar 9, 2017
Previously the error messages are mostly `[object Object]`
after the options get normalized. Use util.inspect to make
it more useful.

Refactor the listen option test, add precise
error message validation and a few more test cases.

PR-URL: #11693
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
Previously the error messages are mostly `[object Object]`
after the options get normalized. Use util.inspect to make
it more useful.

Refactor the listen option test, add precise
error message validation and a few more test cases.

PR-URL: nodejs#11693
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
jasnell mentioned this pull request Apr 4, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

net Issues and PRs related to the net subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL