fixup! fixup! fixup! fixup! fixup! fs: fix rmsync error swallowing

lib/internal/fs/rimraf.js

@@ -222,7 +222,7 @@ function _unlinkSync(path, options) {
       } else if (err.code === 'ENOENT') {
         // The file is already gone.
         return;
-      } else {
+      } else if (i === tries) {
         throw err;
       }
     }
@@ -267,7 +267,7 @@ function _rmdirSync(path, options, originalErr) {
           } else if (err.code === 'ENOENT') {
             // The file is already gone.
             return;
-          } else {
+          } else if (i === tries) {
             throw err;
           }
         }

test/parallel/test-fs-mkdir-recursive-eaccess.js

@@ -26,8 +26,7 @@ function makeDirectoryReadOnly(dir) {
   let accessErrorCode = 'EACCES';
   if (common.isWindows) {
     accessErrorCode = 'EPERM';
-    execSync(`icacls ${dir} /inheritance:r`);
-    execSync(`icacls ${dir} /deny "everyone":W`);
+    execSync(`icacls ${dir} /deny "everyone:(OI)(CI)(DE,DC,AD,WD)"`);
   } else {
     fs.chmodSync(dir, '444');
   }
@@ -36,7 +35,7 @@ function makeDirectoryReadOnly(dir) {
 
 function makeDirectoryWritable(dir) {
   if (common.isWindows) {
-    execSync(`icacls ${dir} /grant "everyone":W`);
+    execSync(`icacls ${dir} /remove:d "everyone"`);
   }
 }
 

test/parallel/test-fs-open-no-close.js

@@ -15,10 +15,17 @@ const debuglog = (arg) => {
 const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
 
-{
-  fs.open(`${tmpdir.path}/dummy`, 'wx+', common.mustCall((err, fd) => {
-    debuglog('fs open() callback');
-    assert.ifError(err);
-  }));
-  debuglog('waiting for callback');
-}
+let openFd;
+
+fs.open(`${tmpdir.path}/dummy`, 'wx+', common.mustCall((err, fd) => {
+  debuglog('fs open() callback');
+  assert.ifError(err);
+  openFd = fd;
+}));
+debuglog('waiting for callback');
+
+process.on('beforeExit', common.mustCall(() => {
+  if (openFd) {
+    fs.closeSync(openFd);
+  }
+}));

test/parallel/test-fs-promises-file-handle-read-worker.js

@@ -19,16 +19,20 @@ if (isMainThread || !workerData) {
       transferList: [handle]
     });
   });
