Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing import trace from error stack when loading a module throws an error #46992

Open
SystemParadox opened this issue Mar 7, 2023 · 17 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js.

Comments

@SystemParadox
Copy link

SystemParadox commented Mar 7, 2023

Version

v16.16.0

Platform

Linux 5.4.0-139-generic #156-Ubuntu SM x86_64 GNU/Linux

Subsystem

modules

What steps will reproduce the bug?

Consider:

// foo.mjs
import './bar.mjs';
// bar.mjs
throw new Error('bar failed');

Run node foo.mjs.

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

Always.

What is the expected behavior?

The stack trace from the error should include a reference to foo.mjs line 1 showing where bar was imported from.

What do you see instead?

file:///.../bar.mjs:1
throw new Error('bar failed');
      ^

Error: bar failed
    at file:///.../bar.mjs:1:7
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:385:24)
    at async loadESM (node:internal/process/esm_loader:88:5)
    at async handleMainPromise (node:internal/modules/run_main:61:12)

Note that the stack trace does not mention foo anywhere. This is not helpful, especially if the error is specifically about how and when bar is imported. For example, it might be a singleton and you're mistakenly creating a duplicate by loading it from both import and require, or it might need something else to be set up on the global first and you're loading it in the wrong order.

The above stack is from node 16. Node 18 produces a shorter but similarly lacking stack:

file:///.../bar.mjs:1
throw new Error('bar failed');
      ^

Error: bar failed
    at file:///.../bar.mjs:1:7
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

Additional information

I am aware that import() and top-level await complicates this, since modules may be effectively loading in parallel (asynchronously behind the scenes). However, it is absolutely essential that this information is made available somehow.

If the output ends up being a list of all modules trying to load the offending module then great, as long as this list is in the order the imports were encountered then that should allow debugging this kind of thing.

At the moment when this occurs it's simply impossible to debug. Since import gets hoisted by the language you can't even add console.log statements around the place to trace what's happening like you can with require. Literally the only option to debug this is to comment out all the imports and re-enable one-by-one (recursively) until you locate the problem. And in practice this inevitably breaks something else, so it's completely unworkable.

@benjamingr
Copy link
Member

@nodejs/modules @nodejs/loaders

@JakobJingleheimer

This comment was marked as outdated.

@JakobJingleheimer JakobJingleheimer added esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders confirmed-bug Issues with confirmed bugs. labels Mar 8, 2023
@JakobJingleheimer
Copy link
Contributor

Gah, sorry, I misread my own output 🤦‍♂️

This may be fixed as part of #44710, which will most definitely change the stack-trace. I'll test there once all the tests are passing consistently.

@GeoffreyBooth
Copy link
Member

in practice this inevitably breaks something else, so it’s completely unworkable.

What are the stack traces produced for the equivalent test case in Deno and in Chrome?

@GeoffreyBooth GeoffreyBooth removed the confirmed-bug Issues with confirmed bugs. label Mar 12, 2023
@SystemParadox
Copy link
Author

SystemParadox commented Mar 16, 2023

This is the output from Deno (1.31.3), basically the same as Node:

error: Uncaught Error: bar failed
throw new Error('bar failed');
      ^
    at file:///.../bar.mjs:1:7

Chrome is also the same, but the key difference is that with Chrome you can at least go to the network tab and look at the "Initiators" list to see why something is being loaded. It's really clumsy (you have to manually search for it in the network tab and you can't double-click anything) and it lacks line numbers, but it's a start.

I don't use Deno but if a Deno user wants to raise a corresponding bug with them and link it here that would probably be a good idea.

@GeoffreyBooth
Copy link
Member

If none of the runtimes are doing this, then it's a feature request rather than a bug. For me it also raises the question of what a stack trace is; I would think it should go to the line that threw the exception, as it does in the current traces, rather than the module that imported the line that threw the exception.

@GeoffreyBooth GeoffreyBooth added feature request Issues that request new features to be added to Node.js. and removed loaders Issues and PRs related to ES module loaders labels Mar 16, 2023
@SystemParadox
Copy link
Author

SystemParadox commented Mar 16, 2023

If none of the runtimes are doing this, then it's a feature request rather than a bug.

It's a very serious and major regression when compared with commonJS. People are shifting to pure ESM now and it's literally impossible to debug this kind of thing. This really needs to be sorted ASAP.

For me it also raises the question of what a stack trace is; I would think it should go to the line that threw the exception, as it does in the current traces, rather than the module that imported the line that threw the exception.

Obviously that should be the top of the stack. But the stack should not stop there, it needs context. Even if due to the technicalities of async module loading it might be requested from multiple places that doesn't change anything. If a module throws an exception we need to see why it was loaded and in what order.

@GeoffreyBooth
Copy link
Member

It’s a very serious and major regression when compared with commonJS. People are shifting to pure ESM now and it’s literally impossible to debug this kind of thing. This really needs to be sorted ASAP.

Pull requests are welcome!

@HinataKah0
Copy link
Contributor

HinataKah0 commented May 2, 2023

I tried looking into internal/modules out of curiosity but can't figure out how to add import calls to the call stack.
I know that V8 has provided this API.

In CJS, require calls are pushed to call stack because it's actually a function call.
But in ESM, it seems to me that the evaluation is done in module_wrap.cc (found it by adding console.log() everywhere and it stopped here before the error is thrown) and I can't figure out how to make it appear in the call stack (because it's not a JS function call and not to mention that the imports are evaluated async). Can I get some pointers?

Pull requests are welcome!

@GeoffreyBooth I assume you have the rough solution in mind, may I know if what I've said is still very far from what you have in mind? Or did I get everything wrong? 🤣

@GeoffreyBooth
Copy link
Member

@GeoffreyBooth I assume you have the rough solution in mind, may I know if what I’ve said is still very far from what you have in mind? Or did I get everything wrong? 🤣

I don’t have a solution in mind, sorry. I would look into how the stack traces are currently assembled to see what other options you may have.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Oct 30, 2023
@SystemParadox
Copy link
Author

This is still a major usability issue and needs attention.

@github-actions github-actions bot removed the stale label Oct 31, 2023
@GeoffreyBooth
Copy link
Member

This is still a major usability issue and needs attention.

Pull requests are still welcome! Including from you, @SystemParadox!

@brillout
Copy link
Contributor

Is there any workaround for this? The workaround here doesn't seem helpful.

It's a major issue for Vike users.

@GeoffreyBooth
Copy link
Member

It's a major issue for Vike users.

Pull requests are encouraged, including from Vike users or maintainers!

Copy link
Contributor

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Mark issue so that it is never considered stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jul 20, 2024
@SystemParadox
Copy link
Author

SystemParadox commented Jul 20, 2024

This is still relevant. It may be worth noting that browsers do have some ability to show this via the "initiator" section on the network tab. But there's nothing for node.

@github-actions github-actions bot removed the stale label Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js.
Projects
Status: Awaiting Triage
Development

No branches or pull requests

6 participants