Repository navigation
Dynamic import does not show location of SyntaxError #49441
Description
Activity
This is something we unfortunately inherited from Chrome/V8. You can try the following in the browser devtools:
await import('data:text/javascript,<not valid syntax>')
We may be able to patch the stack even further. The main issue is that
import()actually makes these errors easily observable. So the fact thaterr.stackis "weird" and doesn't properly start with the usual prefix would leak.That leak will pose a problem as the error stacks proposal attempts to advance; it would be exceedingly ideal to avoid creating new observable stack trace formats/contents.
One thing I'd love to see is to have syntax errors from module executions include the parsed module as the top frame. But I can understand why that may be awkward (there's no actual stack frame running that code).
My primary concern is avoiding new kinds of traces; id rather see no error than a new kind of error. A very close second is seeing useful ones :-)
Somewhat OT: The above stack traces are a great reason to invest into moving even more of the loader into C++. None of these frames are remotely helpful to understand what's going on:
SyntaxError: Unexpected string at Loader.moduleStrategy (internal/modules/esm/translators.js:83:18) at async link (internal/modules/esm/module_job.js:37:21)Compared with the - theoretical - error showing the syntax error location as the top frame and without any JS frames from loader internals:
SyntaxError: Unexpected string at file:///path/to/project/syntax-error.mjs:4:2Reacted by Jordan Harband, Alex Yang, Anatoly Ressin, Timo Tijhof and await-ovoReacted by Anna Henningsen, Alex Yang and Rohit@jkrems If the issue is that there are C++ frames in the middle of a stack, cutting it off at some point, I’d prefer to come up with a solution that provides the full stack trace rather than moving more code into C++…
Reacted by Jordan Harband and Jan Olaf MartinI think to me it's related to a direct comparison with browser stacks which is "suddenly" a thing for modules. With most other APIs (from
setTimeout()torequire()), there's user code calling JavaScript functions. So seeing JS frames of node's implementation doesn't feel too weird to me.But module execution feels different. I didn't create a module job, I ran a file. In the browser, it wouldn't show me stack frames from network requests and other kinds of orchestration needed to run the file. So seeing that in node appears inconsistent.
@jkrems Right, but I feel like “move code to a different language” is ultimately not a good solution to stack traces not being pretty enough – I’d rather find a way to provide better stack traces. In my opinion, we already have far too much module-related C++ code that would be more accessible when written in JS.
Reacted by Jan Olaf Martin, Jordan Harband, Evan Plaice, Alex Yang and Ray FossIt’s always annoyed me that Node internals appear in stack traces. I wish we could just filter them out, or blackbox them, maybe by default which could be changed via a flag.
@addaleax That's definitely fair. Maybe what's missing is some API that allows us to start module execution with a "fresh stack" even when there's technically active JS frames. Feels like something that'd need V8 support if they would also appear "properly" within the module execution itself (try/catch)..?
It’s always annoyed me that Node internals appear in stack traces. I wish we could just filter them out, or blackbox them, maybe by default which could be changed via a flag.
@GeoffreyBooth I generally disagree. Seeing frames for the express subclass of
http.Serverbut not for the parts that happen to be from the base class feels super confusing. In most cases having node's own code appear is pretty valuable imo.The function implementation hiding proposal, which allows functions to opt out of stack frames, may provide a JS-native solution to this problem.
The function implementation hiding proposal, which allows functions to opt out of stack frames, may provide a JS-native solution to this problem.
Maybe. Though it feels a bit brittle. I assume it would mean that we'd have to manually hide each individual function that makes up our implementation and be careful that we never have
Array.forEach& friends on the stack. Possible but a bit too easy to break by accident for my taste.Reacted by Jordan HarbandI can understand arguments for and against node.js internals appearing in the stack, but that's not why I opened this issue. My concern is not so much the extra information, it's the lack of necessary information. Filtering out node.js internals will not tell me where the syntax error occurred. Normally I use a linter but in some cases when testing a specific issue I might run the tests manually and bypass the lint step (that's how I found this issue).
I know @jkrems suggested that this could be a v8 issue but I find it interesting that REPL top-level await and strict unhandled rejections both display the necessary information. This tells me that the information exists as part of the error object at some point. Seems like that information can only be displayed by the default
uncaughtExceptionhandler?function example() { throw new Error('example'); } if (process.env.CATCH_ERROR) { try { example(); } catch (error) { console.error(error); } } else { example(); }
Running this with
CATCH_ERROR=1removes the source output though in this example it's less of an issue as the stack trace contains a line referencing the location of throw inexample().Yes, it can “always” appear on crashes and that’s definitely a good improvement. The issue is that it won’t appear on things that don’t crash or where errors are sent to reporting tools (e.g. log files). Making it part of the default unhandled rejection logging is a good quick fix though!
This bug is a big obstacle for me during development. I need to load apps dynamically, so any syntax error in any of my code will now not show anymore. It's very difficult to find errors without knowing where exactly the error is. This is completely unworkable.
My workaround is to manually add hardcoded import statements to the files that I develop on at a given time. But that's cumbersome, tedious, and I need to do it manually every time. All the time. And this is only after I found the cause of why the stack is wrong. Many other developers would not even know why the stack is mis-attributing and would just tear their hairs without ever knowing why it failed.
As @coreyfarrell said, because stacks are so important, even async/await processing impl found some way to preserve the stack, so I assume there must be some kind of workaround possible for this problem as well.
This bug here is a serious problem for any usage of
import().Reacted by Starbeamrainbowlabs, Anatoly Ressin, AⱯ and nfmshow21 remaining items
This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.Reacted by Ben BuckschStill relevant.
Reacted by Ben Bucksch, Matt Stephenson, Pierre-Yves Bigourdan and David EdellThis issue has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.Still relevant.
Perform a dynamic import with
node -e "import('./syntax-error.mjs').catch(console.error)", the output does not help identify the error and could be seen by users as if node.js itself had a SyntaxError:Now create and run a loader.mjs script:
Running loader.mjs produces useful output:
I was not able to find a way to catch the error of a dynamic import and show the complete SyntaxError. Interestingly
node --unhandled-rejections=strict -e "import('./syntax-error.mjs')"or runningawait import('./syntax-error.mjs')fromnode --experimental-repl-awaitboth display the location of the SyntaxError.