Properly handle close errors in utimesMillis*() (#1069)

Fixes #1065 (comment)

Author: RyanZim

lib/util/__tests__/utimes.test.js

@@ -96,6 +96,51 @@ describe('utimes', () => {
         done()
       })
     })
+
+    it('should report close errors', done => {
+      gracefulFsStub.open = u((pathIgnored, flagsIgnored, modeIgnored, callback) => {
+        if (typeof modeIgnored === 'function') callback = modeIgnored
+        process.nextTick(() => callback(null, 42))
+      })
+
+      const closeError = new Error('close error')
+      gracefulFsStub.close = u((fd, callback) => {
+        process.nextTick(() => callback(closeError))
+      })
+
+      gracefulFsStub.futimes = u((fd, atimeIgnored, mtimeIgnored, callback) => {
+        process.nextTick(callback)
+      })
+
+      utimes.utimesMillis('ignored', 'ignored', 'ignored', err => {
+        assert.strictEqual(err, closeError)
+        done()
+      })
+    })
+
+    it('should throw futimes error even if close also errors', done => {
+      gracefulFsStub.open = u((pathIgnored, flagsIgnored, modeIgnored, callback) => {
+        if (typeof modeIgnored === 'function') callback = modeIgnored
+        process.nextTick(() => callback(null, 42))
+      })
+
+      gracefulFsStub.close = u((fd, callback) => {
+        process.nextTick(() => callback(new Error('close error')))
+      })
+
+      let testError
+      gracefulFsStub.futimes = u((fd, atimeIgnored, mtimeIgnored, callback) => {
+        process.nextTick(() => {
+          testError = new Error('A test error')
+          callback(testError)
+        })
+      })
+
+      utimes.utimesMillis('ignored', 'ignored', 'ignored', err => {
+        assert.strictEqual(err, testError)
+        done()
+      })
+    })
   })
 
   describe('utimesMillisSync()', () => {
@@ -118,5 +163,33 @@ describe('utimes', () => {
       assert.throws(() => utimes.utimesMillisSync('ignored', 'ignored', 'ignored'), testError)
       assert(closeCalled)
     })
+
+    it('should report close errors', () => {
+      gracefulFsStub.openSync = (pathIgnored, flagsIgnored) => 42
+
+      const closeError = new Error('close error')
+      gracefulFsStub.closeSync = (fd) => {
+        throw closeError
+      }
+
+      gracefulFsStub.futimesSync = (fd, atimeIgnored, mtimeIgnored) => {}
+
+      assert.throws(() => utimes.utimesMillisSync('ignored', 'ignored', 'ignored'), closeError)
+    })
+
+    it('should throw futimes error even if close also errors', () => {
+      gracefulFsStub.openSync = (pathIgnored, flagsIgnored) => 42
+
+      gracefulFsStub.closeSync = (fd) => {
+        throw new Error('close error')
+      }
+
+      const testError = new Error('A test error')
+      gracefulFsStub.futimesSync = (fd, atimeIgnored, mtimeIgnored) => {
+        throw testError
+      }
+
+      assert.throws(() => utimes.utimesMillisSync('ignored', 'ignored', 'ignored'), testError)
+    })
   })
 })

lib/util/utimes.js

@@ -16,7 +16,7 @@ async function utimesMillis (path, atime, mtime) {
     try {
       await fs.close(fd)
     } catch (closeErr) {
-      error = closeErr
+      if (!error) error = closeErr
     }
   }
 
@@ -38,7 +38,7 @@ function utimesMillisSync (path, atime, mtime) {
     try {
       fs.closeSync(fd)
     } catch (closeErr) {
-      error = closeErr
+      if (!error) error = closeErr
     }
   }