Crash on Node `>= 24.13`

Open Beginner friendly
#6 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
javascript, node.js
Domain
devtools

Research direction

Start with dist/esm/hooks.mjs and inspect the extensionless-file branches in both loadSync and load, then review the version gate in dist/esm/import.mjs. Add an extensionless CommonJS fixture and verify it can be required with sync hooks on Node >= 24.13, remains recorded for coverage, and no longer throws ERR_INVALID_RETURN_PROPERTY_VALUE.

Written by the indexing model from the issue text.

Description

Summary

On Node >= 24.13, @tapjs/processinfo registers its module hooks through the
synchronous module.registerHooks({ load }) API (see import.mjs version gate).
Node strict-validates the object returned by a synchronous load hook: when
shortCircuit: true is returned, a source must be provided.

loadSync short-circuits extensionless files (package bin scripts such as
cdk, cdklocal, npx, etc.) and returns no source:

// dist/esm/hooks.mjs
export const loadSync = (url, context, nextLoad) => {
    if (url.startsWith('file://')) {
        const filename = fileURLToPath(url);
        const { ext } = parse(filename);
        if (!ext) {
            record(url);
            return {
                ...context,
                format: 'commonjs',
                shortCircuit: true,   // <-- no `source`
            };
        }
    }
    const ret = nextLoad(url, context);
    record(url, ret.source);
    return ret;
};

Node then throws:

node:internal/modules/customization_hooks:293
    throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
          ^
TypeError [ERR_INVALID_RETURN_PROPERTY_VALUE]: Expected a string, an ArrayBuffer,
or a TypedArray to be returned for the "source" from the "load" hook but got undefined.
    at validateSourceStrict (node:internal/modules/customization_hooks:293:11)
    at validateLoadStrict (node:internal/modules/customization_hooks:272:3)
    at nextStep (node:internal/modules/customization_hooks:194:14)
    at loadWithHooks (node:internal/modules/customization_hooks:374:18)
    at Object.newLoader [as .js] (.../node_modules/pirates/lib/index.js:134:7)
    at Module.load (node:internal/modules/cjs/loader:1577:32)

This happens for any child process spawned while the loader is active,
because @tapjs/processinfo propagates its instrumentation to all descendants
via process-on-spawn (it re-injects --import .../import.mjs into
NODE_OPTIONS on every spawn). So a test suite that shells out to a tool whose
entry point is an extensionless bin (e.g. AWS CDK / cdklocal) fails at module
load time, before the tool does any real work.

Why it is version-gated

dist/esm/import.mjs chooses the hook mechanism by Node version:

const useSyncHooks = typeof MODULE.registerHooks === 'function' &&
    (!MODULE.register ||
        version[0] > 25 ||
        (version[0] === 25 && version[1] >= 1) ||
        (version[0] === 24 && version[1] >= 13));   // <-- 24.13+
if (useSyncHooks) {
    MODULE.registerHooks({ load: loadSync });        // strict source validation
} else {
    /* MODULE.register(loader.mjs) — async load hook, not strict-validated */
}
  • Node < 24.13: async MODULE.register path → no strict source validation → works.
  • Node >= 24.13: MODULE.registerHooks sync path → strict validation → crash.

The async load hook in the same file has the identical extensionless branch,
so it is worth fixing both for consistency even though only the sync path is
strict-validated today.

Reproduction

Minimal (no external tools)

extensionless-bin is any file without an extension that is require()d:

// repro.mjs  —  run with: node repro.mjs   (Node >= 24.13)
import { register } from 'node:module';
import { pathToFileURL } from 'node:url';

// Use the package's own loader entry to mirror real usage:
register('@tapjs/processinfo/import', pathToFileURL('./'));

// Now require an extensionless CommonJS file (as package bins are):
import { createRequire } from 'node:module';
const require = createRequire(import.meta.url);
require('./some-extensionless-bin'); // -> ERR_INVALID_RETURN_PROPERTY_VALUE
Real-world (what we hit)

A tap test run (coverage enabled) whose setup shells out to aws-cdk-local:

tap  →  before hook  →  child_process.spawn("npm", ["run", "cdklocal:deploy"])
     →  processinfo re-injects --import into NODE_OPTIONS (via process-on-spawn)
     →  cdk/cdklocal/npx load an extensionless bin
     →  loadSync short-circuits without `source`  →  crash

Observed on node:24.16.0; the same project passes on node:24.12.0.

Proposed fix

For extensionless files, do not short-circuit without a source. Delegate to
nextLoad with only the format hint, so Node reads the real source:

 export const loadSync = (url, context, nextLoad) => {
     if (url.startsWith('file://')) {
         const filename = fileURLToPath(url);
         const { ext } = parse(filename);
         if (!ext) {
             record(url);
-            return {
-                ...context,
-                format: 'commonjs',
-                shortCircuit: true,
-            };
+            return nextLoad(url, { ...context, format: 'commonjs' });
         }
     }
     const ret = nextLoad(url, context);
     record(url, ret.source);
     return ret;
 };

Apply the equivalent change to the async load hook:

 export const load = async (url, context, nextLoad) => {
     if (url.startsWith('file://')) {
         const filename = fileURLToPath(url);
         const { ext } = parse(filename);
         if (!ext) {
             record(url);
-            return {
-                ...context,
-                format: 'commonjs',
-                shortCircuit: true,
-            };
+            return await nextLoad(url, { ...context, format: 'commonjs' });
         }
     }
     const originSource = context.source;
     const ret = await nextLoad(url, context);
     record(url, ret.source, originSource);
     return ret;
 };

Passing format: 'commonjs' preserves the original intent (telling Node the
extensionless file is CommonJS) while letting the default step supply a valid
source, which satisfies the strict validation.

Verification

With the patch applied to dist/esm/hooks.mjs and the loader active via
NODE_OPTIONS="--import .../@tapjs/processinfo/dist/esm/import.mjs" on
Node 24.16.0, the previously-failing cdklocal synth/deploy completes
successfully (no ERR_INVALID_RETURN_PROPERTY_VALUE). The behaviour on
Node < 24.13 is unchanged.

Suggested test

Add a fixture that is an extensionless CommonJS file and assert it can be
require()d while the sync hooks are registered on Node >= 24.13, expecting
no throw and the file still recorded for coverage.

Workaround (for consumers, until released)

Pin the runtime to a Node minor < 24.13 (e.g. node:24.12)

Dominant language
JavaScript
Stars
7
Forks
4
PR merge metrics
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from tapjs/processinfo

All issues in tapjs/processinfo

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.