node icon indicating copy to clipboard operation
node copied to clipboard

Cannot read properties of undefined when requiring `node:fs` with hooks

Open BadIdeaException opened this issue 3 months ago • 4 comments

Version

v22.20.0, v23.11.1, v24.8.0

Platform

Linux exports-undefined 6.8.0-84-generic #84-Ubuntu SMP PREEMPT_DYNAMIC Fri Sep  5 22:36:38 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

module

What steps will reproduce the bug?

register.mjs:

import { registerHooks } from 'node:module';
import { fileURLToPath } from 'node:url';
import { dirname, join } from 'node:path';

function resolve(specifier, context, nextResolve) {
	const path = fileURLToPath(import.meta.url);
	const dir = dirname(path);
	switch (specifier) {
		case 'fs':
		case 'node:fs':
			specifier = join(dir, 'fs-mock.mjs');					
	}

	return nextResolve(specifier, context);
};

registerHooks({ resolve });

fs-mock.mjs:

export const constants = {
	F_OK: 42
}

index.cjs:

const fs = require('node:fs');
console.log(fs.constants.F_OK);

Run node --import=./register.mjs index.cjs.

How often does it reproduce? Is there a required condition?

Happens reliably when fs is being required as node:fs, does not happen when required as fs, or when imported.

What is the expected behavior? Why is that the expected behavior?

Should either require fs-mock.mjs, or require original (builtin) fs module.

What do you see instead?

node:internal/modules/cjs/loader:1155
  return mod.exports;
             ^

TypeError: Cannot read properties of undefined (reading 'exports')
    at loadBuiltinWithHooks (node:internal/modules/cjs/loader:1155:14)
    at Function._load (node:internal/modules/cjs/loader:1197:20)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:237:24)
    at Module.require (node:internal/modules/cjs/loader:1463:12)
    at require (node:internal/modules/helpers:147:16)
    at Object.<anonymous> (/vagrant/index.cjs:1:12)
    at Module._compile (node:internal/modules/cjs/loader:1706:14)
    at Object..js (node:internal/modules/cjs/loader:1839:10)
    at Module.load (node:internal/modules/cjs/loader:1441:32)

Additional information

Output from NODE_DEBUG=* node --import=./register.mjs index.cjs: log.txt

BadIdeaException avatar Sep 25 '25 07:09 BadIdeaException

You are mocking the fs module in ESM but the original is CommonJs

timfish avatar Oct 02 '25 10:10 timfish

Thanks for the suggestion, but I don't think that's it:

  1. It works when I replace require('node:fs') with require('fs'), without changing the contents of fs-mock.mjs. It evens works when I directly require('mock-fs'), or when I require('dummy') and change the resolution logic accordingly.
  2. When I add a throw statement at the top of fs-mock.mjs, no error is thrown, which would seem to indicate that fs-mock.mjs never even gets loaded. Update: Just to make extra sure, I revoked all read access to fs-mock.mjs, but there is no EACCES. This confirms the module never gets loaded at all.

I know that Node's module resolution and loading code takes a different path for loading core modules when using require. I think this bug has to do with that.

BadIdeaException avatar Oct 04 '25 09:10 BadIdeaException

I tried debugging Node.js core and observed the same behavior you’re discussing—not only for "node:fs", but for any specifier that starts with "node:". Even a custom specifier like "node:scoheart" hits the same path.

Concretely, every "node:" request enters the conditional StringPrototypeStartsWith(request, 'node:'), then proceeds into loadBuiltinWithHooks, and eventually attempts to load via loadBuiltinModule. The relevant source looks like this:

// node/lib/internal/modules/cjs/loader.js

function loadBuiltinWithHooks(id, url, format) {
  if (loadHooks.length) {
    url ??= `node:${id}`;
    // TODO(joyeecheung): do we really want to invoke the load hook for the builtins?
    const loadResult = loadWithHooks(
      url,
      format || 'builtin',
      /* importAttributes */ undefined,
      getCjsConditionsArray(),
      getDefaultLoad(url, id)
    );
    if (loadResult.format && loadResult.format !== 'builtin') {
      return undefined;  // Format has been overridden, return undefined for the caller to continue loading.
    }
  }

  // No hooks or the hooks have not overridden the format. Load it as a builtin module and return the exports.
  const mod = loadBuiltinModule(id);
  return mod.exports;
}

Module._load = function(request, parent, isMain) {
  ...
  const { url, format, filename } = resolveForCJSWithHooks(request, parent, isMain);

  // For backwards compatibility, if the request itself starts with node:, load it before checking
  // Module._cache. Otherwise, load it after the check.
  if (StringPrototypeStartsWith(request, 'node:')) {
    const result = loadBuiltinWithHooks(filename, url, format);
    if (result) {
      return result;
    }
    // The format of the builtin has been overridden by user hooks. Continue loading.
  }

  const cachedModule = Module._cache[filename];
  ...
}

What I see at runtime:

  1. request === "node:fs" with no hooks registered, resolveForCJSWithHooks returns:
{
  url: "node:fs",
  format: "builtin",
  filename: "fs"
}

This leads to loadBuiltinModule("fs"), which returns the built-in fs module as expected.

  1. request === "node:fs" with custom hooks registered, resolveForCJSWithHooks returns:
{
  url: "file:///Users/scoheart/Code/opensource/node/mytest/fsmock.mjs",
  format: undefined,
  filename: "/Users/scoheart/Code/opensource/node/mytest/fsmock.mjs"
}

Because the original request still starts with "node:", it still enters the if (StringPrototypeStartsWith(request, 'node:')) branch and calls:

loadBuiltinWithHooks(
  filename, // id === "/Users/.../fsmock.mjs"
  url,      // "file:///Users/.../fsmock.mjs"
  format    // undefined
)

loadBuiltinModule(id) // here id is the filename path —— "/Users/.../fsmock.mjs"

Since id is now an absolute path (not a known builtin like "fs"), loadBuiltinModule("/Users/.../fsmock.mjs") fails—there is obviously no builtin module by that name—leading to the error I’m seeing.

I’m not entirely sure whether this should be considered a bug or simply the intended behavior of the "node:" scheme plus the “backwards compatibility” branch in _load.

Question to maintainers

Is this the expected, intentional behavior for "node:" specifiers (including custom ones), or should custom "node:" specifiers be treated differently so they don’t fall through to loadBuiltinModule unless they resolve to an actual builtin?

Scoheart avatar Oct 05 '25 09:10 Scoheart

I think in this case, it should use the overriden resolution result as expected. I have a fix in https://github.com/joyeecheung/node/tree/fix-node-hooks though I will need to further test it with the load hooks before sending a PR.

joyeecheung avatar Dec 06 '25 06:12 joyeecheung

I have a fix

Nice :+1: I hope that gets merged soon, then.

BadIdeaException avatar Dec 15 '25 19:12 BadIdeaException