Merge pull request #119 from raszi/gh-115

raszi · web-flow · commit ce1fad91983a · 2017-04-10T15:30:00.000+02:00
Fixes #115
diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,3 @@
 node_modules/
 .idea/
+.*.swp
diff --git a/.travis.yml b/.travis.yml
@@ -23,6 +23,7 @@ node_js:
   - "5.11"
   - "6.0"
   - "6.1"
+  - "7"
   - "node"
 sudo: false
 cache:
diff --git a/lib/tmp.js b/lib/tmp.js
@@ -233,7 +233,9 @@ function file(options, callback) {
             try {
               fs.unlinkSync(name);
             } catch (e) {
-              err = e;
+              if (!isENOENT(e)) {
+                err = e;
+              }
             }
             return cb(err);
           }
@@ -375,12 +377,20 @@ function _prepareTmpFileRemoveCallback(name, fd, opts) {
       // under some node/windows related circumstances, a temporary file
       // may have not be created as expected or the file was already closed
       // by the user, in which case we will simply ignore the error
-      if (e.errno != -EBADF && e.errno != -ENOENT) {
+      if (!isEBADF(e) && !isENOENT(e)) {
+        // reraise any unanticipated error
+        throw e;
+      }
+    }
+    try {
+      fs.unlinkSync(fdPath[1]);
+    }
+    catch (e) {
+      if (!isENOENT(e)) {
         // reraise any unanticipated error
         throw e;
       }
     }
-    fs.unlinkSync(fdPath[1]);
   }, [fd, name]);
 
   if (!opts.keep) {
@@ -456,6 +466,44 @@ function _garbageCollector() {
   }
 }
 
