Retry after rename EPERM error (#119)

Author: jasongin

lib/addRemove.js

@@ -142,7 +142,7 @@ function downloadAndExtractAsync(version) {
 		childNames.forEach(childName => {
 			let oldPath = path.join(extractedDirPath, childName);
 			let newPath = path.join(targetDir, childName);
-			fs.renameSync(oldPath, newPath);
+			renameWithRetry(oldPath, newPath);
 		});
 
 		// Remove the now-empty directory.
@@ -215,6 +215,38 @@ function remove(version) {
 	return result;
 }
 
+/**
+ * Synchronously renames a file or directory, retrying to handle
+ * occasional 'EPERM' or 'EACCS' errors.
+ */
+function renameWithRetry(from, to) {
+	// Drived/simplified from https://github.com/isaacs/node-graceful-fs/pull/119
+	let backoff = 0;
+	const backoffUntil = Date.now() + 5000;
+	function tryRename() {
+		try {
+			fs.renameSync(from, to);
+		} catch (e) {
+			if (!isWindows) {
+				// The retry with backoff is only applicable to Windows.
+				throw e;
+			} else if ((e.code === 'EACCS' || e.code === 'EPERM') && Date.now() < backoffUntil) {
+				if (backoff < 100) {
+					backoff += 10;
+				}
+				const waitUntil = Date.now() + backoff;
+				while (Date.now() < waitUntil) {}
+				tryRename();
+			} else if (backoff > 0 && e.code === 'ENOENT') {
+				// The source no longer exists; assume it was renamed.
+			} else {
+				throw e;
+			}
+		}
+	}
+	tryRename();
+}
+
 /**
  * Creates a hierarchy of directories as necessary.
  */
@@ -286,10 +318,9 @@ async function fixNpmCmdShimsAsync(targetDir) {
 	if (!isWindows) return;
 
 	try {
-		// This assumes the npm version carried with the node installation
-		// includes a `cmd-shim` module. Currently true for at least npm >= 3.
 		const cmdShimPath = path.join(
 			targetDir, 'node_modules', 'npm', 'node_modules', 'cmd-shim');
+		fs.statSync(cmdShimPath);
 		const cmdShim = require(cmdShimPath);
 
 		// Enumerate .cmd files in the target directory and fix if they are shims.
@@ -317,6 +348,12 @@ async function fixNpmCmdShimsAsync(targetDir) {
 			});
 		}
 	} catch (e) {
+		if (e.code === 'ENOENT') {
+			// Currently all npm >= 3 include the cmd-shim module, but maybe
+			// someday it won't? Also it does not exist with test mocking.
+			return;
+		}
+
 		// Not a fatal error. Most things still work if the shims are not fixed.
 		// The only problem may be that the global npm package cannot be upgraded.
 		console.warn('Warning: Failed to fix npm cmd shims: ' + e.message);
@@ -326,4 +363,5 @@ async function fixNpmCmdShimsAsync(targetDir) {
 module.exports = {
 	addAsync,
 	remove,
+	renameWithRetry,
 };

test/mocks/fs.js

@@ -12,6 +12,7 @@ const mockFs = {
 	statMap: {},
 	dataMap: {},
 	unlinkPaths: [],
+	nextRenameError: null,
 
 	reset() {
 		this.trace = false;
@@ -183,6 +184,12 @@ const mockFs = {
 		newPath = this.fixSep(newPath);
 		if (this.trace) console.log('renameSync(' + oldPath, newPath + ')');
 
+		if (this.nextRenameError) {
+			const e = this.nextRenameError;
+			this.nextRenameError = null;
+			throw e;
+		}
+
 		if (this.dirMap[oldPath]) {
 			// Support for renaming directories is limited to a single level.
 			// Subdirectory paths currently do not get updated.

test/modules/addRemoveTests.js

@@ -5,6 +5,8 @@ const test = require('ava').test;
 const rewire = require('rewire');
 const Error = require('../../lib/error');
 
+const isWindows = process.platform === 'win32';
+
 test.before(require('../checkNodeVersion'));
 
 const testHome = '/home/test/nvs/'.replace(/\//g, path.sep);
@@ -157,6 +159,11 @@ test('Add - download', t => {
 		},
 	});
 
+	if (isWindows) {
+		// Simulate occasional EPERM during rename, which should be handled by a retry.
+		mockFs.nextRenameError = new Error('Test rename error', 'EPERM');
+	}
+
 	return nvsAddRemove.addAsync(version).then(message => {
 		t.regex(message[0], /^Added at/);
 		t.truthy(nvsUse.getVersionBinary(version));