fix: do not crash on error in fs.walk filter (#18886)

Author: mdjermanovic

lib/eslint/eslint-helpers.js

@@ -15,7 +15,6 @@ const fsp = fs.promises;
 const isGlob = require("is-glob");
 const hash = require("../cli-engine/hash");
 const minimatch = require("minimatch");
-const util = require("util");
 const fswalk = require("@nodelib/fs.walk");
 const globParent = require("glob-parent");
 const isPathInside = require("is-path-inside");
@@ -24,7 +23,6 @@ const isPathInside = require("is-path-inside");
 // Fixup references
 //-----------------------------------------------------------------------------
 
-const doFsWalk = util.promisify(fswalk.walk);
 const Minimatch = minimatch.Minimatch;
 const MINIMATCH_OPTIONS = { dot: true };
 
@@ -270,56 +268,92 @@ async function globSearch({
      */
     const unmatchedPatterns = new Set([...relativeToPatterns.keys()]);
 
-    const filePaths = (await doFsWalk(basePath, {
+    const filePaths = (await new Promise((resolve, reject) => {
 
-        deepFilter(entry) {
-            const relativePath = normalizeToPosix(path.relative(basePath, entry.path));
-            const matchesPattern = matchers.some(matcher => matcher.match(relativePath, true));
-
-            return matchesPattern && !configs.isDirectoryIgnored(entry.path);
-        },
-        entryFilter(entry) {
-            const relativePath = normalizeToPosix(path.relative(basePath, entry.path));
+        let promiseRejected = false;
 
-            // entries may be directories or files so filter out directories
-            if (entry.dirent.isDirectory()) {
+        /**
+         * Wraps a boolean-returning filter function. The wrapped function will reject the promise if an error occurs.
+         * @param {Function} filter A filter function to wrap.
+         * @returns {Function} A function similar to the wrapped filter that rejects the promise if an error occurs.
+         */
+        function wrapFilter(filter) {
+            return (...args) => {
+
+                // No need to run the filter if an error has been thrown.
+                if (!promiseRejected) {
+                    try {
+                        return filter(...args);
+                    } catch (error) {
+                        promiseRejected = true;
+                        reject(error);
+                    }
+                }
                 return false;
-            }
+            };
+        }
 
-            /*
-             * Optimization: We need to track when patterns are left unmatched
-             * and so we use `unmatchedPatterns` to do that. There is a bit of
-             * complexity here because the same file can be matched by more than
-             * one pattern. So, when we start, we actually need to test every
-             * pattern against every file. Once we know there are no remaining
-             * unmatched patterns, then we can switch to just looking for the
-             * first matching pattern for improved speed.
-             */
-            const matchesPattern = unmatchedPatterns.size > 0
-                ? matchers.reduce((previousValue, matcher) => {
-                    const pathMatches = matcher.match(relativePath);
+        fswalk.walk(
+            basePath,
+            {
+                deepFilter: wrapFilter(entry => {
+                    const relativePath = normalizeToPosix(path.relative(basePath, entry.path));
+                    const matchesPattern = matchers.some(matcher => matcher.match(relativePath, true));
+
+                    return matchesPattern && !configs.isDirectoryIgnored(entry.path);
+                }),
+                entryFilter: wrapFilter(entry => {
+                    const relativePath = normalizeToPosix(path.relative(basePath, entry.path));
+
+                    // entries may be directories or files so filter out directories
+                    if (entry.dirent.isDirectory()) {
+                        return false;
+                    }
 
                     /*
-                     * We updated the unmatched patterns set only if the path
-                     * matches and the file isn't ignored. If the file is
-                     * ignored, that means there wasn't a match for the
-                     * pattern so it should not be removed.
-                     *
-                     * Performance note: isFileIgnored() aggressively caches
-                     * results so there is no performance penalty for calling
-                     * it twice with the same argument.
+                     * Optimization: We need to track when patterns are left unmatched
+                     * and so we use `unmatchedPatterns` to do that. There is a bit of
+                     * complexity here because the same file can be matched by more than
+                     * one pattern. So, when we start, we actually need to test every
+                     * pattern against every file. Once we know there are no remaining
+                     * unmatched patterns, then we can switch to just looking for the
+                     * first matching pattern for improved speed.
                      */
-                    if (pathMatches && !configs.isFileIgnored(entry.path)) {
-                        unmatchedPatterns.delete(matcher.pattern);
-                    }
-
-                    return pathMatches || previousValue;
-                }, false)
-                : matchers.some(matcher => matcher.match(relativePath));
-
-            return matchesPattern && !configs.isFileIgnored(entry.path);
-        }
-
+                    const matchesPattern = unmatchedPatterns.size > 0
+                        ? matchers.reduce((previousValue, matcher) => {
+                            const pathMatches = matcher.match(relativePath);
+
+                            /*
+                             * We updated the unmatched patterns set only if the path
+                             * matches and the file isn't ignored. If the file is
+                             * ignored, that means there wasn't a match for the
+                             * pattern so it should not be removed.
+                             *
+                             * Performance note: isFileIgnored() aggressively caches
+                             * results so there is no performance penalty for calling
+                             * it twice with the same argument.
+                             */
+                            if (pathMatches && !configs.isFileIgnored(entry.path)) {
+                                unmatchedPatterns.delete(matcher.pattern);
+                            }
+
+                            return pathMatches || previousValue;
+                        }, false)
+                        : matchers.some(matcher => matcher.match(relativePath));
+
+                    return matchesPattern && !configs.isFileIgnored(entry.path);
+                })
+            },
+            (error, entries) => {
+
+                // If the promise is already rejected, calling `resolve` or `reject` will do nothing.
+                if (error) {
+                    reject(error);
+                } else {
+                    resolve(entries);
+                }
+            }
+        );
     })).map(entry => entry.path);
 
     // now check to see if we have any unmatched patterns

tests/lib/eslint/flat-eslint.js

@@ -4541,7 +4541,40 @@ describe("FlatESLint", () => {
             });
         });
 
+        describe("Error while globbing", () => {
 
+            it("should throw an error with a glob pattern if an invalid config was provided", async () => {
+
+                const cwd = getFixturePath("files");
+
+                eslint = new FlatESLint({
+                    cwd,
+                    overrideConfig: [{ invalid: "foobar" }]
+                });
+
+                await assert.rejects(eslint.lintFiles("*.js"));
+            });
+
+            it("should throw an error with a glob pattern if an error occurs traversing a directory", async () => {
+
+                const fsWalk = require("@nodelib/fs.walk");
+                const error = new Error("Boom!");
+
+                sinon
+                    .stub(fsWalk, "walk")
+                    .value(sinon.stub().yieldsRight(error)); // call the callback passed to `fs.walk` with an error
+
+                const cwd = getFixturePath("files");
+
+                eslint = new FlatESLint({
+                    cwd,
+                    overrideConfigFile: true
+                });
+
+                await assert.rejects(eslint.lintFiles("*.js"), error);
+            });
+
+        });
     });
 
     describe("Fix Types", () => {