+/**
+ * Helper for testing against EBADF to compensate changes made to Node 7.x under Windows.
+ */
+function isEBADF(error) {
+  return isExpectedError(error, -EBADF, 'EBADF');
+}
+
+/**
+ * Helper for testing against ENOENT to compensate changes made to Node 7.x under Windows.
+ */
+function isENOENT(error) {
+  return isExpectedError(error, -EBADF, 'EBADF');
+}
+
+/**
+ * Helper to determine whether the expected error code matches the actual code and errno,
+ * which will differ between the supported node versions.
+ *
+ * - Node >= 7.0:
+ *   error.code {String}
+ *   error.errno {String|Number} any numerical value will be negated
+ *
+ * - Node >= 6.0 < 7.0:
+ *   error.code {String}
+ *   error.errno {Number} negated
+ *
+ * - Node >= 4.0 < 6.0: introduces SystemError
+ *   error.code {String}
+ *   error.errno {Number} negated
+ *
+ * - Node >= 0.10 < 4.0:
+ *   error.code {Number} negated
+ *   error.errno n/a
+ */
+function isExpectedError(error, code, errno) {
+  return error.code == code || error.code == errno;
+}
+
 /**
  * Sets the graceful cleanup.
  *
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "tmp",
-  "version": "0.0.31",
+  "version": "0.0.32",
   "description": "Temporary file and directory creator",
   "author": "KARASZI István <github@spam.raszi.hu> (http://raszi.hu/)",
   "keywords": [
diff --git a/test/base.js b/test/base.js
@@ -192,6 +192,14 @@ function _testIssue62Sync(cb) {
   _spawnTestWithoutError('issue62-sync.js', [], cb);
 }
 
+function _testIssue115File(cb) {
+  _spawnTestWithError('issue115-file.js', [], cb);
+}
+
+function _testIssue115FileSync(cb) {
+  _spawnTestWithError('issue115-file-sync.js', [], cb);
+}
+
 module.exports.testStat = _testStat;
 module.exports.testPrefix = _testPrefix;
 module.exports.testPrefixSync = _testPrefixSync;
@@ -208,5 +216,7 @@ module.exports.testName = _testName;
 module.exports.testNameSync = _testNameSync;
 module.exports.testUnsafeCleanup = _testUnsafeCleanup;
 module.exports.testIssue62 = _testIssue62;
+module.exports.testIssue115File = _testIssue115File;
+module.exports.testIssue115FileSync = _testIssue115FileSync;
 module.exports.testUnsafeCleanupSync = _testUnsafeCleanupSync;
 module.exports.testIssue62Sync = _testIssue62Sync;
diff --git a/test/file-test.js b/test/file-test.js
@@ -242,6 +242,30 @@ vows.describe('File creation').addBatch({
       removeCallback();
       assert.ok(!existsSync(name), 'File should be removed');
     }
-  }
+  },
+
+  'issue115 async: user deleted tmp file prior to cleanup': {
+    topic: function () {
+      Test.testIssue115File(this.callback);
+    },
+
+    'should not return with an error': assert.isNull,
+    'should return with a name': Test.assertName,
+    'should not exist': function (err, name) {
+      assert.ok(!existsSync(name), 'File should be removed');
+    }
+  },
+
+  'issue115 sync: user deleted tmp file prior to cleanup': {
+    topic: function () {
+      Test.testIssue115FileSync(this.callback);
+    },
+
+    'should not return with an error': assert.isNull,
+    'should return with a name': Test.assertName,
+    'should not exist': function (err, name) {
+      assert.ok(!existsSync(name), 'File should be removed');
+    }
+  },
 
 }).exportTo(module);
diff --git a/test/issue115-file-sync.js b/test/issue115-file-sync.js
@@ -0,0 +1,25 @@
+var
+  fs    = require('fs'),
+  join  = require('path').join,
+  tmp   = require('../lib/tmp'),
+  spawn = require('./spawn');
+
+spawn.tmpFunction({ unsafeCleanup: true }, function (err, name) {
+  if (err) {
+    spawn.err(err, spawn.exit);
+    return;
+  }
+
+  try {
+    // creates a tmp file and then closes the file descriptor as per issue 115
+    // https://github.com/raszi/node-tmp/issues/115
+
+    tmpobj = tmp.fileSync();
+    fs.closeSync(tmpobj.fd);
+    tmpobj.removeCallback();
+    spawn.out(tmpobj.name, spawn.exit);
+  } catch (e) {
+    spawn.err(e.toString() + ' with code: ' + e.code + ' and errno: ' + e.errno, spawn.exit);
+  }
+});
+
diff --git a/test/issue115-file.js b/test/issue115-file.js
@@ -0,0 +1,26 @@
+var
+  fs    = require('fs'),
+  join  = require('path').join,
+  tmp   = require('../lib/tmp'),
+  spawn = require('./spawn');
+
+spawn.tmpFunction({ unsafeCleanup: true }, function (err, name) {
+  if (err) {
+    spawn.err(err, spawn.exit);
+    return;
+  }
+
+  try {
+    // creates a tmp file and then closes the file descriptor it as per issue 115
+    // https://github.com/raszi/node-tmp/issues/115
+
+    tmp.file(function (err, name, fd, callback) {
+      fs.closeSync(fd);
+      callback();
+      spawn.out(name, spawn.exit);
+    });
+  } catch (e) {
+    spawn.err(e.toString() + ' with code: ' + e.code + ' and errno: ' + e.errno, spawn.exit);
+  }
+});
+

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
`1`	`1`	`node_modules/`
`2`	`2`	`.idea/`
	`3`	`+.*.swp`
-Original file line number
+Diff line change
   - "5.11"
   - "6.0"
   - "6.1"
 +  - "7"
   - "node"
 sudo: false
 cache:
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "tmp",`
`3`		`- "version": "0.0.31",`
	`3`	`+ "version": "0.0.32",`
`4`	`4`	`"description": "Temporary file and directory creator",`
`5`	`5`	`"author": "KARASZI István <github@spam.raszi.hu> (http://raszi.hu/)",`
`6`	`6`	`"keywords": [`