-  fs.promises.open(file, 'r').then((handle) => {
-    fs.createReadStream(null, { fd: handle });
-    assert.throws(() => {
-      new Worker(__filename, {
-        workerData: { handle },
-        transferList: [handle]
+  fs.promises.open(file, 'r').then(async (handle) => {
+    try {
+      fs.createReadStream(null, { fd: handle });
+      assert.throws(() => {
+        new Worker(__filename, {
+          workerData: { handle },
+          transferList: [handle]
+        });
+      }, {
+        code: 25,
       });
-    }, {
-      code: 25,
-    });
+    } finally {
+      await handle.close();
+    }
   });
 } else {
   let output = '';

test/parallel/test-fs-promises-file-handle-readFile.js

@@ -56,13 +56,16 @@ async function doReadAndCancel() {
   {
     const filePathForHandle = path.resolve(tmpDir, 'dogs-running.txt');
     const fileHandle = await open(filePathForHandle, 'w+');
-    const buffer = Buffer.from('Dogs running'.repeat(10000), 'utf8');
-    fs.writeFileSync(filePathForHandle, buffer);
-    const signal = AbortSignal.abort();
-    await assert.rejects(readFile(fileHandle, { signal }), {
-      name: 'AbortError'
-    });
-    await fileHandle.close();
+    try {
+      const buffer = Buffer.from('Dogs running'.repeat(10000), 'utf8');
+      fs.writeFileSync(filePathForHandle, buffer);
+      const signal = AbortSignal.abort();
+      await assert.rejects(readFile(fileHandle, { signal }), {
+        name: 'AbortError'
+      });
+    } finally {
+      await fileHandle.close();
+    }
   }
 
   // Signal aborted on first tick

test/parallel/test-fs-promises-file-handle-writeFile.js

@@ -30,13 +30,17 @@ async function validateWriteFile() {
 async function doWriteAndCancel() {
   const filePathForHandle = path.resolve(tmpDir, 'dogs-running.txt');
   const fileHandle = await open(filePathForHandle, 'w+');
-  const buffer = Buffer.from('dogs running'.repeat(512 * 1024), 'utf8');
-  const controller = new AbortController();
-  const { signal } = controller;
-  process.nextTick(() => controller.abort());
-  await assert.rejects(writeFile(fileHandle, buffer, { signal }), {
-    name: 'AbortError'
-  });
+  try {
+    const buffer = Buffer.from('dogs running'.repeat(512 * 1024), 'utf8');
+    const controller = new AbortController();
+    const { signal } = controller;
+    process.nextTick(() => controller.abort());
+    await assert.rejects(writeFile(fileHandle, buffer, { signal }), {
+      name: 'AbortError'
+    });
+  } finally {
+    await fileHandle.close();
+  }
 }
 
 validateWriteFile()

test/parallel/test-fs-promises.js

@@ -90,6 +90,18 @@ async function getHandle(dest) {
   return open(dest, 'r+');
 }
 
+async function executeOnHandle(dest, func) {
+  let handle;
+  try {
+    handle = await getHandle(dest);
+    await func(handle);
+  } finally {
+    if (handle) {
+      await handle.close();
+    }
+  }
+}
+
 {
   async function doTest() {
     tmpdir.refresh();
@@ -98,140 +110,138 @@ async function getHandle(dest) {
 
     // handle is object
     {
-      const handle = await getHandle(dest);
-      assert.strictEqual(typeof handle, 'object');
-      await handle.close();
+      await executeOnHandle(dest, async (handle) => {
+        assert.strictEqual(typeof handle, 'object');
+      });
     }
 
     // file stats
     {
-      const handle = await getHandle(dest);
-      let stats = await handle.stat();
-      verifyStatObject(stats);
-      assert.strictEqual(stats.size, 35);
+      await executeOnHandle(dest, async (handle) => {
+        let stats = await handle.stat();
+        verifyStatObject(stats);
+        assert.strictEqual(stats.size, 35);
 
-      await handle.truncate(1);
+        await handle.truncate(1);
 
-      stats = await handle.stat();
-      verifyStatObject(stats);
-      assert.strictEqual(stats.size, 1);
+        stats = await handle.stat();
+        verifyStatObject(stats);
+        assert.strictEqual(stats.size, 1);
 
-      stats = await stat(dest);
-      verifyStatObject(stats);
+        stats = await stat(dest);
+        verifyStatObject(stats);
 
-      stats = await handle.stat();
-      verifyStatObject(stats);
+        stats = await handle.stat();
+        verifyStatObject(stats);
 
-      await handle.datasync();
-      await handle.sync();
-      await handle.close();
+        await handle.datasync();
+        await handle.sync();
+      });
     }
 
     // Test fs.read promises when length to read is zero bytes
     {
       const dest = path.resolve(tmpDir, 'test1.js');
-      const handle = await getHandle(dest);
-      const buf = Buffer.from('DAWGS WIN');
-      const bufLen = buf.length;
-      await handle.write(buf);
-      const ret = await handle.read(Buffer.alloc(bufLen), 0, 0, 0);
-      assert.strictEqual(ret.bytesRead, 0);
-
-      await unlink(dest);
-      await handle.close();
+      await executeOnHandle(dest, async (handle) => {
+        const buf = Buffer.from('DAWGS WIN');
+        const bufLen = buf.length;
+        await handle.write(buf);
+        const ret = await handle.read(Buffer.alloc(bufLen), 0, 0, 0);
+        assert.strictEqual(ret.bytesRead, 0);
+
+        await unlink(dest);
+      });
     }
 
     // Use fallback buffer allocation when input not buffer
     {
-      const handle = await getHandle(dest);
-      const ret = await handle.read(0, 0, 0, 0);
-      assert.strictEqual(ret.buffer.length, 16384);
+      await executeOnHandle(dest, async (handle) => {
+        const ret = await handle.read(0, 0, 0, 0);
+        assert.strictEqual(ret.buffer.length, 16384);
+      });
     }
 
     // Bytes written to file match buffer
     {
-      const handle = await getHandle(dest);
-      const buf = Buffer.from('hello fsPromises');
-      const bufLen = buf.length;
-      await handle.write(buf);
-      const ret = await handle.read(Buffer.alloc(bufLen), 0, bufLen, 0);
-      assert.strictEqual(ret.bytesRead, bufLen);
-      assert.deepStrictEqual(ret.buffer, buf);
-      await handle.close();
+      await executeOnHandle(dest, async (handle) => {
+        const buf = Buffer.from('hello fsPromises');
+        const bufLen = buf.length;
+        await handle.write(buf);
+        const ret = await handle.read(Buffer.alloc(bufLen), 0, bufLen, 0);
+        assert.strictEqual(ret.bytesRead, bufLen);
+        assert.deepStrictEqual(ret.buffer, buf);
+      });
     }
 
     // Truncate file to specified length
     {
-      const handle = await getHandle(dest);
-      const buf = Buffer.from('hello FileHandle');
-      const bufLen = buf.length;
-      await handle.write(buf, 0, bufLen, 0);
-      const ret = await handle.read(Buffer.alloc(bufLen), 0, bufLen, 0);
-      assert.strictEqual(ret.bytesRead, bufLen);
-      assert.deepStrictEqual(ret.buffer, buf);
-      await truncate(dest, 5);
-      assert.deepStrictEqual((await readFile(dest)).toString(), 'hello');
-      await handle.close();
+      await executeOnHandle(dest, async (handle) => {
+        const buf = Buffer.from('hello FileHandle');
+        const bufLen = buf.length;
+        await handle.write(buf, 0, bufLen, 0);
+        const ret = await handle.read(Buffer.alloc(bufLen), 0, bufLen, 0);
+        assert.strictEqual(ret.bytesRead, bufLen);
+        assert.deepStrictEqual(ret.buffer, buf);
+        await truncate(dest, 5);
+        assert.deepStrictEqual((await readFile(dest)).toString(), 'hello');
+      });
     }
 
     // Invalid change of ownership
     {
-      const handle = await getHandle(dest);
+      await executeOnHandle(dest, async (handle) => {
+        await chmod(dest, 0o666);
+        await handle.chmod(0o666);
 
-      await chmod(dest, 0o666);
-      await handle.chmod(0o666);
+        await chmod(dest, (0o10777));
+        await handle.chmod(0o10777);
 
-      await chmod(dest, (0o10777));
-      await handle.chmod(0o10777);
-
-      if (!common.isWindows) {
-        await chown(dest, process.getuid(), process.getgid());
-        await handle.chown(process.getuid(), process.getgid());
-      }
-
-      assert.rejects(
-        async () => {
-          await chown(dest, 1, -2);
-        },
-        {
-          code: 'ERR_OUT_OF_RANGE',
-          name: 'RangeError',
-          message: 'The value of "gid" is out of range. ' +
-                  'It must be >= -1 && <= 4294967295. Received -2'
-        });
-
-      assert.rejects(
-        async () => {
-          await handle.chown(1, -2);
-        },
-        {
-          code: 'ERR_OUT_OF_RANGE',
-          name: 'RangeError',
-          message: 'The value of "gid" is out of range. ' +
-                    'It must be >= -1 && <= 4294967295. Received -2'
-        });
+        if (!common.isWindows) {
+          await chown(dest, process.getuid(), process.getgid());
+          await handle.chown(process.getuid(), process.getgid());
+        }
 
-      await handle.close();
+        await assert.rejects(
+          async () => {
+            await chown(dest, 1, -2);
+          },
+          {
+            code: 'ERR_OUT_OF_RANGE',
+            name: 'RangeError',
+            message: 'The value of "gid" is out of range. ' +
+                     'It must be >= -1 && <= 4294967295. Received -2'
+          });
+
+        await assert.rejects(
+          async () => {
+            await handle.chown(1, -2);
+          },
+          {
+            code: 'ERR_OUT_OF_RANGE',
+            name: 'RangeError',
+            message: 'The value of "gid" is out of range. ' +
+                      'It must be >= -1 && <= 4294967295. Received -2'
+          });
+      });
     }
 
     // Set modification times
     {
-      const handle = await getHandle(dest);
-
-      await utimes(dest, new Date(), new Date());
-
-      try {
-        await handle.utimes(new Date(), new Date());
-      } catch (err) {
-        // Some systems do not have futimes. If there is an error,
-        // expect it to be ENOSYS
-        common.expectsError({
-          code: 'ENOSYS',
-          name: 'Error'
-        })(err);
-      }
-
-      await handle.close();
+      await executeOnHandle(dest, async (handle) => {
+
+        await utimes(dest, new Date(), new Date());
+
+        try {
+          await handle.utimes(new Date(), new Date());
+        } catch (err) {
+          // Some systems do not have futimes. If there is an error,
+          // expect it to be ENOSYS
+          common.expectsError({
+            code: 'ENOSYS',
+            name: 'Error'
+          })(err);
+        }
+      });
     }
 
     // Set modification times with lutimes
@@ -438,29 +448,28 @@ async function getHandle(dest) {
 
     // Regression test for https://github.com/nodejs/node/issues/38168
     {
-      const handle = await getHandle(dest);
-
-      assert.rejects(
-        async () => handle.write('abc', 0, 'hex'),
-        {
-          code: 'ERR_INVALID_ARG_VALUE',
-          message: /'encoding' is invalid for data of length 3/
-        }
-      );
+      await executeOnHandle(dest, async (handle) => {
+        await assert.rejects(
+          async () => handle.write('abc', 0, 'hex'),
+          {
+            code: 'ERR_INVALID_ARG_VALUE',
+            message: /'encoding' is invalid for data of length 3/
+          }
+        );
 
-      const ret = await handle.write('abcd', 0, 'hex');
-      assert.strictEqual(ret.bytesWritten, 2);
-      await handle.close();
+        const ret = await handle.write('abcd', 0, 'hex');
+        assert.strictEqual(ret.bytesWritten, 2);
+      });
     }
 
     // Test prototype methods calling with contexts other than FileHandle
     {
-      const handle = await getHandle(dest);
-      assert.rejects(() => handle.stat.call({}), {
-        code: 'ERR_INTERNAL_ASSERTION',
-        message: /handle must be an instance of FileHandle/
+      await executeOnHandle(dest, async (handle) => {
+        await assert.rejects(() => handle.stat.call({}), {
+          code: 'ERR_INTERNAL_ASSERTION',
+          message: /handle must be an instance of FileHandle/
+        });
       });
-      await handle.close();
     }
   }
 

test/parallel/test-fs-read-stream-pos.js

@@ -49,6 +49,7 @@ setInterval(() => {
           return false;
         });
       assert.strictEqual(brokenLines.length, 0);
+      stream.destroy();
       process.exit();
       return;
     }