OS12095746: Missed ModuleReady callback

This change addresses three issue:

1) Early error in root module
Chakra fails to call host's ModuleReady callback when it encounters
an early error during parsing of a root module (no parent)
Fix: add 'ModuleReady' notification to ParseSource()

2) Error in dependent module used by a 2nd root module
Error in a dependent module is bubbled up the hierarchy and notified
through the root module (or the dynamically-imported module). The 2nd
encounter of the dependent module referenced by a 2nd root module fails
to trigger a ModuleReady notification to the host
Fix: add checks for dependent modules' errorObject and report failure through HRESULT

3) import("mod.js").then().catch()
This sematic should be supported.
Fix: add parsing for post-fix operators for "import()" code path in parser.

bin/ch/Helpers.cpp

@@ -139,7 +139,7 @@ HRESULT Helpers::LoadScriptFromFile(LPCSTR filename, LPCSTR& contents, UINT* len
     //
     if (fopen_s(&file, filename, "rb") != 0)
     {
-        if (!HostConfigFlags::flags.AsyncModuleLoadIsEnabled)
+        if (!HostConfigFlags::flags.MuteHostErrorMsgIsEnabled)
         {
 #ifdef _WIN32
             DWORD lastError = GetLastError();

bin/ch/HostConfigFlagsList.h

@@ -11,7 +11,9 @@ FLAG(int,  InspectMaxStringLength,          "Max string length to dump in locals
 FLAG(BSTR, Serialized,                      "If source is UTF8, deserializes from bytecode file", NULL)
 FLAG(bool, OOPJIT,                          "Run JIT in a separate process", false)
 FLAG(bool, EnsureCloseJITServer,            "JIT process will be force closed when ch is terminated", true)
-FLAG(bool, AsyncModuleLoad,                 "Silence host error output for module load failures to enable promise testing", false)
+FLAG(bool, IgnoreScriptErrorCode,           "Don't return error code on script error", false)
+FLAG(bool, MuteHostErrorMsg,                "Mute host error output, e.g. module load failures", false)
+FLAG(bool, TraceHostCallback,               "Output traces for host callbacks", false)
 FLAG(bool, $262,                            "load $262 harness", false)
 #undef FLAG
 #endif

bin/ch/WScriptJsrt.cpp

@@ -362,7 +362,7 @@ JsErrorCode WScriptJsrt::LoadModuleFromString(LPCSTR fileName, LPCSTR fileConten
     unsigned int fileContentLength = (fileContent == nullptr) ? 0 : (unsigned int)strlen(fileContent);
     errorCode = ChakraRTInterface::JsParseModuleSource(requestModule, dwSourceCookie, (LPBYTE)fileContent,
         fileContentLength, JsParseModuleSourceFlags_DataIsUTF8, &errorObject);
-    if ((errorCode != JsNoError) && errorObject != JS_INVALID_REFERENCE && fileContent != nullptr)
+    if ((errorCode != JsNoError) && errorObject != JS_INVALID_REFERENCE && fileContent != nullptr && !HostConfigFlags::flags.IgnoreScriptErrorCode)
     {
         ChakraRTInterface::JsSetException(errorObject);
         return errorCode;
@@ -1275,6 +1275,11 @@ bool WScriptJsrt::PrintException(LPCSTR fileName, JsErrorCode jsErrorCode)
     LPCWSTR errorTypeString = ConvertErrorCodeToMessage(jsErrorCode);
     JsValueRef exception;
     ChakraRTInterface::JsGetAndClearException(&exception);
+    if (HostConfigFlags::flags.MuteHostErrorMsgIsEnabled)
+    {
+        return false;
+    }
+
     if (exception != nullptr)
     {
         if (jsErrorCode == JsErrorCode::JsErrorScriptCompile || jsErrorCode == JsErrorCode::JsErrorScriptException)
@@ -1473,7 +1478,7 @@ HRESULT WScriptJsrt::ModuleMessage::Call(LPCSTR fileName)
             hr = Helpers::LoadScriptFromFile(specifierStr.GetString(), fileContent);
             if (FAILED(hr))
             {
-                if (!HostConfigFlags::flags.AsyncModuleLoadIsEnabled)
+                if (!HostConfigFlags::flags.MuteHostErrorMsgIsEnabled)
                 {
                     fprintf(stderr, "Couldn't load file.\n");
                 }
@@ -1558,6 +1563,12 @@ JsErrorCode WScriptJsrt::NotifyModuleReadyCallback(_In_opt_ JsModuleRecord refer
         {
             fileName.Initialize(specifier);
         }
+
+        if (HostConfigFlags::flags.TraceHostCallbackIsEnabled)
+        {
+            printf("NotifyModuleReadyCallback(exception) %s\n", fileName.GetString());
+        }
+
         PrintException(*fileName, JsErrorScriptException);
     }
     else

lib/Parser/Parse.cpp

@@ -2465,7 +2465,10 @@ ParseNodePtr Parser::ParseImport()
     // import()
     if (m_token.tk == tkLParen)
     {
-        return ParseImportCall<buildAST>();
+        ParseNodePtr pnode = ParseImportCall<buildAST>();
+        BOOL fCanAssign;
+        IdentToken token;
+        return ParsePostfixOperators<buildAST>(pnode, TRUE, FALSE, FALSE, &fCanAssign, &token);
     }
 
     m_pscan->SeekTo(parsedImport);
@@ -3260,12 +3263,8 @@ LFunction :
         if (m_scriptContext->GetConfig()->IsES6ModuleEnabled())
         {
             m_pscan->Scan();
-            if (m_token.tk == tkLParen)
-            {
-                return ParseImportCall<buildAST>();
-            }
-
-            Error(ERRnoLparen);
+            ChkCurTokNoScan(tkLParen, ERRnoLparen);
+            pnode = ParseImportCall<buildAST>();
         }
         else
         {

lib/Runtime/Language/SourceTextModuleRecord.cpp

@@ -84,11 +84,17 @@ namespace Js
 
     HRESULT SourceTextModuleRecord::ParseSource(__in_bcount(sourceLength) byte* sourceText, uint32 sourceLength, SRCINFO * srcInfo, Var* exceptionVar, bool isUtf8)
     {
-        OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("ParseSource(%s)\n"), this->GetSpecifierSz());
-        Assert(!wasParsed);
+        Assert(!wasParsed || sourceText == nullptr);
         Assert(parser == nullptr);
         Assert(exceptionVar != nullptr);
         HRESULT hr = NOERROR;
+
+        // Return if loading failure has been reported
+        if (sourceText == nullptr && wasParsed)
+        {
+            return hr;
+        }
+
         ScriptContext* scriptContext = GetScriptContext();
         CompileScriptException se;
         ArenaAllocator* allocator = scriptContext->GeneralAllocator();
@@ -100,10 +106,12 @@ namespace Js
             *exceptionVar = pError;
             return E_NOTIMPL;
         }
+
         // Host indicates that the current module failed to load.
         if (sourceText == nullptr)
         {
             Assert(sourceLength == 0);
+            OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("Failed to load: %s\n"), this->GetSpecifierSz());
             hr = E_FAIL;
             JavascriptError *pError = scriptContext->GetLibrary()->CreateURIError();
             JavascriptError::SetErrorMessageProperties(pError, hr, this->GetSpecifierSz(), scriptContext);
@@ -111,6 +119,7 @@ namespace Js
         }
         else
         {
+            OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("ParseSource(%s)\n"), this->GetSpecifierSz());
             try
             {
                 AUTO_NESTED_HANDLED_EXCEPTION_TYPE((ExceptionType)(ExceptionType_OutOfMemory | ExceptionType_StackOverflow));
@@ -157,6 +166,7 @@ namespace Js
                 }
             }
         }
+
         if (FAILED(hr))
         {
             if (*exceptionVar == nullptr)
@@ -174,12 +184,26 @@ namespace Js
                 this->errorObject = *exceptionVar;
             }
 
-            OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>NotifyParentAsNeeded\n"));
             if (this->promise != nullptr)
             {
                 SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this);
             }
 
+            // Notify host if current module is dynamically-loaded module, or is root module and the host hasn't been notified
+            bool isScriptActive = scriptContext->GetThreadContext()->IsScriptActive();
+            if (this->promise != nullptr || (isRootModule && !hadNotifyHostReady))
+            {
+                OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>NotifyHostAboutModuleReady %s (ParseSource error)\n"), this->GetSpecifierSz());
+                LEAVE_SCRIPT_IF(scriptContext, isScriptActive,
+                {
+                    scriptContext->GetHostScriptContext()->NotifyHostAboutModuleReady(this, this->errorObject);
+                });
+
+                hadNotifyHostReady = true;
+            }
+
+            // Notify parent if applicable
+            OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>NotifyParentAsNeeded\n"));
             NotifyParentsAsNeeded();
         }
         return hr;
@@ -251,6 +275,7 @@ namespace Js
                     {
                         bool isScriptActive = scriptContext->GetThreadContext()->IsScriptActive();
 
+                        OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>NotifyHostAboutModuleReady %s (PostProcessDynamicModuleImport)\n"), this->GetSpecifierSz());
                         LEAVE_SCRIPT_IF(scriptContext, isScriptActive,
                         {
                             hr = scriptContext->GetHostScriptContext()->NotifyHostAboutModuleReady(this, this->errorObject);
@@ -293,6 +318,7 @@ namespace Js
                 ModuleDeclarationInstantiation();
                 if (!hadNotifyHostReady && !WasEvaluated())
                 {
+                    OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>NotifyHostAboutModuleReady %s (PrepareForModuleDeclarationInitialization)\n"), this->GetSpecifierSz());
                     LEAVE_SCRIPT_IF(scriptContext, isScriptActive,
                     {
                         hr = scriptContext->GetHostScriptContext()->NotifyHostAboutModuleReady(this, this->errorObject);
@@ -335,6 +361,7 @@ namespace Js
 
             if (this->promise != nullptr || (isRootModule && !hadNotifyHostReady))
             {
+                OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>NotifyHostAboutModuleReady %s (OnChildModuleReady)\n"), this->GetSpecifierSz());
                 LEAVE_SCRIPT_IF(scriptContext, isScriptActive,
                 {
                     hr = scriptContext->GetHostScriptContext()->NotifyHostAboutModuleReady(this, this->errorObject);
@@ -710,16 +737,30 @@ namespace Js
                         return true;
                     }
                     moduleRecord = SourceTextModuleRecord::FromHost(moduleRecordBase);
-                    OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>SetParent in (%s)\n"), moduleRecord->GetSpecifierSz());
-                    moduleRecord->SetParent(this, moduleName);
+                    Var errorObject = moduleRecord->GetErrorObject();
+                    if (errorObject == nullptr)
+                    {
+                        OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>SetParent in (%s)\n"), moduleRecord->GetSpecifierSz());
+                        moduleRecord->SetParent(this, moduleName);
+                    }
+                    else
+                    {
+                        this->errorObject = errorObject;
+                        hr = E_FAIL;
+                        return true;
+                    }
                 }
                 return false;
             });
             if (FAILED(hr))
             {
-                JavascriptError *error = scriptContext->GetLibrary()->CreateError();
-                JavascriptError::SetErrorMessageProperties(error, hr, _u("fetch import module failed"), scriptContext);
-                this->errorObject = error;
+                if (this->errorObject == nullptr)
+                {
+                    JavascriptError *error = scriptContext->GetLibrary()->CreateError();
+                    JavascriptError::SetErrorMessageProperties(error, hr, _u("fetch import module failed"), scriptContext);
+                    this->errorObject = error;
+                }
+
                 OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\tfetch import module failed\n"));
                 NotifyParentsAsNeeded();
             }

test/es6/bug_OS12095746.baseline

@@ -0,0 +1,6 @@
+NotifyModuleReadyCallback(exception) bug_OS12095746_mod0.js
+NotifyModuleReadyCallback(exception) bug_OS12095746_mod1.js
+mod0 catch:Syntax error
+mod1 catch:Expected ';'
+NotifyModuleReadyCallback(exception) bug_OS12095746_mod2.js
+mod2 catch:Expected ';'

test/es6/bug_OS12095746.js

@@ -0,0 +1,17 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+import("bug_OS12095746_mod0.js")
+    .then((m)=>{ console.log('mod0 fail'); })
+    .catch((e)=>{ console.log("mod0 catch:"+e.message); });
+
+import('bug_OS12095746_mod1.js')
+    .then((m)=>{ console.log('mod1 fail'); })
+    .catch((e)=>{
+        console.log('mod1 catch:'+e.message);
+        import('bug_OS12095746_mod2.js')
+            .then((m)=>{ print('mod2 fail'); })
+            .catch((e)=>{ print('mod2 catch:'+e.message); });
+        });

test/es6/bug_OS12095746_mod0.js

@@ -0,0 +1,7 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+var foo = null;
+export foo;

test/es6/bug_OS12095746_mod1.js

@@ -0,0 +1,6 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+import test from "./bug_OS12095746_moddep.js";

test/es6/bug_OS12095746_mod2.js

@@ -0,0 +1,6 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+import test from "./bug_OS12095746_moddep.js";

test/es6/bug_OS12095746_moddep.js

@@ -0,0 +1,7 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+// Parse error in a dependent module
+1A

test/es6/rlexe.xml

@@ -1286,8 +1286,8 @@
 <test>
   <default>
     <files>await-futreserved-only-in-modules.js</files>
-    <compile-flags>-ES6Module</compile-flags>
-    <tags>exclude_dynapogo,exclude_xplat</tags>
+    <compile-flags>-MuteHostErrorMsg -ES6Module</compile-flags>
+    <tags>exclude_dynapogo, exclude_xplat</tags>
   </default>
 </test>
 <test>
@@ -1307,7 +1307,7 @@
 <test>
     <default>
         <files>module-syntax.js</files>
-        <compile-flags>-ES6Module -args summary -endargs</compile-flags>
+        <compile-flags>-MuteHostErrorMsg -ES6Module -args summary -endargs</compile-flags>
         <tags>exclude_dynapogo,exclude_sanitize_address</tags>
     </default>
 </test>
@@ -1321,7 +1321,7 @@
 <test>
     <default>
         <files>module-functionality.js</files>
-        <compile-flags>-ES6Module -args summary -endargs</compile-flags>
+        <compile-flags>-MuteHostErrorMsg -ES6Module -args summary -endargs</compile-flags>
         <tags>exclude_dynapogo,exclude_sanitize_address</tags>
     </default>
 </test>
@@ -1335,14 +1335,14 @@
 <test>
     <default>
         <files>dynamic-module-import-specifier.js</files>
-        <compile-flags>-AsyncModuleLoad -ES6Module -args summary -endargs</compile-flags>
+        <compile-flags>-MuteHostErrorMsg -ES6Module -args summary -endargs</compile-flags>
         <tags>exclude_sanitize_address</tags>
     </default>
 </test>
 <test>
     <default>
         <files>module-syntax.js</files>
-        <compile-flags>-ES6Module -force:deferparse -args summary -endargs</compile-flags>
+        <compile-flags>-MuteHostErrorMsg -ES6Module -force:deferparse -args summary -endargs</compile-flags>
         <tags>exclude_sanitize_address</tags>
     </default>
 </test>
@@ -1367,6 +1367,14 @@
         <tags>exclude_dynapogo,exclude_sanitize_address,bugfix</tags>
     </default>
 </test>
+<test>
+    <default>
+        <files>bug_OS12095746.js</files>
+        <compile-flags>-MuteHostErrorMsg -IgnoreScriptErrorCode -TraceHostCallback -ES6Module</compile-flags>
+        <tags>exclude_dynapogo,exclude_sanitize_address,bugfix</tags>
+        <baseline>bug_OS12095746.baseline</baseline>
+    </default>
+</test>
 <test>
     <default>
         <files>test_bug_2645.js</files>