node-klaw icon indicating copy to clipboard operation
node-klaw copied to clipboard

Maximum call stack size exceeded

Open aikar opened this issue 5 years ago • 10 comments

RangeError: Maximum call stack size exceeded at node_modules/klaw/src/index.js:45:23 at go$readdir$cb (node_modules/graceful-fs/graceful-fs.js:187:14) at FSReqWrap.oncomplete (fs.js:135:15)

version 2.1.1 but nothing in new version relates here.

I have a folder with 200k+ files in it that crashes klaw.

        const crawl = async (subdir: string, cb) => {
            const done = resolvable();
            klaw(path.join(dir, subdir), {
                filter: (item) => {
                    return item.endsWith(".job");
                }
            }).on('data', (item) => {
                cb(item.path);               
            }).on('end', () => done.resolve());
            await done;
        };

aikar avatar Nov 09 '19 02:11 aikar

Ended up switching to https://www.npmjs.com/package/walk due to this bug.

aikar avatar Jan 15 '20 23:01 aikar

Hey @jprichardson how would you feel changing this https://github.com/jprichardson/node-klaw/blob/master/src/index.js#L53 to not use apply? Either spreading the array or concat will avoid stack size range error and also probably would be faster?

i think concat might probably be the first solution but then array will need to be mutated

oorestisime avatar Sep 17 '20 13:09 oorestisime

Certainly. Should modernize the code as well (separate PR)

jprichardson avatar Sep 17 '20 15:09 jprichardson

Thanks for the quick response. PR opened :)

oorestisime avatar Sep 18 '20 06:09 oorestisime

@oorestisime would you be willing to put a test in place verifying that this actually fixes the problem? Thanks.

jprichardson avatar Sep 19 '20 06:09 jprichardson

I can take a look yeah. but this means adding a test with more than 200k files or so (haven't looked how the tests are )

oorestisime avatar Sep 20 '20 17:09 oorestisime

Couldn't a test harness be used to generate the files?

jprichardson avatar Sep 22 '20 03:09 jprichardson

We experienced this problem with around 60K files in a folder.

mhahn-sighthoundlabs avatar May 19 '21 22:05 mhahn-sighthoundlabs

hey i am back at this. i completely forgot about it. what are our options here. change seems fairly trivial imho

oorestisime avatar Aug 17 '21 15:08 oorestisime

@oorestisime's fix it working for us with 260k pages.

pea avatar Apr 19 '22 09:04 pea