Update: De-duplicate error handling into file-operations

Author: erikkemperman

lib/dest/write-contents/index.js

@@ -7,6 +7,8 @@ var writeStream = require('./write-stream');
 var writeBuffer = require('./write-buffer');
 var writeSymbolicLink = require('./write-symbolic-link');
 
+var fo = require('../../file-operations');
+
 function writeContents(opt) {
 
   function writeFile(file, enc, callback) {
@@ -38,26 +40,13 @@ function writeContents(opt) {
     // This is invoked by the various writeXxx modules when they've finished
     // writing the contents.
     function onWritten(writeErr) {
-      if (isErrorFatal(writeErr)) {
+      if (fo.isFatalOverwriteError(writeErr, file.flag)) {
         return callback(writeErr);
       }
 
       callback(null, file);
     }
 
-    function isErrorFatal(err) {
-      if (!err) {
-        return false;
-      }
-
-      if (err.code === 'EEXIST' && file.flag === 'wx') {
-        // Handle scenario for file overwrite failures.
-        return false;
-      }
-
-      // Otherwise, this is a fatal error
-      return true;
-    }
   }
 
   return through.obj(writeFile);

lib/dest/write-contents/write-symbolic-link.js

@@ -2,19 +2,17 @@
 
 var fs = require('graceful-fs');
 
+var fo = require('../../file-operations');
+
 function writeSymbolicLink(file, onWritten) {
   // TODO handle symlinks properly
   fs.symlink(file.symlink, file.path, function(symlinkErr) {
-    if (isFatalError(symlinkErr)) {
+    if (fo.isFatalOverwriteError(symlinkErr)) {
       return onWritten(symlinkErr);
     }
 
     onWritten();
   });
 }
 
-function isFatalError(err) {
-  return (err && err.code !== 'EEXIST');
-}
-
 module.exports = writeSymbolicLink;

lib/file-operations.js

@@ -42,6 +42,28 @@ function isValidUnixId(id) {
   return true;
 }
 
+function isFatalOverwriteError(err, flag) {
+  if (!err) {
+    return false;
+  }
+
+  if (err.code === 'EEXIST' && flag === 'wx') {
+    // Handle scenario for file overwrite failures.
+    return false;
+  }
+
+  // Otherwise, this is a fatal error
+  return true;
+}
+
+function isFatalUnlinkError(err) {
+  if (!err || err.code === 'ENOENT') {
+    return false;
+  }
+
+  return true;
+}
+
 function getModeDiff(fsMode, vinylMode) {
   var modeDiff = 0;
 
@@ -441,6 +463,8 @@ function cleanup(callback) {
 module.exports = {
   closeFd: closeFd,
   isValidUnixId: isValidUnixId,
+  isFatalOverwriteError: isFatalOverwriteError,
+  isFatalUnlinkError: isFatalUnlinkError,
   getModeDiff: getModeDiff,
   getTimesDiff: getTimesDiff,
   getOwnerDiff: getOwnerDiff,

lib/symlink/index.js

@@ -60,32 +60,18 @@ function symlink(outFolder, opt) {
     }
 
     function onUnlink(unlinkErr) {
-      if (unlinkErr && unlinkErr.code !== 'ENOENT') {
-        return callback(unlinkErr);;
+      if (fo.isFatalUnlinkError(unlinkErr)) {
+        return callback(unlinkErr);
       }
       fs.symlink(srcPath, file.path, symType, onSymlink);
     }
 
     function onSymlink(symlinkErr) {
-      if (isErrorFatal(symlinkErr)) {
+      if (fo.isFatalOverwriteError(symlinkErr, file.flag)) {
         return callback(symlinkErr);
       }
       callback(null, file);
     }
-
-    function isErrorFatal(err) {
-      if (!err) {
-        return false;
-      }
-
-      if (err.code === 'EEXIST' && file.flag === 'wx') {
-        // Handle scenario for file overwrite failures.
-        return false;
-      }
-
-      // Otherwise, this is a fatal error
-      return true;
-    }
   }
 
   var stream = pumpify.obj(

test/file-operations.js

@@ -30,6 +30,8 @@ var getModeDiff = fo.getModeDiff;
 var getTimesDiff = fo.getTimesDiff;
 var getOwnerDiff = fo.getOwnerDiff;
 var isValidUnixId = fo.isValidUnixId;
+var isFatalOverwriteError = fo.isFatalOverwriteError;
+var isFatalUnlinkError = fo.isFatalUnlinkError;
 var updateMetadata = fo.updateMetadata;
 var createWriteStream = fo.createWriteStream;
 
@@ -170,6 +172,70 @@ describe('isValidUnixId', function() {
   });
 });
 
+describe('isFatalOverwriteError', function() {
+
+  it('returns false if not given any error', function(done) {
+    var result = isFatalOverwriteError(null);
+
+    expect(result).toEqual(false);
+
+    done();
+  });
+
+  it('returns true if code != EEXIST', function(done) {
+    var result = isFatalOverwriteError({ code: 'EOTHER' });
+
+    expect(result).toEqual(true);
+
+    done();
+  });
+
+  it('returns false if code == EEXIST and flag == wx', function(done) {
+    var result = isFatalOverwriteError({ code: 'EEXIST' }, 'wx');
+
+    expect(result).toEqual(false);
+
+    done();
+  });
+
+  it('returns true if error.code == EEXIST and file.flag != wx', function(done) {
+    var result = isFatalOverwriteError({ code: 'EEXIST' }, 'w');
+
+    expect(result).toEqual(true);
+
+    done();
+  });
+
+});
+
+describe('isFatalUnlinkError', function() {
+
+  it('returns false if not given any error', function(done) {
+    var result = isFatalUnlinkError(null);
+
+    expect(result).toEqual(false);
+
+    done();
+  });
+
+  it('returns false if code == ENOENT', function(done) {
+    var result = isFatalUnlinkError({ code: 'ENOENT' }, 'wx');
+
+    expect(result).toEqual(false);
+
+    done();
+  });
+
+  it('returns true if code != ENOENT', function(done) {
+    var result = isFatalUnlinkError({ code: 'EOTHER' });
+
+    expect(result).toEqual(true);
+
+    done();
+  });
+
+});
+
 describe('getModeDiff', function() {
 
   it('returns 0 if both modes are the same', function(done) {