More robust handling of errors moving across virtual drives

lib/move/__tests__/move.test.js

@@ -2,22 +2,39 @@ var assert = require('assert')
 var os = require('os')
 var path = require('path')
 var rimraf = require('rimraf')
-var fs = require('fs')
+var fs = require('graceful-fs')
 var fse = require(process.cwd())
 
 /* global afterEach, beforeEach, describe, it */
 
 var FIXTURES_DIR = ''
 var SRC_FIXTURES_DIR = path.join(__dirname, './fixtures')
 
-// makes fs.rename return cross-device error.
-var mock_fs = {}
-mock_fs.rename = function (src, dest, callback) {
-  setTimeout(function () {
-    var err = new Error()
-    err.code = 'EXDEV'
-    callback(err)
-  }, 10)
+function createAsyncErrFn (errCode) {
+  var fn = function () {
+    fn.callCount++
+    var callback = arguments[arguments.length - 1]
+    setTimeout(function () {
+      var err = new Error()
+      err.code = errCode
+      callback(err)
+    }, 10)
+  }
+  fn.callCount = 0
+  return fn
+}
+
+var originalRename = fs.rename
+var originalLink = fs.link
+
+function setUpMockFs (errCode) {
+  fs.rename = createAsyncErrFn(errCode)
+  fs.link = createAsyncErrFn(errCode)
+}
+
+function tearDownMockFs () {
+  fs.rename = originalRename
+  fs.link = originalLink
 }
 
 describe('move', function () {
@@ -97,18 +114,17 @@ describe('move', function () {
     var src = FIXTURES_DIR + '/a-file'
     var dest = FIXTURES_DIR + '/a-file-dest'
 
-    var oldRename = fs.rename
-    fs.rename = mock_fs.rename
+    setUpMockFs('EXDEV')
 
     fse.move(src, dest, function (err) {
       assert.ifError(err)
+      assert.strictEqual(fs.link.callCount, 1)
+
       fs.readFile(dest, 'utf8', function (err, contents) {
         assert.ifError(err)
         assert.strictEqual(contents, 'sonic the hedgehog\n')
 
-        // restore
-        fs.rename = oldRename
-
+        tearDownMockFs()
         done()
       })
     })
@@ -131,21 +147,65 @@ describe('move', function () {
     })
   })
 
-  it('should move folders across devices', function (done) {
+  it('should move folders across devices with EISDIR erro', function (done) {
+    var src = FIXTURES_DIR + '/a-folder'
+    var dest = FIXTURES_DIR + '/a-folder-dest'
+
+    setUpMockFs('EISDIR')
+
+    fse.move(src, dest, function (err) {
+      assert.ifError(err)
+      assert.strictEqual(fs.link.callCount, 1)
+
+      fs.readFile(dest + '/another-folder/file3', 'utf8', function (err, contents) {
+        assert.ifError(err)
+        assert.strictEqual(contents, 'knuckles\n')
+
+        tearDownMockFs('EISDIR')
+
+        done()
+      })
+    })
+  })
+
+  it('should clobber folders across devices', function (done) {
     var src = FIXTURES_DIR + '/a-folder'
     var dest = FIXTURES_DIR + '/a-folder-dest'
 
-    var oldRename = fs.rename
-    fs.rename = mock_fs.rename
+    fs.mkdirSync(dest)
+
+    setUpMockFs('EXDEV')
+
+    fse.move(src, dest, {clobber: true}, function (err) {
+      assert.ifError(err)
+      assert.strictEqual(fs.rename.callCount, 1)
+
+      fs.readFile(dest + '/another-folder/file3', 'utf8', function (err, contents) {
+        assert.ifError(err)
+        assert.strictEqual(contents, 'knuckles\n')
+
+        tearDownMockFs('EXDEV')
+
+        done()
+      })
+    })
+  })
+
+  it('should move folders across devices with EXDEV error', function (done) {
+    var src = FIXTURES_DIR + '/a-folder'
+    var dest = FIXTURES_DIR + '/a-folder-dest'
+
+    setUpMockFs('EXDEV')
 
     fse.move(src, dest, function (err) {
       assert.ifError(err)
+      assert.strictEqual(fs.link.callCount, 1)
+
       fs.readFile(dest + '/another-folder/file3', 'utf8', function (err, contents) {
         assert.ifError(err)
         assert.strictEqual(contents, 'knuckles\n')
 
-        // restore
-        fs.rename = oldRename
+        tearDownMockFs()
 
         done()
       })

lib/move/index.js

@@ -61,17 +61,13 @@ function mv (source, dest, options, callback) {
         }
 
         if (err.code !== 'EXDEV') return callback(err)
-        moveFileAcrossDevice(source, dest, clobber, limit, callback)
+        moveAcrossDevice(source, dest, clobber, limit, callback)
       })
     } else {
       fs.link(source, dest, function (err) {
         if (err) {
-          if (err.code === 'EXDEV') {
-            moveFileAcrossDevice(source, dest, clobber, limit, callback)
-            return
-          }
-          if (err.code === 'EISDIR' || err.code === 'EPERM') {
-            moveDirAcrossDevice(source, dest, clobber, limit, callback)
+          if (err.code === 'EXDEV' || err.code === 'EISDIR' || err.code === 'EPERM') {
+            moveAcrossDevice(source, dest, clobber, limit, callback)
             return
           }
           callback(err)
@@ -83,6 +79,21 @@ function mv (source, dest, options, callback) {
   }
 }
 
+function moveAcrossDevice (source, dest, clobber, limit, callback) {
+  fs.stat(source, function (err, stat) {
+    if (err) {
+      callback(err)
+      return
+    }
+
+    if (stat.isDirectory()) {
+      moveDirAcrossDevice(source, dest, clobber, limit, callback)
+    } else {
+      moveFileAcrossDevice(source, dest, clobber, limit, callback)
+    }
+  })
+}
+
 function moveFileAcrossDevice (source, dest, clobber, limit, callback) {
   var outFlags = clobber ? 'w' : 'wx'
   var ins = fs.createReadStream(source)