From 5d805a1d4dacd33964c9cb8e496e4a3583bc279c Mon Sep 17 00:00:00 2001 From: islandryu Date: Mon, 9 Jun 2025 22:53:43 +0900 Subject: [PATCH 1/8] module: correctly detect top-level await in ambiguous contexts Fixes: https://github.com/nodejs/node/issues/58331 --- src/node_contextify.cc | 16 ++++ test/es-module/test-esm-detect-ambiguous.mjs | 81 ++++++++++++++++---- 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index c4db3eb076d49b..a2b886e0860d51 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1647,6 +1647,12 @@ static const auto throws_only_in_cjs_error_messages = "await is only valid in async functions and " "the top level bodies of modules"}; +static std::vector maybe_top_level_await_errors = { + // example: `func(await 1);` + "missing ) after argument list", + // example: `if(await 1)` + "SyntaxError: Unexpected"}; + // If cached_data is provided, it would be used for the compilation and // the on-disk compilation cache from NODE_COMPILE_CACHE (if configured) // would be ignored. @@ -1877,6 +1883,16 @@ bool ShouldRetryAsESM(Realm* realm, break; } } + + for (const auto& error_message : maybe_top_level_await_errors) { + if (message_view.find(error_message) != std::string_view::npos) { + // If the error message is related to top-level await, we can try to + // compile it as ESM. + maybe_valid_in_esm = true; + break; + } + } + if (!maybe_valid_in_esm) { return false; } diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs index a583da981fed5c..ef0a6bfc368b59 100644 --- a/test/es-module/test-esm-detect-ambiguous.mjs +++ b/test/es-module/test-esm-detect-ambiguous.mjs @@ -427,22 +427,75 @@ describe('when working with Worker threads', () => { }); }); -describe('cjs & esm ambiguous syntax case', () => { - it('should throw an ambiguous syntax error when using top-level await with require', async () => { - const { stderr, code, signal } = await spawnPromisified( - process.execPath, - [ - '--input-type=module', - '--eval', - `await 1;\nconst fs = require('fs');`, - ] - ); +describe('maybe top-level await syntax errors that are not recognized as top-level await errors', () => { + const expressions = [ + // string + { expression: '""' }, + // number + { expression: '0' }, + // boolean + { expression: 'true' }, + // null + { expression: 'null' }, + // undefined + { expression: 'undefined' }, + // object + { expression: '{}' }, + // array + { expression: '[]' }, + // new + { expression: 'new Date()' }, + // identifier + { initialize: 'const a = 2;', expression: 'a' }, + ]; + it('should not crash the process', async () => { + for (const { expression, initialize } of expressions) { + const wrapperExpressions = [ + `function callAwait() {}; callAwait(await ${expression});`, + `if (await ${expression}) {}`, + `{ key: await ${expression} }`, + `[await ${expression}]`, + `(await ${expression})`, + ]; + for (const wrapperExpression of wrapperExpressions) { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--eval', + ` + ${initialize || ''} + ${wrapperExpression} + `, + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, ''); + strictEqual(code, 0); + strictEqual(signal, null); + } + } + }); - match( - stderr, - /ReferenceError: Cannot determine intended module format because both require\(\) and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, replace require\(\) with import\./ - ); + it('should crash when the expression is not valid', async () => { + let { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--eval', + ` + function callAwait() {} + callAwait(await "" ""); + `, + ]); + match(stderr, /SyntaxError: missing \) after argument list/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + ({ code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--eval', + ` + function callAwait() {} + if (a "") {} + `, + ])); + match(stderr, /SyntaxError: Unexpected string/); + strictEqual(stdout, ''); strictEqual(code, 1); strictEqual(signal, null); }); From a8a3a33e3b162ea93e535fe28bba907560fcf9c1 Mon Sep 17 00:00:00 2001 From: islandryu Date: Tue, 10 Jun 2025 23:51:59 +0900 Subject: [PATCH 2/8] use std::array --- src/node_contextify.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index a2b886e0860d51..d308f10e005426 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1647,11 +1647,11 @@ static const auto throws_only_in_cjs_error_messages = "await is only valid in async functions and " "the top level bodies of modules"}; -static std::vector maybe_top_level_await_errors = { - // example: `func(await 1);` - "missing ) after argument list", - // example: `if(await 1)` - "SyntaxError: Unexpected"}; +static const auto maybe_top_level_await_errors = + std::array{// example: `func(await 1);` + "missing ) after argument list", + // example: `if(await 1)` + "SyntaxError: Unexpected"}; // If cached_data is provided, it would be used for the compilation and // the on-disk compilation cache from NODE_COMPILE_CACHE (if configured) From 9d6a322a5abb583e480e5e0c083a6a1653c70004 Mon Sep 17 00:00:00 2001 From: islandryu Date: Mon, 16 Jun 2025 21:29:52 +0900 Subject: [PATCH 3/8] move test --- test/es-module/test-esm-detect-ambiguous.mjs | 74 ------------------ ...tax-errors-not-recognized-as-tla-error.mjs | 77 +++++++++++++++++++ 2 files changed, 77 insertions(+), 74 deletions(-) create mode 100644 test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs index ef0a6bfc368b59..8199fa46f737a1 100644 --- a/test/es-module/test-esm-detect-ambiguous.mjs +++ b/test/es-module/test-esm-detect-ambiguous.mjs @@ -426,77 +426,3 @@ describe('when working with Worker threads', () => { strictEqual(signal, null); }); }); - -describe('maybe top-level await syntax errors that are not recognized as top-level await errors', () => { - const expressions = [ - // string - { expression: '""' }, - // number - { expression: '0' }, - // boolean - { expression: 'true' }, - // null - { expression: 'null' }, - // undefined - { expression: 'undefined' }, - // object - { expression: '{}' }, - // array - { expression: '[]' }, - // new - { expression: 'new Date()' }, - // identifier - { initialize: 'const a = 2;', expression: 'a' }, - ]; - it('should not crash the process', async () => { - for (const { expression, initialize } of expressions) { - const wrapperExpressions = [ - `function callAwait() {}; callAwait(await ${expression});`, - `if (await ${expression}) {}`, - `{ key: await ${expression} }`, - `[await ${expression}]`, - `(await ${expression})`, - ]; - for (const wrapperExpression of wrapperExpressions) { - const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--eval', - ` - ${initialize || ''} - ${wrapperExpression} - `, - ]); - - strictEqual(stderr, ''); - strictEqual(stdout, ''); - strictEqual(code, 0); - strictEqual(signal, null); - } - } - }); - - it('should crash when the expression is not valid', async () => { - let { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--eval', - ` - function callAwait() {} - callAwait(await "" ""); - `, - ]); - match(stderr, /SyntaxError: missing \) after argument list/); - strictEqual(stdout, ''); - strictEqual(code, 1); - strictEqual(signal, null); - - ({ code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--eval', - ` - function callAwait() {} - if (a "") {} - `, - ])); - match(stderr, /SyntaxError: Unexpected string/); - strictEqual(stdout, ''); - strictEqual(code, 1); - strictEqual(signal, null); - }); -}); diff --git a/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs b/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs new file mode 100644 index 00000000000000..a9dc6785c45ecd --- /dev/null +++ b/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs @@ -0,0 +1,77 @@ +import { spawnPromisified } from '../common/index.mjs'; +import { describe, it } from 'node:test'; +import { strictEqual, match } from 'node:assert'; + +describe('maybe top-level await syntax errors that are not recognized as top-level await errors', () => { + const expressions = [ + // string + { expression: '""' }, + // number + { expression: '0' }, + // boolean + { expression: 'true' }, + // null + { expression: 'null' }, + // undefined + { expression: 'undefined' }, + // object + { expression: '{}' }, + // array + { expression: '[]' }, + // new + { expression: 'new Date()' }, + // identifier + { initialize: 'const a = 2;', expression: 'a' }, + ]; + it('should not crash the process', async () => { + for (const { expression, initialize } of expressions) { + const wrapperExpressions = [ + `function callAwait() {}; callAwait(await ${expression});`, + `if (await ${expression}) {}`, + `{ key: await ${expression} }`, + `[await ${expression}]`, + `(await ${expression})`, + ]; + for (const wrapperExpression of wrapperExpressions) { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--eval', + ` + ${initialize || ''} + ${wrapperExpression} + `, + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, ''); + strictEqual(code, 0); + strictEqual(signal, null); + } + } + }); + + it('should crash when the expression is not valid', async () => { + let { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--eval', + ` + function callAwait() {} + callAwait(await "" ""); + `, + ]); + match(stderr, /SyntaxError: missing \) after argument list/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + + ({ code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--eval', + ` + function callAwait() {} + if (a "") {} + `, + ])); + match(stderr, /SyntaxError: Unexpected string/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); +}); From c4f371b5e3a520af01606c244c063567b70ce3a4 Mon Sep 17 00:00:00 2001 From: islandryu Date: Sun, 22 Jun 2025 21:39:29 +0900 Subject: [PATCH 4/8] fix format --- src/node_contextify.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index d308f10e005426..bc3447109b900e 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1648,10 +1648,10 @@ static const auto throws_only_in_cjs_error_messages = "the top level bodies of modules"}; static const auto maybe_top_level_await_errors = - std::array{// example: `func(await 1);` - "missing ) after argument list", - // example: `if(await 1)` - "SyntaxError: Unexpected"}; + std::array{ + "missing ) after argument list", // example: `func(await 1);` + "SyntaxError: Unexpected" // example: `if(await 1)` + }; // If cached_data is provided, it would be used for the compilation and // the on-disk compilation cache from NODE_COMPILE_CACHE (if configured) From 2b3000ced4efff3a2760c38448262dfe0bde982c Mon Sep 17 00:00:00 2001 From: islandryu Date: Sun, 29 Jun 2025 18:07:55 +0900 Subject: [PATCH 5/8] add test for unrelated error --- ...tax-errors-not-recognized-as-tla-error.mjs | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs b/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs index a9dc6785c45ecd..156d6c07684c1d 100644 --- a/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs +++ b/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs @@ -49,29 +49,30 @@ describe('maybe top-level await syntax errors that are not recognized as top-lev } }); - it('should crash when the expression is not valid', async () => { - let { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--eval', - ` - function callAwait() {} - callAwait(await "" ""); - `, - ]); - match(stderr, /SyntaxError: missing \) after argument list/); - strictEqual(stdout, ''); - strictEqual(code, 1); - strictEqual(signal, null); + it('should throw the error for unrelated syntax errors', async () => { + const expression = "foo bar"; + const wrapperExpressions = [ + [`function callSyntaxError() {}; callSyntaxError(${expression});`, /missing \) after argument list/], + [`if (${expression}) {}`, /Unexpected identifier/], + [`{ key: ${expression} }`, /Unexpected identifier/], + [`[${expression}]`, /Unexpected identifier/], + [`(${expression})`, /Unexpected identifier/], + [`const ${expression} = 1;`, /Missing initializer in const declaration/], + [`console.log('PI: ' Math.PI);`, /missing \) after argument list/], + [`callAwait(await "" "");`, /missing \) after argument list/] + ]; - ({ code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--eval', - ` - function callAwait() {} - if (a "") {} - `, - ])); - match(stderr, /SyntaxError: Unexpected string/); - strictEqual(stdout, ''); - strictEqual(code, 1); - strictEqual(signal, null); + for (const [wrapperExpression, error] of wrapperExpressions) { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--eval', + ` + ${wrapperExpression} + `, + ]); + match(stderr, error); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + } }); }); From a60b3e04e120ffb71e02c78a7fe47daa748db203 Mon Sep 17 00:00:00 2001 From: islandryu Date: Sun, 29 Jun 2025 18:09:01 +0900 Subject: [PATCH 6/8] undo a test that has been erased --- test/es-module/test-esm-detect-ambiguous.mjs | 21 ++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs index 8199fa46f737a1..a583da981fed5c 100644 --- a/test/es-module/test-esm-detect-ambiguous.mjs +++ b/test/es-module/test-esm-detect-ambiguous.mjs @@ -426,3 +426,24 @@ describe('when working with Worker threads', () => { strictEqual(signal, null); }); }); + +describe('cjs & esm ambiguous syntax case', () => { + it('should throw an ambiguous syntax error when using top-level await with require', async () => { + const { stderr, code, signal } = await spawnPromisified( + process.execPath, + [ + '--input-type=module', + '--eval', + `await 1;\nconst fs = require('fs');`, + ] + ); + + match( + stderr, + /ReferenceError: Cannot determine intended module format because both require\(\) and top-level await are present\. If the code is intended to be CommonJS, wrap await in an async function\. If the code is intended to be an ES module, replace require\(\) with import\./ + ); + + strictEqual(code, 1); + strictEqual(signal, null); + }); +}); From e65cc6cc022bff7ea244953c94049d9bf41b2add Mon Sep 17 00:00:00 2001 From: islandryu Date: Sun, 29 Jun 2025 18:40:30 +0900 Subject: [PATCH 7/8] fix lint --- ...tax-errors-not-recognized-as-tla-error.mjs | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs b/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs index 156d6c07684c1d..54391a4b7f2014 100644 --- a/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs +++ b/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs @@ -50,29 +50,29 @@ describe('maybe top-level await syntax errors that are not recognized as top-lev }); it('should throw the error for unrelated syntax errors', async () => { - const expression = "foo bar"; - const wrapperExpressions = [ - [`function callSyntaxError() {}; callSyntaxError(${expression});`, /missing \) after argument list/], - [`if (${expression}) {}`, /Unexpected identifier/], - [`{ key: ${expression} }`, /Unexpected identifier/], - [`[${expression}]`, /Unexpected identifier/], - [`(${expression})`, /Unexpected identifier/], - [`const ${expression} = 1;`, /Missing initializer in const declaration/], - [`console.log('PI: ' Math.PI);`, /missing \) after argument list/], - [`callAwait(await "" "");`, /missing \) after argument list/] - ]; + const expression = 'foo bar'; + const wrapperExpressions = [ + [`function callSyntaxError() {}; callSyntaxError(${expression});`, /missing \) after argument list/], + [`if (${expression}) {}`, /Unexpected identifier/], + [`{ key: ${expression} }`, /Unexpected identifier/], + [`[${expression}]`, /Unexpected identifier/], + [`(${expression})`, /Unexpected identifier/], + [`const ${expression} = 1;`, /Missing initializer in const declaration/], + [`console.log('PI: ' Math.PI);`, /missing \) after argument list/], + [`callAwait(await "" "");`, /missing \) after argument list/], + ]; - for (const [wrapperExpression, error] of wrapperExpressions) { - const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--eval', - ` + for (const [wrapperExpression, error] of wrapperExpressions) { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--eval', + ` ${wrapperExpression} `, - ]); - match(stderr, error); - strictEqual(stdout, ''); - strictEqual(code, 1); - strictEqual(signal, null); - } + ]); + match(stderr, error); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + } }); }); From 993bcfc0b8698ce817292ed5784771f968d78829 Mon Sep 17 00:00:00 2001 From: Shima Ryuhei <65934663+islandryu@users.noreply.github.com> Date: Tue, 1 Jul 2025 12:06:44 +0900 Subject: [PATCH 8/8] Update test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs Co-authored-by: Joyee Cheung --- .../test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs b/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs index 54391a4b7f2014..f6d1ca962c94a3 100644 --- a/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs +++ b/test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs @@ -2,7 +2,7 @@ import { spawnPromisified } from '../common/index.mjs'; import { describe, it } from 'node:test'; import { strictEqual, match } from 'node:assert'; -describe('maybe top-level await syntax errors that are not recognized as top-level await errors', () => { +describe('unusual top-level await syntax errors', () => { const expressions = [ // string { expression: '""' },