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.

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

Fix cb() never called in search with --json option #2831

Closed
wants to merge 1 commit into from
Closed

Fix cb() never called in search with --json option #2831

wants to merge 1 commit into from

Conversation

fraqe
Copy link
Contributor

@fraqe fraqe commented Mar 5, 2021 •
edited

Running npm search using --json option throws a ERR! cb() never called! error. Since version 7, checked on osX and Linux.

This PR fixes it by properly calling Minipass.prototype.end() upon completion in the JSONOutputStream class.

Also add a test case to ensure that search --json output a json

@fraqe fraqe requested a review from a team as a code owner March 5, 2021 19:55
isaacs
isaacs approved these changes Mar 5, 2021
View reviewed changes
Copy link
Contributor

@isaacs isaacs left a comment

Choose a reason for hiding this comment

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

This is obviously correct. Sorry for letting the bug slip through!

@darcyclarke darcyclarke added Bug thing that needs fixing Needs Review Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release semver:patch semver patch level for changes and removed Bug thing that needs fixing labels Mar 6, 2021
wraithgar pushed a commit that referenced this pull request Mar 9, 2021
@fraqe @wraithgar
Fix cb() never called in search with --json option
8b59788 
PR-URL: #2831
Credit: @fraqe
Close: #2831
Reviewed-by: @isaacs
@wraithgar wraithgar mentioned this pull request Mar 9, 2021
Merged
@wraithgar wraithgar closed this in e0a3a52 Mar 9, 2021
@ruyadorno ruyadorno mentioned this pull request Mar 9, 2021
Closed
This was referenced Mar 10, 2021
Closed
Merged
Closed
Closed
Closed
Closed
Closed
@ruyadorno ruyadorno mentioned this pull request Mar 11, 2021
Closed
This was referenced Mar 12, 2021
Closed
Closed
Closed
Closed
Merged
Closed
Closed
This was referenced Mar 12, 2021
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
Merged
Merged
Closed
Closed
Closed
Closed
Merged
Merged
Closed
Closed
Merged
Closed
Closed
Closed
Merged
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isaacs isaacs isaacs approved these changes

Assignees
No one assigned
Labels
release: next These items should be addressed in the next release Release 7.x work is associated with a specific npm 7 release semver:patch semver patch level for changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@fraqe @isaacs @ruyadorno @darcyclarke