Skip to content

module: fix require(esm) error annotation with tracing subscriber - #66930

Open
CodingRule wants to merge 1 commit into
nodejs:mainfrom
CodingRule:fix/require-esm-error-annotation
Open

CodingRule wants to merge 1 commit into
nodejs:mainfrom
CodingRule:fix/require-esm-error-annotation

Conversation

@CodingRule

@CodingRule CodingRule commented Oct 22, 2026 •

Copy link
Copy Markdown

reconstructErrorStack() takes the first stack frame as the require() caller. When a module.require tracing subscriber exists, that frame is TracingChannel.traceSync, so the arrow points at a nonexistent line of the parent file:

test.cjs:568
undefined
                     ^

Pick the first frame that belongs to the parent file instead. Works for nested callers too (the first parent frame is the require() call site).

Adds a test case with a tracing:module.require:start subscriber; it fails on current main and passes with this change.

Fixes: #55350

AI disclosure

This change was made with help from an AI coding assistant (Claude Mythos 5.1). I reviewed the diff and the reasoning behind it myself.

What was verified:

  • Reproduced the bug on the current nightly (v27.0.0-nightly20261020) with a tracing:module.require:start subscriber: the arrow points at test.cjs:568 with undefined.
  • Ran reconstructErrorStack() from both the original and patched loader.js against real stacks captured from the nightly, in four shapes: plain, with subscriber, nested callers, nested with subscriber. The patched version annotates the require() call site in all four. The original fails in both subscriber cases.
  • The new test case fails on the unfixed nightly and the two existing cases still pass.
  • tools/eslint on both files with --max-warnings=0.

Not run locally: the test against a binary built from this branch (no ClangCL toolchain on my machine). I'll follow up on review feedback myself.

reconstructErrorStack() took the first stack frame as the caller, but
with a module.require tracing subscriber that frame is
TracingChannel.traceSync, so the arrow pointed at a nonexistent line
of the parent file. Pick the first frame that belongs to the parent.

Fixes: nodejs#55350
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. labels Oct 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong error annotation when commonjs requires an ES module

2 participants