Open Source
Node.js

Published: August 24, 2026 · by Srinu Desetti · nodejs/node#64895 (opens in a new tab) · merged August 23, 2026

Fixing fs.glob Silently Dropping Files in Node.js Core

Node.js is the runtime under most of the JavaScript ecosystem. My contribution — merged into Node.js core — fixes a bug in fs.glob() where one return statement caused valid files to disappear from results, and did so unpredictably, depending on the order the filesystem returned directory entries.

The fix was six lines — all deletions.

Node.js contribution

What fs.glob Does

fs.glob() finds files matching a pattern:

fs.glob("a/**/*.js")

a/
├── b/
│   ├── one.js
│   └── two.js
├── c/
│   └── three.js
└── d/
    └── four.txt

Expected: a/b/one.js, a/b/two.js, a/c/three.js

Internally, glob reads a directory's children with readdir() and processes each one — with two nested loops: the outer loop over children, and an inner loop over the pattern's matching states (indexes).

Why Glob Has a Cache

Some patterns reach the same directory multiple times. a/**/../* walks into b, then .. back to a; into c, then .. back to a again. Without caching, a gets processed repeatedly. So Node stores path + pattern state — "I already processed this path for this pattern."

The correct guard lives at the start of traversal:

lib/internal/fs/glob.js — outer guard (correct)
async* #iterateSubpatterns(path, pattern) {
  const seen = this.#cache.add(path, pattern);

  if (seen) {
    return;   // already processed this directory + pattern — stop
  }

  // continue processing...
}

This return is right: it asks "have I already processed this whole directory for this pattern?" and stops the duplicate traversal.

The Bug

A second, redundant check sat inside the child-processing loop:

lib/internal/fs/glob.js — inner check (the bug)
for (let i = 0; i < children.length; i++) {
  const entry = children[i];

  for (const index of pattern.indexes) {
    if (
      this.#cache.seen(entryPath, pattern, index) ||
      this.#cache.seen(entryPath, pattern, index + 1)
    ) {
      return;   // ← exits the ENTIRE function, not just this child
    }

    const current = pattern.at(index);
    // matching logic...
  }
}

return doesn't skip one child — it exits the whole traversal function. Imagine children = [b, c, x, z] and c was already seen:

b  → processed ✅
c  → seen → return ❌
x  → never processed
z  → never processed

Valid files silently missing from the results.

Why it was flaky

readdir() does not guarantee order. The same directory on two machines:

Machine A: [b, c, x, z]  →  b ✅  c return  →  x, z lost
Machine B: [c, b, x, z]  →  c return         →  b, x, z lost

Same code, different filesystem order, different results. That's why test-fs-glob.mjs was flaky — the bug only surfaced when a seen entry happened to come before its siblings.

The Fix: Delete, Don't Patch

The obvious patch is returncontinue (skip one child, keep going). But that isn't what I did.

The outer cache.add() guard already prevents duplicate traversal — so the inner check was redundant work on top of being wrong. The right fix is to remove it entirely:

lib/internal/fs/glob.js — the fix (6 lines removed)
for (const index of pattern.indexes) {
-  if (
-    this.#cache.seen(entryPath, pattern, index) ||
-    this.#cache.seen(entryPath, pattern, index + 1)
-  ) {
-    return;
-  }

  const current = pattern.at(index);
  // matching logic...
}

No cache logic was removed. cache.seen() still exists elsewhere — for example to ask "has this state already been scheduled?" before adding a subpattern — and those uses are correct because they never terminate child traversal.

Root cause, not symptom. The bug looked like a cache problem — it wasn't. It was control flow: one keyword in the wrong scope. Diagnosing that is what made the fix a deletion instead of another layer of logic.

The Regression Test

A flaky bug needs a deterministic test. The test pins directory iteration order so the bad ordering happens on every run:

test/parallel/test-fs-glob.mjs — pinning order (concept)
// Force the seen entry to come first, every time
fs.readdirSync = function (path, options) {
  return ["c", "b", "x", "z"];
};

// Without the fix:  a/c
// With the fix:     a/b, a/c, a/x, a/z

Without the fix the result is just a/c; with it, all four entries. No more luck-of-the-filesystem.

Impact

  • fs.glob() no longer silently drops files — a correctness bug in a core filesystem API that affected anyone globbing directory trees where a path can be reached through more than one pattern state.
  • A flaky core test became deterministic — the intermittent test-fs-glob.mjs failures had a real cause, and now have a real test.
  • Less code: six lines deleted, zero added to the logic — the cache stays exactly as it was.

What I Contributed

  • Traced the flaky test to its root cause: a return inside the child loop, not a cache issue.
  • Determined the inner check was redundant given the outer cache.add() guard, and removed it rather than patching it.
  • Wrote a regression test that pins readdir order to reproduce the bug deterministically.
  • Worked through review with Node.js collaborators (approved by two), including the Developer Certificate of Origin sign-off, until the PR was merged. This PR superseded my earlier attempt #62901.

Changed file: lib/internal/fs/glob.js · Linked issue: nodejs/node#62897 (opens in a new tab)

View the pull request → nodejs/node#64895 (opens in a new tab)


← All contributions · Next: Axios circular reference fix →


Frequently Asked Questions

Why did fs.glob drop files only sometimes?

Because readdir() doesn't guarantee entry order. The bug only triggered when an already-seen entry came before its siblings in the returned list — then the return statement exited the traversal and every later sibling was lost. Different machines returned different orders, so the same code produced different results.

Why remove the check instead of changing return to continue?

Because the check was redundant. The cache.add() guard at the start of traversal already prevents reprocessing a directory for a pattern, so the inner check did duplicate work — and did it wrong. Changing return to continue would have kept unnecessary logic; deleting it fixed the bug with less code.

Did the fix remove caching from fs.glob?

No. The cache and the outer cache.add() guard are untouched, and cache.seen() is still used elsewhere to check whether a state has already been scheduled. Only the incorrect inner check inside the child loop was removed.

How does the regression test make a flaky bug reliable?

It pins the directory iteration order so the already-seen entry always comes first. That forces the exact ordering that triggered the bug, so the test fails every time without the fix and passes every time with it.