Fix/1357 silent extraction failure

src/nvm.go

@@ -412,6 +412,16 @@ func rollback(version string) error {
 			writeToErrorLog(err)
 			return fmt.Errorf("Error rolling back node v%s installation: %v.", version, err)
 		}
+
+		// Nothing to roll back - the version directory was never created.
+		return nil
+	}
+
+	// Remove the partially-installed version directory so a failed or canceled
+	// install doesn't leave a broken version behind for `nvm use` to pick up.
+	if err := os.RemoveAll(p); err != nil {
+		writeToErrorLog(err)
+		return fmt.Errorf("Error rolling back node v%s installation: %v.", version, err)
 	}
 
 	return nil
@@ -712,7 +722,8 @@ func install(version string, cpuarch string) {
 			if file.Exists(filepath.Join(root, "v"+version, "node_modules", "npm")) {
 				utility.DebugLogf("move %v to %v", filepath.Join(root, "v"+version), filepath.Join(env.root, "v"+version))
 				if rnerr := utility.Rename(filepath.Join(root, "v"+version), filepath.Join(env.root, "v"+version)); rnerr != nil {
-					status <- Status{Err: err}
+					status <- Status{Err: fmt.Errorf("failed to move node v%s into %s: %v", version, env.root, rnerr)}
+					return
 				}
 				utility.DebugFn(func() {
 					utility.DebugLogf("env root: %v", env.root)
@@ -733,6 +744,12 @@ func install(version string, cpuarch string) {
 					npmv := getNpmVersion(version)
 					status <- Status{Text: fmt.Sprintf("npm v%s installed successfully.\n\nIf you want to use this version, type\n\nnvm use %s", npmv, version), Done: true}
 				}
+
+				// npm was bundled with the Node.js archive and is already in place.
+				// Return here so we don't fall through into the standalone-npm
+				// download path below, which operates on the temp directory that
+				// was just moved out from under us.
+				return
 			}
 
 			// If successful, add npm

src/utility/rename.go

@@ -5,16 +5,48 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"time"
 )
 
+// Rename moves old to new.
+//
+// It first attempts a cheap filesystem rename, which only works when the source
+// and destination live on the same volume. That rename can still fail on
+// Windows when a file in the source tree is briefly locked right after an unzip
+// (e.g. antivirus scanning a freshly-extracted 100MB+ node.exe), so it is
+// retried with a short backoff. If the rename ultimately fails — whether because
+// the paths are on different volumes or because the directory entry simply
+// cannot be moved — it falls back to a recursive copy + delete, which opens each
+// file individually instead of relying on a single atomic directory rename.
+//
+// This makes the temp->install-root move reliable. Previously a same-volume
+// os.Rename failure was returned verbatim and aborted the install, leaving the
+// fully-extracted files stranded in the temp directory while nvm reported
+// success — the root cause of issue #1357.
 func Rename(old, new string) error {
-	old_drive := filepath.VolumeName(old)
-	new_drive := filepath.VolumeName(new)
-
-	if old_drive == new_drive {
-		return os.Rename(old, new)
+	sameVolume := filepath.VolumeName(old) == filepath.VolumeName(new)
+
+	// Fast path: a real rename is only possible within a single volume.
+	if sameVolume {
+		var err error
+		for _, backoff := range []time.Duration{0, 1, 2, 4} {
+			if backoff > 0 {
+				time.Sleep(backoff * time.Second)
+			}
+			if err = os.Rename(old, new); err == nil {
+				return nil
+			}
+		}
+		// The rename kept failing even though both paths are on the same
+		// volume. Fall through to copy + remove rather than aborting.
 	}
 
+	return copyThenRemove(old, new)
+}
+
+// copyThenRemove recursively copies old to new and then deletes old. It is used
+// both for cross-volume moves and as a fallback when an in-volume rename fails.
+func copyThenRemove(old, new string) error {
 	// Get file or directory info
 	info, err := os.Stat(old)
 	if err != nil {

src/web/web.go

@@ -285,9 +285,17 @@ func GetNodeJS(root string, v string, a string, append bool) bool {
 
 				zip := root + "\\v" + v + "\\" + strings.Replace(filepath.Base(url), ".zip", "", 1)
 				utility.DebugLogf("moving %v to %v", zip, root+"\\v"+v)
-				err = fs.Move(zip, root+"\\v"+v, true)
+				// Lift the extracted files out of the nested "node-vX-win-arch" folder
+				// into the version root. Do NOT ignore errors here: if a file fails to
+				// move (e.g. out of disk space, locked by antivirus, permission issue)
+				// we must surface it, otherwise the RemoveAll below would silently
+				// delete the un-moved files while the caller still reports a successful
+				// install (see issue #1357).
+				err = fs.Move(zip, root+"\\v"+v, false)
 				if err != nil {
-					fmt.Println("ERROR moving file: " + err.Error())
+					fmt.Println("Error extracting from Node archive (failed to move files into place): " + err.Error())
+					os.RemoveAll(root + "\\v" + v)
+					return false
 				}
 				utility.DebugLog("move succeeded")
 
@@ -306,6 +314,16 @@ func GetNodeJS(root string, v string, a string, append bool) bool {
 						utility.DebugLog(string(out))
 					}
 				})
+
+				// Verify the extraction actually produced a usable node binary before
+				// declaring success. Without this, a partial/blocked extraction is
+				// reported as "Complete" and the user ends up with an empty version
+				// directory (issue #1357).
+				if !file.Exists(root + "\\v" + v + "\\node.exe") {
+					fmt.Println("Error: extraction completed but node.exe is missing. The download may be corrupt or extraction was blocked.")
+					os.RemoveAll(root + "\\v" + v)
+					return false
+				}
 			}
 			fmt.Println("Complete")
 			return true