[bug] --abort-on-uncaught-exception causes containsModuleSyntax to improperly throw Uncaught SyntaxError: Cannot use import statement outside a module #50878

Activity

1. KidkArolis commented on Nov 23, 2023

   

   KidkArolis

   on Nov 23, 2023

   Author

   More actions

   Let me know if you need more details on how to reproduce, I could perhaps create a small repo to show the issue if it helps.
2. aduh95 commented on Nov 23, 2023

   

   aduh95

   on Nov 23, 2023

   Contributor

   More actions

   Could you produce a repro that doesn't include downloading any external code? If not, you should report it to the dependency you are using.
3. KidkArolis commented on Nov 23, 2023

   

   KidkArolis

   on Nov 23, 2023

   Author

   More actions

   Working on reproducing it in a standalone file, but this behaviour change happens strictly if running this with Node.js 20.10.0:

   # Node 20.9.0 - works
   NODE_OPTIONS="--abort-on-uncaught-exception" ./node_modules/.bin/knex migrate:make test
   
   # Node 20.10.0 - does not work
   NODE_OPTIONS="--abort-on-uncaught-exception" ./node_modules/.bin/knex migrate:make test
   
   # Node 20.10.0 - works when no flag passed
   ./node_modules/.bin/knex migrate:make test
   
4. tjenkinson commented on Nov 23, 2023

   

   tjenkinson

   on Nov 23, 2023

   Last edited by tjenkinson

   More actions

   I've put together a repo here: https://github.com/tjenkinson/node-20.10.0-ERR_REQUIRE_ESM-crash

   It seems instead of the ERR_REQUIRE_ESM error being thrown now when esm is require'd it crashes.

   From the debugger it looks like it might be a crash happening in the internal containsModuleSyntax module?
5. tjenkinson commented on Nov 23, 2023

   

   tjenkinson

   on Nov 23, 2023

   More actions

   Just pushed a new commit to that repro that changes it slightly. It seems to be when it's a module in node_modules that it trips up.
6. tjenkinson commented on Nov 23, 2023

   

   tjenkinson

   on Nov 23, 2023

   Last edited by tjenkinson

   More actions

   Looks like the crash is here:

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

   Line 1406 in 4e713a3

   const usesEsm = containsModuleSyntax(content, filename);

   And this was added in a9b2535
7. targos commented on Nov 23, 2023

   

   targos

   on Nov 23, 2023

   Member

   More actions

   Here's a C++ backtrace:

     * frame #0: 0x0000000102254f78 node`v8::base::OS::Abort() [inlined] v8::base::OS::Abort()::$_0::operator()(this=<unavailable>) const at platform-posix.cc:698:5 [opt]
       frame #1: 0x0000000102254f78 node`v8::base::OS::Abort() at platform-posix.cc:698:5 [opt]
       frame #2: 0x000000010095ef6c node`v8::internal::Isolate::CreateMessageOrAbort(this=<unavailable>, exception=<unavailable>, location=<unavailable>) at isolate.cc:1799:7 [opt]
       frame #3: 0x000000010095e0e4 node`v8::internal::Isolate::ThrowInternal(this=0x0000000128008000, raw_exception=Tagged<v8::internal::Object> @ x24, location=0x000000016fdfc390) at isolate.cc:1888:36 [opt]
       frame #4: 0x000000010095d9ac node`v8::internal::Isolate::ThrowAt(this=0x0000000128008000, exception=Handle<v8::internal::JSObject> @ x21, location=0x000000016fdfc390) at isolate.cc:1655:10 [opt]
       frame #5: 0x0000000101049728 node`v8::internal::PendingCompilationErrorHandler::ThrowPendingError(this=<unavailable>, isolate=0x0000000128008000, script=Handle<v8::internal::Script> @ x21) const at pending-compilation-error-handler.cc:200:12 [opt]
       frame #6: 0x00000001007e018c node`v8::internal::(anonymous namespace)::CompileToplevel(v8::internal::ParseInfo*, v8::internal::Handle<v8::internal::Script>, v8::internal::MaybeHandle<v8::internal::ScopeInfo>, v8::internal::Isolate*, v8::internal::IsCompiledScope*) [inlined] v8::internal::(anonymous namespace)::FailWithPreparedPendingException(isolate=0x0000000128008000, script=Handle<v8::internal::Script> @ x20, pending_error_handler=0x000000016fdfc750, flag=KEEP_EXCEPTION) at compiler.cc:1437:30 [opt]
       frame #7: 0x00000001007e0174 node`v8::internal::(anonymous namespace)::CompileToplevel(v8::internal::ParseInfo*, v8::internal::Handle<v8::internal::Script>, v8::internal::MaybeHandle<v8::internal::ScopeInfo>, v8::internal::Isolate*, v8::internal::IsCompiledScope*) [inlined] v8::internal::(anonymous namespace)::FailWithPendingException(isolate=0x0000000128008000, script=Handle<v8::internal::Script> @ x20, parse_info=0x000000016fdfc5b8, flag=KEEP_EXCEPTION) at compiler.cc:1449:10 [opt]
       frame #8: 0x00000001007e0174 node`v8::internal::(anonymous namespace)::CompileToplevel(parse_info=0x000000016fdfc5b8, script=Handle<v8::internal::Script> @ x20, maybe_outer_scope_info=MaybeHandle<v8::internal::ScopeInfo> @ x23, isolate=0x0000000128008000, is_compiled_scope=<unavailable>) at compiler.cc:1571:5 [opt]
       frame #9: 0x00000001007e3e30 node`v8::internal::Compiler::GetWrappedFunction(source=<unavailable>, arguments=<unavailable>, context=<unavailable>, script_details=0x000000016fdfc858, cached_data=<unavailable>, compile_options=<unavailable>, no_cache_reason=<unavailable>) at compiler.cc:3810:20 [opt]
       frame #10: 0x000000010061521c node`v8::ScriptCompiler::CompileFunctionInternal(v8_context=<unavailable>, source=0x000000016fdfcda0, arguments_count=<unavailable>, arguments=0x00006000009650e0, context_extension_count=<unavailable>, context_extensions=<unavailable>, options=kNoCompileOptions, no_cache_reason=kNoCacheNoReason, script_or_module_out=0x0000000000000000) at api.cc:2724:10 [opt]
       frame #11: 0x0000000100614bc4 node`v8::ScriptCompiler::CompileFunction(context=Local<v8::Context> @ x0, source=<unavailable>, arguments_count=<unavailable>, arguments=0x00006000009650e0, context_extension_count=<unavailable>, context_extensions=<unavailable>, options=kNoCompileOptions, no_cache_reason=<unavailable>) at api.cc:2646:10 [opt]
       frame #12: 0x000000010024a030 node`node::contextify::ContextifyContext::CompileFunctionAndCacheResult(env=0x0000000121834000, parsing_context=Local<v8::Context> @ 0x000000016fdfcb50, source=0x000000016fdfcda0, params=size=5, context_extensions=size=0, options=kNoCompileOptions, produce_cached_data=true, id_symbol=Local<v8::Symbol> @ 0x000000016fdfcb48, try_catch=0x000000016fdfccf0) at node_contextify.cc:1345:35
       frame #13: 0x000000010024695c node`node::contextify::ContextifyContext::ContainsModuleSyntax(args=0x000000016fdfd788) at node_contextify.cc:1472:3
   
