From 2eb39e801e685c8b1bf30c887ab854ae44a5ff3d Mon Sep 17 00:00:00 2001 From: cklein Date: Sat, 25 Feb 2017 03:02:52 +0100 Subject: [PATCH] Fixes #115 Fixes #117 Add node 7 to travis build matrix --- .gitignore | 1 + .travis.yml | 1 + lib/tmp.js | 48 ++++++++++++++++++++++++++++++++++---- package.json | 2 +- test/base.js | 10 ++++++++ test/file-test.js | 26 ++++++++++++++++++++- test/issue115-file-sync.js | 26 +++++++++++++++++++++ test/issue115-file.js | 27 +++++++++++++++++++++ 8 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 test/issue115-file-sync.js create mode 100644 test/issue115-file.js diff --git a/.gitignore b/.gitignore index 78f2710..275d710 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ node_modules/ .idea/ +.*.swp diff --git a/.travis.yml b/.travis.yml index 180e204..60527cb 100644 --- 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 index 26db126..5118816 100644 --- 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); } @@ -367,20 +369,20 @@ function dirSync(options) { function _prepareTmpFileRemoveCallback(name, fd, opts) { const removeCallback = _prepareRemoveCallback(function _removeCallback(fdPath) { try { - if (0 <= fdPath[0]) { + if (-1 != fdPath[0]) { fs.closeSync(fdPath[0]); } + fs.unlinkSync(fdPath[1]); } catch (e) { // 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; } } - fs.unlinkSync(fdPath[1]); }, [fd, name]); if (!opts.keep) { @@ -456,6 +458,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 index e6694df..c712a4b 100644 --- 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 (http://raszi.hu/)", "keywords": [ diff --git a/test/base.js b/test/base.js index b20bd55..580efc9 100644 --- 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 index e4209aa..a7ee1ab 100644 --- 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 new file mode 100644 index 0000000..7457612 --- /dev/null +++ b/test/issue115-file-sync.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 deletes it as per issue 115 + // https://github.com/raszi/node-tmp/issues/115 + + tmpobj = tmp.fileSync(); + fs.closeSync(tmpobj.fd); + fs.unlinkSync(tmpobj.name); + 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 new file mode 100644 index 0000000..9782196 --- /dev/null +++ b/test/issue115-file.js @@ -0,0 +1,27 @@ +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 deletes it as per issue 115 + // https://github.com/raszi/node-tmp/issues/115 + + tmp.file(function (err, name, fd, callback) { + fs.closeSync(fd); + fs.unlinkSync(name); + callback(); + spawn.out(name, spawn.exit); + }); + } catch (e) { + spawn.err(e.toString() + ' with code: ' + e.code + ' and errno: ' + e.errno, spawn.exit); + } +}); +