coreybutler / nvm-windows

A node.js version management utility for Windows. Ironically written in Go.
MIT License
37.36k stars 3.32k forks source link

"nvm use" and" nvm install" set a successful exit code when failing #738

Open daxelrod opened 2 years ago

daxelrod commented 2 years ago

Issue:

When nvm use is passed a non-installed version of Node, it correctly outputs an error message, but then sets its exit code 0, indicating success. Similarlly, when nvm install is passed an unrecognized version, it correctly outputs an error message, but also sets exit code 0, indicating success.

These make using nvm in a CI environment, or as part of any other automation, difficult because it is not easy to detect when they've failed.

How To Reproduce:

On Windows 10 21H1:

  1. Uninstall all versions of Node
  2. Install nvm via nvm-setup
  3. Run Windows PowerShell as Administrator:
PS C:\WINDOWS\system32> nvm version
1.1.9
PS C:\WINDOWS\system32> nvm list

No installations recognized.
PS C:\WINDOWS\system32> nvm use 14
node v14.18.3 (64-bit) is not installed or cannot be found.
PS C:\WINDOWS\system32> echo $LASTEXITCODE
0
PS C:\WINDOWS\system32> nvm install carrot
Unrecognized version: "carrot"

Running version 1.1.9.

Usage:

  nvm arch                     : Show if node is running in 32 or 64 bit mode.
  nvm current                  : Display active version.
  nvm install <version> [arch] : The version can be a specific version, "latest" for the latest current version, or "lts" for the
                                 most recent LTS version. Optionally specify whether to install the 32 or 64 bit version (defaults
                                 to system arch). Set [arch] to "all" to install 32 AND 64 bit versions.
                                 Add --insecure to the end of this command to bypass SSL validation of the remote download server.
  nvm list [available]         : List the node.js installations. Type "available" at the end to see what can be installed. Aliased as ls.
  nvm on                       : Enable node.js version management.
  nvm off                      : Disable node.js version management.
  nvm proxy [url]              : Set a proxy to use for downloads. Leave [url] blank to see the current proxy.
                                 Set [url] to "none" to remove the proxy.
  nvm node_mirror [url]        : Set the node mirror. Defaults to https://nodejs.org/dist/. Leave [url] blank to use default url.
  nvm npm_mirror [url]         : Set the npm mirror. Defaults to https://github.com/npm/cli/archive/. Leave [url] blank to default url.
  nvm uninstall <version>      : The version must be a specific version.
  nvm use [version] [arch]     : Switch to use the specified version. Optionally use "latest", "lts", or "newest".
                                 "newest" is the latest installed version. Optionally specify 32/64bit architecture.
                                 nvm use <arch> will continue using the selected version, but switch to 32/64 bit mode.
  nvm root [path]              : Set the directory where nvm should store different versions of node.js.
                                 If <path> is not set, the current root will be displayed.
  nvm version                  : Displays the current running version of nvm for Windows. Aliased as v.

PS C:\WINDOWS\system32> echo $LASTEXITCODE
0

Expected Behavior:

I would expect any failure ofinstallor use to result in the exit code being anything other than 0, to indicate failure.

PS C:\WINDOWS\system32> nvm use 14
node v14.18.3 (64-bit) is not installed or cannot be found.
PS C:\WINDOWS\system32> echo $LASTEXITCODE
1
PS C:\WINDOWS\system32> nvm install carrot
Unrecognized version: "carrot"
...
PS C:\WINDOWS\system32> echo $LASTEXITCODE
1

Thank You:

nvm-windows is an invaluable part of the Node ecosystem on Windows. Thank you very much for creating and maintaining it!

coreybutler commented 2 years ago

This seems reasonable. I'll look into this in the next round of updates. If someone wants to submit a PR before then, it would save some time.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

daxelrod commented 2 years ago

Apologies that I have not had an opportunity to work on a PR for this. Commenting to make sure that this issue doesn't get closed for being stale specifically because you mentioned interest in it. Thanks!

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] commented 2 years ago

This issue was closed because it has been stalled for 7 days with no activity.

mcdurdin commented 2 months ago

I encountered this issue as well today, are there plans to fix it? The workaround for me currently is to run node --version after nvm use and parse the result, but that seems unnecessarily complicated!

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

(I know this is a philosophical issue but I think you need to get rid of the bot that pretends that things are fixed when they aren't: it's hostile to users 😭)

coreybutler commented 2 months ago

@mcdurdin I've tried to configure that bot more times than I can count. It's not perfect, but it does close a lot of abandoned issues that I request more information for and never get.

I'll re-open this so I'll remember it for the next release. It may be a while though, seeing as our code-signing cert got locked and I'm still trying to get it unlocked with the vendor.

mcdurdin commented 2 months ago

@coreybutler Thank you!