8. targos commented on Nov 23, 2023

   

   targos

   on Nov 23, 2023

   Member

   More actions

   The crash happens within a TryCatch scope, so it seems like a V8 bug. It shouldn't abort in this case.
9. targos commented on Nov 23, 2023

   

   targos

   on Nov 23, 2023

   Member

   More actions

   I added some logs to the PredictExceptionCatcher and it returns here:

   node/deps/v8/src/execution/isolate.cc

   Lines 2437 to 2438 in 4e713a3

   	// Handler not found.
   	return NOT_CAUGHT;

   /cc @nodejs/cpp-reviewers
10. joyeecheung commented on Nov 30, 2023

    

    joyeecheung

    on Nov 30, 2023

    Last edited by joyeecheung

    Member

    More actions

    This looks like a negligence from our side of not disabling aborting temporarily for the syntax check in our ShouldAbortOnUncaughtException callback.
11. joyeecheung commented on Nov 30, 2023

    

    joyeecheung

    on Nov 30, 2023

    Member

    More actions

    cc @nodejs/loaders
12. joyeecheung

    added

    loadersIssues and PRs related to ES module loaders.Issues and PRs related to ES module loaders.

    on Nov 30, 2023
13. GeoffreyBooth

    added

    moduleIssues and PRs related to the module subsystem.Issues and PRs related to the module subsystem.

    esmIssues and PRs related to the ECMAScript Modules implementation.Issues and PRs related to the ECMAScript Modules implementation.

    and removed

    loadersIssues and PRs related to ES module loaders.Issues and PRs related to ES module loaders.

    on Nov 30, 2023
14. anonrig commented on Dec 1, 2023

    

    anonrig

    on Dec 1, 2023

    Member

    More actions

    @KidkArolis I opened a PR that fixes this issue.

    🎉React with 🎉3Reacted by Tom Jenkinson, Karolis Narkevicius and Paul "Joey" Clark
15. targos commented on Dec 1, 2023

    

    targos

    on Dec 1, 2023

    Member

    More actions

    @joyeecheung how do you explain that PredictExceptionCatcher doesn't find the TryCatch scope?
16. joyeecheung commented on Dec 1, 2023

    

    joyeecheung

    on Dec 1, 2023

    Member

    More actions

    how do you explain that PredictExceptionCatcher doesn't find the TryCatch scope?

    I think that's for JavaScript try {} catch {}, not v8::TryCatch.

    👍React with 👍1Reacted by Michaël Zasso
17. GeoffreyBooth

    added

    confirmed-bugIssues and PRs for confirmed bugs.Issues and PRs for confirmed bugs.

    on Dec 3, 2023
18. GeoffreyBooth

    changed the title [-]Uncaught SyntaxError: Cannot use import statement outside a module in 20.10.0[/-] [+][bug] `--abort-on-uncaught-exception` causes `containsModuleSyntax` to improperly throw `Uncaught SyntaxError: Cannot use import statement outside a module`[/+] on Dec 3, 2023
19. nodejs-github-bot

    closed this as completedin #50987on Dec 3, 2023