fix(node): fix fs.readdir

node/_fs/_fs_readdir.ts

@@ -1,6 +1,8 @@
 import { asyncIterableToCallback } from "./_fs_watch.ts";
 import Dirent from "./_fs_dirent.ts";
-import { fromFileUrl } from "../path.ts";
+import { denoErrorToNodeError } from "../_errors.ts";
+import { getValidatedPath } from "../internal/fs/utils.js";
+import { Buffer } from "../buffer.ts";
 
 function toDirent(val: Deno.DirEntry): Dirent {
   return new Dirent(val);
@@ -20,18 +22,18 @@ type readDirBoth = (
 ) => void;
 
 export function readdir(
-  path: string | URL,
+  path: string | Buffer | URL,
   options: { withFileTypes?: false; encoding?: string },
   callback: readDirCallback,
 ): void;
 export function readdir(
-  path: string | URL,
+  path: string | Buffer | URL,
   options: { withFileTypes: true; encoding?: string },
   callback: readDirCallbackDirent,
 ): void;
 export function readdir(path: string | URL, callback: readDirCallback): void;
 export function readdir(
-  path: string | URL,
+  path: string | Buffer | URL,
   optionsOrCallback: readDirOptions | readDirCallback | readDirCallbackDirent,
   maybeCallback?: readDirCallback | readDirCallbackDirent,
 ) {
@@ -43,7 +45,7 @@ export function readdir(
     ? optionsOrCallback
     : null;
   const result: Array<string | Dirent> = [];
-  path = path instanceof URL ? fromFileUrl(path) : path;
+  path = getValidatedPath(path);
 
   if (!callback) throw new Error("No callback function supplied");
 
@@ -58,7 +60,7 @@ export function readdir(
   }
 
   try {
-    asyncIterableToCallback(Deno.readDir(path), (val, done) => {
+    asyncIterableToCallback(Deno.readDir(path.toString()), (val, done) => {
       if (typeof path !== "string") return;
       if (done) {
         callback(null, result);
@@ -67,9 +69,11 @@ export function readdir(
       if (options?.withFileTypes) {
         result.push(toDirent(val));
       } else result.push(decode(val.name));
+    }, (e) => {
+      callback(denoErrorToNodeError(e as Error, { syscall: "readdir" }));
     });
-  } catch (error) {
-    callback(error instanceof Error ? error : new Error("[non-error thrown]"));
+  } catch (e) {
+    callback(denoErrorToNodeError(e as Error, { syscall: "readdir" }));
   }
 }
 
@@ -83,19 +87,19 @@ function decode(str: string, encoding?: string): string {
 }
 
 export function readdirSync(
-  path: string | URL,
+  path: string | Buffer | URL,
   options: { withFileTypes: true; encoding?: string },
 ): Dirent[];
 export function readdirSync(
-  path: string | URL,
+  path: string | Buffer | URL,
   options?: { withFileTypes?: false; encoding?: string },
 ): string[];
 export function readdirSync(
-  path: string | URL,
+  path: string | Buffer | URL,
   options?: readDirOptions,
 ): Array<string | Dirent> {
   const result = [];
-  path = path instanceof URL ? fromFileUrl(path) : path;
+  path = getValidatedPath(path);
 
   if (options?.encoding) {
     try {
@@ -107,10 +111,14 @@ export function readdirSync(
     }
   }
 
-  for (const file of Deno.readDirSync(path)) {
-    if (options?.withFileTypes) {
-      result.push(toDirent(file));
-    } else result.push(decode(file.name));
+  try {
+    for (const file of Deno.readDirSync(path.toString())) {
+      if (options?.withFileTypes) {
+        result.push(toDirent(file));
+      } else result.push(decode(file.name));
+    }
+  } catch (e) {
+    throw denoErrorToNodeError(e as Error, { syscall: "readdir" });
   }
   return result;
 }

node/_fs/_fs_watch.ts

@@ -22,6 +22,7 @@ export function asyncIterableIteratorToCallback<T>(
 export function asyncIterableToCallback<T>(
   iter: AsyncIterable<T>,
   callback: (val: T, done?: boolean) => void,
+  errCallback: (e: unknown) => void,
 ) {
   const iterator = iter[Symbol.asyncIterator]();
   function next() {
@@ -32,7 +33,7 @@ export function asyncIterableToCallback<T>(
       }
       callback(obj.value);
       next();
-    });
+    }, errCallback);
   }
   next();
 }
@@ -91,6 +92,8 @@ export function watch(
   asyncIterableToCallback<Deno.FsEvent>(iterator, (val, done) => {
     if (done) return;
     fsWatcher.emit("change", val.kind, val.paths[0]);
+  }, (e) => {
+    fsWatcher.emit("error", e);
   });
 
   return fsWatcher;

node/_tools/config.json

@@ -149,6 +149,8 @@
       "test-fs-chmod-mask.js",
       "test-fs-chmod.js",
       "test-fs-chown-type-check.js",
+      "test-fs-readdir-stack-overflow.js",
+      "test-fs-readdir.js",
       "test-fs-readfile-empty.js",
       "test-fs-rm.js",
       "test-fs-rmdir-recursive-sync-warns-not-found.js",
@@ -369,6 +371,7 @@
     "parallel": [
       "test-child-process-fork.js",
       "test-fs-access.js",
+      "test-fs-readdir-stack-overflow.js",
       "test-fs-write-file-invalid-path.js",
       "test-fs-write-file-sync.js",
       "test-fs-write-file.js",

node/_tools/suites/parallel/test-fs-readdir-stack-overflow.js

@@ -0,0 +1,26 @@
+// deno-fmt-ignore-file
+// deno-lint-ignore-file
+
+// Copyright Joyent and Node contributors. All rights reserved. MIT license.
+// Taken from Node 16.13.0
+// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+
+'use strict';
+
+require('../common');
+
+const assert = require('assert');
+const fs = require('fs');
+
+function recurse() {
+  fs.readdirSync('.');
+  recurse();
+}
+
+assert.throws(
+  () => recurse(),
+  {
+    name: 'RangeError',
+    message: 'Maximum call stack size exceeded'
+  }
+);

node/_tools/suites/parallel/test-fs-readdir.js

@@ -0,0 +1,60 @@
+// deno-fmt-ignore-file
+// deno-lint-ignore-file
+
+// Copyright Joyent and Node contributors. All rights reserved. MIT license.
+// Taken from Node 16.13.0
+// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+
+const tmpdir = require('../common/tmpdir');
+
+const readdirDir = tmpdir.path;
+const files = ['empty', 'files', 'for', 'just', 'testing'];
+
+// Make sure tmp directory is clean
+tmpdir.refresh();
+
+// Create the necessary files
+files.forEach(function(currentFile) {
+  fs.closeSync(fs.openSync(`${readdirDir}/${currentFile}`, 'w'));
+});
+
+// Check the readdir Sync version
+assert.deepStrictEqual(files, fs.readdirSync(readdirDir).sort());
+
+// Check the readdir async version
+fs.readdir(readdirDir, common.mustSucceed((f) => {
+  assert.deepStrictEqual(files, f.sort());
+}));
+
+// readdir() on file should throw ENOTDIR
+// https://github.com/joyent/node/issues/1869
+assert.throws(function() {
+  fs.readdirSync(__filename);
+}, /Error: ENOTDIR: not a directory/);
+
+fs.readdir(__filename, common.mustCall(function(e) {
+  assert.strictEqual(e.code, 'ENOTDIR');
+}));
+
+[false, 1, [], {}, null, undefined].forEach((i) => {
+  assert.throws(
+    () => fs.readdir(i, common.mustNotCall()),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError'
+    }
+  );
+  assert.throws(
+    () => fs.readdirSync(i),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError'
+    }
+  );
+});

node/internal_binding/_libuv_winerror.ts

@@ -153,7 +153,7 @@ export function uvTranslateSysError(sysErrno: number): string {
     case winErrors.ERROR_BAD_PATHNAME:
       return "ENOENT";
     case winErrors.ERROR_DIRECTORY:
-      return "ENOENT";
+      return "ENOTDIR";
     case winErrors.ERROR_ENVVAR_NOT_FOUND:
       return "ENOENT";
     case winErrors.ERROR_FILE_NOT_FOUND: