Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot catch exception in filter function of fs.walk #105

Open
fasttime opened this issue Apr 8, 2024 · 1 comment
Open

Cannot catch exception in filter function of fs.walk #105

fasttime opened this issue Apr 8, 2024 · 1 comment

Comments

@fasttime
Copy link

fasttime commented Apr 8, 2024

Describe the bug

When a filter callback supplied to fs.walk throws an error, the process terminates abnormally, without a chance for the caller to catch the error.

Steps to reproduce

Example 1

// example-1.js
const fsWalk = require('@nodelib/fs.walk');

try {
    fsWalk.walk(
        '.',
        {
            entryFilter() {
                throw new Error('Something is broken');
            }
        },
        error => {
            if (error) {
                console.log('Error caught in callback');
            }
        }
    );
} catch (error) {
    console.log('Error caught outside callback');
}

Example 2

// example-2.mjs
import { promisify } from 'node:util';
import { walk } from '@nodelib/fs.walk';

const walkAsync = promisify(walk);
try {
    await walkAsync(
        '.',
        {
            deepFilter() {
                throw new Error('Something is broken');
            }
        }
    );
} catch (error) {
    console.log('Error caught. All is good.');
}

Expected behavior

In both examples above, the error thrown in the filter (entryFilter or deepFilter), should be caught and execution should continue normally.

Actually, both examples will crash the process with an uncaught exception in a different call context, e.g.

$ node example-2.mjs
file:///.../example-2.mjs:10
                throw new Error('Something is broken');
                ^

Error: Something is broken
    at deepFilter (file:///.../example-2.mjs:10:23)
    at Object.isAppliedFilter (.../node_modules/@nodelib/fs.walk/out/readers/common.js:12:31)
    at AsyncReader._handleEntry (.../node_modules/@nodelib/fs.walk/out/readers/async.js:89:50)
    at .../node_modules/@nodelib/fs.walk/out/readers/async.js:65:22
    at callSuccessCallback (.../node_modules/@nodelib/fs.scandir/out/providers/async.js:103:5)
    at .../node_modules/@nodelib/fs.scandir/out/providers/async.js:29:13
    at node:fs:193:23
    at node:internal/util:522:12
    at getDirents (node:internal/fs/utils:286:7)
    at req.oncomplete (node:fs:1480:7)

Environment

  • OS Version: macOS 14.4.1
  • Node.js Version: v21.7.2
  • @nodelib Version: fs.walk 2.0.0, fs.scandir 3.0.0
@fasttime
Copy link
Author

fasttime commented Apr 8, 2024

I can fix the problem in fs.scandir by ensuring that the failure callback is always called if the success callback throws an error (these are actually the same function with different parameters).

--- a/packages/fs/fs.scandir/src/providers/async.ts
+++ b/packages/fs/fs.scandir/src/providers/async.ts
@@ -112 +112,7 @@ function callSuccessCallback(callback: AsyncCallback, result: Entry[]): void {
-       callback(null, result);
+    try {
+        callback(null, result);
+    } catch (error) {
+        if (error instanceof Error) {
+            callFailureCallback(callback, error);
+        }
+    }
 }

There are certainly other ways to handle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant