-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix(view): error on missing version #5035
Conversation
Co-authored-by: Jordan Harband <ljharb@gmail.com>
One space too many Co-authored-by: Jordan Harband <ljharb@gmail.com>
This is a continuation of #5011, I fixed the tests. |
no statistically significant performance changes detected timing results
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about a dist-tag that doesn’t exist?
This PR also fixes that situation ~/D/n/cli (gar/5011|✔) $ node . view lodash@asdf
npm ERR! code E404
npm ERR! 404 No match found for version asdf
npm ERR! 404
npm ERR! 404 'lodash@asdf' is not in this registry. |
npm changed the way the `npm view` command handles missing versions. Before it exited with a non-error. Now it errors. Ref: npm/cli#5035 This modifies the script logic to handle those new changes.
* chore: clean up logging in npm script * fix: catch error if npm version missing npm changed the way the `npm view` command handles missing versions. Before it exited with a non-error. Now it errors. Ref: npm/cli#5035 This modifies the script logic to handle those new changes.
@wraithgar Probably not worth a new issue but I wanted to note that this caused a We have since recovered and worked around the issue and ensured the Specifically we were checking if a version had been published to npm via the CLI using: npm show electron@1.2.3 --json And assuming that "no output" meant the version had not been published and "valid output" could be parsed into the registry metadata. In both cases since the dawn of npm the command exited cleanly, this change caused a non-zero exit code in the unpublished scenario which then propagated as an error through our release scripts causing an infinite backoff-and-retry (as we assumed a non-zero exit code meant npm was having issues). Not sure if this is even gonna scratch on the radar, but I'd consider a change in exit code semantics for a CLI tool a breaking change and would be reserved for the next major version of npm. |
This fixes an error in npm show. When calling npm show with a specific
version of a package that does not exist, it does not show anything and
gives a zero exit code. This has been changed: now it gives a 404 Error
similar to if the package does not exist. Can be tested with npm show
express@5.0.0 (local: node bin/npm-cli.js info express@5.0.0)
Fixes #4964
Credit: @lukaskuhn-lku