Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

stream: improve the error message of ERR_INVALID_ARG_TYPE #17145

Closed
wants to merge 1 commit into from
Closed

stream: improve the error message of ERR_INVALID_ARG_TYPE #17145

wants to merge 1 commit into from

Conversation

starkwang
Copy link
Contributor

@starkwang starkwang commented Nov 20, 2017 •
edited
Loading

The expected argument of ERR_INVALID_ARG_TYPE can be an array, which is better than a single string.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

stream

@starkwang
stream: improve the error message of ERR_INVALID_ARG_TYPE
c84b201 
The `expected` argument of `ERR_INVALID_ARG_TYPE` can be an
array, which is better than a single string.
@nodejs-github-bot nodejs-github-bot added the stream Issues and PRs related to the stream subsystem. label Nov 20, 2017
@joyeecheung
Copy link
Member

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

@joyeecheung joyeecheung added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 21, 2017
joyeecheung
joyeecheung approved these changes Nov 21, 2017
View reviewed changes
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

This does not need 48-hours. LGTM once CI is green

jasnell pushed a commit that referenced this pull request Nov 22, 2017
@starkwang @jasnell
stream: improve the error message of ERR_INVALID_ARG_TYPE
0a948b9 
The `expected` argument of `ERR_INVALID_ARG_TYPE` can be an
array, which is better than a single string.

PR-URL: #17145
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@jasnell
Copy link
Member

jasnell commented Nov 22, 2017

Landed in 0a948b9

@jasnell jasnell closed this Nov 22, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@starkwang @MylesBorins
stream: improve the error message of ERR_INVALID_ARG_TYPE
0a75c7d 
The `expected` argument of `ERR_INVALID_ARG_TYPE` can be an
array, which is better than a single string.

PR-URL: #17145
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@starkwang @MylesBorins
stream: improve the error message of ERR_INVALID_ARG_TYPE
b36c726 
The `expected` argument of `ERR_INVALID_ARG_TYPE` can be an
array, which is better than a single string.

PR-URL: #17145
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
@MylesBorins
Copy link
Contributor

Should this have been semver-major? I thought we were not landing error changes as semver-patch until the entire system was updated

My apologies if I am mistaken.

Please adjust lts labels as appropriate

@joyeecheung
Copy link
Member

joyeecheung commented Dec 20, 2017 •
edited
Loading

@MylesBorins The error message prior to the changed one was Invalid non-string/buffer chunk, then it's migrated in v9.x to something like The "chunk" argument must be of type string/Buffer/Uint8Array. Received type null, this PR just changes that again to The "chunk" argument must be one of type string, Buffer, or Uint8Array. Received type null. The error code is untouched so I think it's reasonable to backport this to 9.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joyeecheung joyeecheung joyeecheung approved these changes

@cjihrig cjihrig cjihrig approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

@apapirovski apapirovski apapirovski approved these changes

Assignees
No one assigned
Labels
fast-track PRs that do not need to wait for 48 hours to land. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@starkwang @joyeecheung @jasnell @MylesBorins @apapirovski @Trott @cjihrig @gibfahn @nodejs-github-bot