[MERGE #4608 @rhuanjl] Implement HostPromiseRejectionTracker (#2530)

Merge pull request #4608 from rhuanjl:HostPromiseRejection

Responding to #2530

This PR contains:
1. Internal CC machinery for HostPromiseRejectionTracker [ecmaspec 25.4.1.9](https://tc39.github.io/ecma262/#sec-host-promise-rejection-tracker) - this should enable any host to implement a handler for uncaught promise rejections e.g. the method defined here [WhatW spec 8.1.3.12](https://html.spec.whatwg.org/multipage/webappapis.html#unhandled-promise-rejections)
2. A very simplistic implementation in ch (behind a command line flag)
3. Test case

I'm expecting feedback/changes before this is merged but wanted to get other people's thoughts on it in this state. Please give me your thoughts/comments.

Cross ref: [Edge uservoice request for this feature](https://wpdev.uservoice.com/forums/257854-microsoft-edge-developer/suggestions/16665397-detect-unhandled-promise-rejection?tracking_code=4f4e218df62afae7db37fb5be5f4ff1c) - note this PR will not put the feature in edge - it just creates the hooks the edge development team would have to track them through.

**cc** @liminzhu @saschanaz @fatcerberus @MSLaguana

**Notes:**
**1. Not spec compliant for "await" EDIT: snip - re-read spec this comment was wrong what I've done in this PR is correct**

**2. The ch implementation I've done is pretty bad,** it simply prints to the terminal whenever a notification is made, either handled or rejected - reasonable for testing that the interface is operational but certainly not what the WhatW spec would have a Host do.

**3. Parameters passed to the callback function** - per spec HostPromiseRejectionTracker should be passed the Promise and rejected/handled. I've gone for:
a) the promise
b) the promise's result - I know this could be retrieved from the promise object but I've added it for implementer's convenience
c) rejected/handled boolean - true for handled false for rejected - possibly should be an int or an enum of some kind

bin/ChakraCore/ChakraCore.def

@@ -62,3 +62,5 @@ JsLessThan
 JsLessThanOrEqual
 
 JsCreateEnhancedFunction
+
+JsSetHostPromiseRejectionTracker

bin/ch/ChakraRtInterface.cpp

@@ -120,6 +120,7 @@ bool ChakraRTInterface::LoadChakraDll(ArgInfo* argInfo, HINSTANCE *outLibrary)
     m_jsApiHooks.pfJsrtGetValueType = (JsAPIHooks::JsrtGetValueType)GetChakraCoreSymbol(library, "JsGetValueType");
     m_jsApiHooks.pfJsrtSetIndexedProperty = (JsAPIHooks::JsrtSetIndexedPropertyPtr)GetChakraCoreSymbol(library, "JsSetIndexedProperty");
     m_jsApiHooks.pfJsrtSetPromiseContinuationCallback = (JsAPIHooks::JsrtSetPromiseContinuationCallbackPtr)GetChakraCoreSymbol(library, "JsSetPromiseContinuationCallback");
+    m_jsApiHooks.pfJsrtSetHostPromiseRejectionTracker = (JsAPIHooks::JsrtSetHostPromiseRejectionTrackerPtr)GetChakraCoreSymbol(library, "JsSetHostPromiseRejectionTracker");
     m_jsApiHooks.pfJsrtGetContextOfObject = (JsAPIHooks::JsrtGetContextOfObject)GetChakraCoreSymbol(library, "JsGetContextOfObject");
     m_jsApiHooks.pfJsrtInitializeModuleRecord = (JsAPIHooks::JsInitializeModuleRecordPtr)GetChakraCoreSymbol(library, "JsInitializeModuleRecord");
     m_jsApiHooks.pfJsrtParseModuleSource = (JsAPIHooks::JsParseModuleSourcePtr)GetChakraCoreSymbol(library, "JsParseModuleSource");

bin/ch/ChakraRtInterface.h

@@ -54,6 +54,7 @@ struct JsAPIHooks
     typedef JsErrorCode (WINAPI *JsrtGetValueType)(JsValueRef value, JsValueType *type);
     typedef JsErrorCode (WINAPI *JsrtSetIndexedPropertyPtr)(JsValueRef object, JsValueRef index, JsValueRef value);
     typedef JsErrorCode (WINAPI *JsrtSetPromiseContinuationCallbackPtr)(JsPromiseContinuationCallback callback, void *callbackState);
+    typedef JsErrorCode (WINAPI *JsrtSetHostPromiseRejectionTrackerPtr)(JsHostPromiseRejectionTrackerCallback callback, void *callbackState);
     typedef JsErrorCode (WINAPI *JsrtGetContextOfObject)(JsValueRef object, JsContextRef *callbackState);
 
     typedef JsErrorCode(WINAPI *JsrtDiagStartDebugging)(JsRuntimeHandle runtimeHandle, JsDiagDebugEventCallback debugEventCallback, void* callbackState);
@@ -152,6 +153,7 @@ struct JsAPIHooks
     JsrtGetValueType pfJsrtGetValueType;
     JsrtSetIndexedPropertyPtr pfJsrtSetIndexedProperty;
     JsrtSetPromiseContinuationCallbackPtr pfJsrtSetPromiseContinuationCallback;
+    JsrtSetHostPromiseRejectionTrackerPtr pfJsrtSetHostPromiseRejectionTracker;
     JsrtGetContextOfObject pfJsrtGetContextOfObject;
     JsrtDiagStartDebugging pfJsrtDiagStartDebugging;
     JsrtDiagStopDebugging pfJsrtDiagStopDebugging;
@@ -356,6 +358,7 @@ class ChakraRTInterface
     static JsErrorCode WINAPI JsGetValueType(JsValueRef value, JsValueType *type) { return HOOK_JS_API(GetValueType(value, type)); }
     static JsErrorCode WINAPI JsSetIndexedProperty(JsValueRef object, JsValueRef index, JsValueRef value) { return HOOK_JS_API(SetIndexedProperty(object, index, value)); }
     static JsErrorCode WINAPI JsSetPromiseContinuationCallback(JsPromiseContinuationCallback callback, void *callbackState) { return HOOK_JS_API(SetPromiseContinuationCallback(callback, callbackState)); }
+    static JsErrorCode WINAPI JsSetHostPromiseRejectionTracker(JsHostPromiseRejectionTrackerCallback callback, void *callbackState) { return HOOK_JS_API(SetHostPromiseRejectionTracker(callback, callbackState)); }
     static JsErrorCode WINAPI JsGetContextOfObject(JsValueRef object, JsContextRef* context) { return HOOK_JS_API(GetContextOfObject(object, context)); }
     static JsErrorCode WINAPI JsDiagStartDebugging(JsRuntimeHandle runtimeHandle, JsDiagDebugEventCallback debugEventCallback, void* callbackState) { return HOOK_JS_API(DiagStartDebugging(runtimeHandle, debugEventCallback, callbackState)); }
     static JsErrorCode WINAPI JsDiagStopDebugging(JsRuntimeHandle runtimeHandle, void** callbackState) { return HOOK_JS_API(DiagStopDebugging(runtimeHandle, callbackState)); }

bin/ch/HostConfigFlagsList.h

@@ -15,5 +15,6 @@ FLAG(bool, IgnoreScriptErrorCode,           "Don't return error code on script e
 FLAG(bool, MuteHostErrorMsg,                "Mute host error output, e.g. module load failures", false)
 FLAG(bool, TraceHostCallback,               "Output traces for host callbacks", false)
 FLAG(bool, Test262,                         "load Test262 harness", false)
+FLAG(bool, TrackRejectedPromises,           "Enable tracking of unhandled promise rejections", false)
 #undef FLAG
 #endif

bin/ch/WScriptJsrt.cpp

@@ -1871,3 +1871,32 @@ void WScriptJsrt::PromiseContinuationCallback(JsValueRef task, void *callbackSta
     WScriptJsrt::CallbackMessage *msg = new WScriptJsrt::CallbackMessage(0, task);
     messageQueue->InsertSorted(msg);
 }
+
+void WScriptJsrt::PromiseRejectionTrackerCallback(JsValueRef promise, JsValueRef reason, bool handled, void *callbackState)
+{
+    Assert(promise != JS_INVALID_REFERENCE);
+    Assert(reason != JS_INVALID_REFERENCE);
+    JsValueRef strValue;
+    JsErrorCode error = ChakraRTInterface::JsConvertValueToString(reason, &strValue);
+
+    if (!handled)
+    {
+        wprintf(_u("Uncaught promise rejection\n"));
+    }
+    else
+    {
+        wprintf(_u("Promise rejection handled\n"));
+    }
+
+    if (error == JsNoError)
+    {
+        AutoString str(strValue);
+        if (str.GetError() == JsNoError)
+        {
+            wprintf(_u("%ls\n"), str.GetWideString());
+        }
+    }
+
+    fflush(stdout);
+}
+

bin/ch/WScriptJsrt.h

@@ -58,6 +58,7 @@ class WScriptJsrt
     static JsErrorCode NotifyModuleReadyCallback(_In_opt_ JsModuleRecord referencingModule, _In_opt_ JsValueRef exceptionVar);
     static JsErrorCode InitializeModuleCallbacks();
     static void CALLBACK PromiseContinuationCallback(JsValueRef task, void *callbackState);
+    static void CALLBACK PromiseRejectionTrackerCallback(JsValueRef promise, JsValueRef reason, bool handled, void *callbackState);
 
     static LPCWSTR ConvertErrorCodeToMessage(JsErrorCode errorCode)
     {

bin/ch/ch.cpp

@@ -757,6 +757,11 @@ HRESULT ExecuteTest(const char* fileName)
             IfFailGo(E_FAIL);
         }
 
+        if (HostConfigFlags::flags.TrackRejectedPromises)
+        {
+            ChakraRTInterface::JsSetHostPromiseRejectionTracker(WScriptJsrt::PromiseRejectionTrackerCallback, nullptr);
+        }
+
         len = strlen(fullPath);
         if (HostConfigFlags::flags.GenerateLibraryByteCodeHeaderIsEnabled)
         {

lib/Jsrt/ChakraCore.h

@@ -130,6 +130,27 @@ typedef struct JsNativeFunctionInfo
 /// <returns>The result of the call, if any.</returns>
 typedef _Ret_maybenull_ JsValueRef(CHAKRA_CALLBACK * JsEnhancedNativeFunction)(_In_ JsValueRef callee, _In_ JsValueRef *arguments, _In_ unsigned short argumentCount, _In_ JsNativeFunctionInfo *info, _In_opt_ void *callbackState);
 
+/// <summary>
+///     A Promise Rejection Tracker callback.
+/// </summary>
+/// <remarks>
+///     The host can specify a promise rejection tracker callback in <c>JsSetHostPromiseRejectionTracker</c>.
+///     If a promise is rejected with no reactions or a reaction is added to a promise that was rejected
+///     before it had reactions by default nothing is done.
+///     A Promise Rejection Tracker callback may be set - which will then be called when this occurs.
+///     Note - per draft ECMASpec 2018 25.4.1.9 this function should not set or return an exception
+///     Note also the promise and reason parameters may be garbage collected after this function is called
+///     if you wish to make further use of them you will need to use JsAddRef to preserve them
+///     However if you use JsAddRef you must also call JsRelease and not hold unto them after 
+///     a handled notification (both per spec and to avoid memory leaks)
+/// </remarks>
+/// <param name="promise">The promise object, represented as a JsValueRef.</param>
+/// <param name="reason">The value/cause of the rejection, represented as a JsValueRef.</param>
+/// <param name="handled">Boolean - false for promiseRejected: i.e. if the promise has just been rejected with no handler, 
+///                         true for promiseHandled: i.e. if it was rejected before without a handler and is now being handled.</param>
+/// <param name="callbackState">The state passed to <c>JsSetHostPromiseRejectionTracker</c>.</param>
+typedef void (CHAKRA_CALLBACK *JsHostPromiseRejectionTrackerCallback)(_In_ JsValueRef promise, _In_ JsValueRef reason, _In_ bool handled, _In_opt_ void *callbackState);
+
 /// <summary>
 ///     Creates a new enhanced JavaScript function.
 /// </summary>
@@ -993,5 +1014,27 @@ CHAKRA_API
         _In_ JsValueRef object,
         _In_ JsValueRef key,
         _Out_ bool *hasOwnProperty);
+
+/// <summary>
+///     Sets whether any action should be taken when a promise is rejected with no reactions
+///     or a reaction is added to a promise that was rejected before it had reactions.
+///     By default in either of these cases nothing occurs.
+///     This function allows you to specify if something should occur and provide a callback
+///     to implement whatever should occur.
+/// </summary>
+/// <remarks>
+///     Requires an active script context.
+/// </remarks>
+/// <param name="promiseRejectionTrackerCallback">The callback function being set.</param>
+/// <param name="callbackState">
+///     User provided state that will be passed back to the callback.
+/// </param>
+/// <returns>
+///     The code <c>JsNoError</c> if the operation succeeded, a failure code otherwise.
+/// </returns>
+CHAKRA_API
+    JsSetHostPromiseRejectionTracker(
+        _In_ JsHostPromiseRejectionTrackerCallback promiseRejectionTrackerCallback, 
+        _In_opt_ void *callbackState);
 #endif // _CHAKRACOREBUILD
 #endif // _CHAKRACORE_H_

lib/Jsrt/Jsrt.cpp

@@ -5334,4 +5334,13 @@ CHAKRA_API JsGetDataViewInfo(
     END_JSRT_NO_EXCEPTION
 }
 
+CHAKRA_API JsSetHostPromiseRejectionTracker(_In_ JsHostPromiseRejectionTrackerCallback promiseRejectionTrackerCallback, _In_opt_ void *callbackState)
+{
+  return ContextAPINoScriptWrapper_NoRecord([&](Js::ScriptContext *scriptContext) -> JsErrorCode {
+    scriptContext->GetLibrary()->SetNativeHostPromiseRejectionTrackerCallback((Js::JavascriptLibrary::HostPromiseRejectionTrackerCallback) promiseRejectionTrackerCallback, callbackState);
+    return JsNoError;
+  },
+    /*allowInObjectBeforeCollectCallback*/true);
+}
+
 #endif // _CHAKRACOREBUILD

lib/Runtime/Library/JavascriptLibrary.cpp

@@ -5300,6 +5300,32 @@ namespace Js
         this->nativeHostPromiseContinuationFunctionState = state;
     }
 
+    void JavascriptLibrary::SetNativeHostPromiseRejectionTrackerCallback(HostPromiseRejectionTrackerCallback function, void *state)
+    {
+        this->nativeHostPromiseRejectionTracker = function;
+        this->nativeHostPromiseContinuationFunctionState = state;
+    }
+
+    void JavascriptLibrary::CallNativeHostPromiseRejectionTracker(Var promise, Var reason, bool handled)
+    {
+        if (this->nativeHostPromiseRejectionTracker != nullptr)
+        {
+            BEGIN_LEAVE_SCRIPT(scriptContext);
+            try
+            {
+               this->nativeHostPromiseRejectionTracker(promise, reason, handled, this->nativeHostPromiseContinuationFunctionState);
+            }
+            catch (...)
+            {
+                // Hosts are required not to pass exceptions back across the callback boundary. If
+                // this happens, it is a bug in the host, not something that we are expected to
+                // handle gracefully.
+                Js::Throw::FatalInternalError();
+            }
+            END_LEAVE_SCRIPT(scriptContext);
+        }
+    }
+
     void JavascriptLibrary::SetJsrtContext(FinalizableObject* jsrtContext)
     {
         // With JsrtContext supporting cross context, ensure that it doesn't get GCed

lib/Runtime/Library/JavascriptLibrary.h

@@ -242,6 +242,7 @@ namespace Js
         static DWORD GetRandSeed1Offset() { return offsetof(JavascriptLibrary, randSeed1); }
         static DWORD GetTypeDisplayStringsOffset() { return offsetof(JavascriptLibrary, typeDisplayStrings); }
         typedef bool (CALLBACK *PromiseContinuationCallback)(Var task, void *callbackState);
+        typedef void (CALLBACK *HostPromiseRejectionTrackerCallback)(Var promise, Var reason, bool handled, void *callbackState);
 
         Var GetUndeclBlockVar() const { return undeclBlockVarSentinel; }
         bool IsUndeclBlockVar(Var var) const { return var == undeclBlockVarSentinel; }
@@ -492,6 +493,9 @@ namespace Js
         FieldNoBarrier(PromiseContinuationCallback) nativeHostPromiseContinuationFunction;
         Field(void *) nativeHostPromiseContinuationFunctionState;
 
+        FieldNoBarrier(HostPromiseRejectionTrackerCallback) nativeHostPromiseRejectionTracker = nullptr;
+        Field(void *) nativeHostPromiseRejectionTrackerState;
+
         typedef SList<Js::FunctionProxy*, Recycler> FunctionReferenceList;
         typedef JsUtil::WeakReferenceDictionary<uintptr_t, DynamicType, DictionarySizePolicy<PowerOf2Policy, 1>> JsrtExternalTypesCache;
 
@@ -949,6 +953,8 @@ namespace Js
         JavascriptFunction* GetThrowerFunction() const { return throwerFunction; }
 
         void SetNativeHostPromiseContinuationFunction(PromiseContinuationCallback function, void *state);
+        void SetNativeHostPromiseRejectionTrackerCallback(HostPromiseRejectionTrackerCallback function, void *state);
+        void CallNativeHostPromiseRejectionTracker(Var promise, Var reason, bool handled);
 
         void SetJsrtContext(FinalizableObject* jsrtContext);
         FinalizableObject* GetJsrtContext();

lib/Runtime/Library/JavascriptPromise.cpp

@@ -12,6 +12,7 @@ namespace Js
         Assert(type->GetTypeId() == TypeIds_Promise);
 
         this->status = PromiseStatusCode_Undefined;
+        this->isHandled = false;
         this->result = nullptr;
         this->resolveReactions = nullptr;
         this->rejectReactions = nullptr;
@@ -660,6 +661,10 @@ namespace Js
         {
             reactions = this->GetRejectReactions();
             newStatus = PromiseStatusCode_HasRejection;
+            if (!GetIsHandled())
+            {
+                scriptContext->GetLibrary()->CallNativeHostPromiseRejectionTracker(this, resolution, false);
+            }
         }
         else
         {
@@ -838,13 +843,19 @@ namespace Js
             EnqueuePromiseReactionTask(resolveReaction, sourcePromise->result, scriptContext);
             break;
         case PromiseStatusCode_HasRejection:
+            if (!sourcePromise->GetIsHandled())
+            {
+                scriptContext->GetLibrary()->CallNativeHostPromiseRejectionTracker(sourcePromise, sourcePromise->result, true);
+            }
             EnqueuePromiseReactionTask(rejectReaction, sourcePromise->result, scriptContext);
             break;
         default:
             AssertMsg(false, "Promise status is in an invalid state");
             break;
         }
 
+        sourcePromise->SetIsHandled();
+
         return promiseCapability->GetPromise();
     }
 

lib/Runtime/Library/JavascriptPromise.h

@@ -461,6 +461,8 @@ namespace Js
             PromiseStatusCode_HasRejection
         };
 
+        bool GetIsHandled() { return isHandled; }
+        void SetIsHandled() { isHandled = true; }
         PromiseStatus GetStatus() const { return status; }
         Var GetResult() const { return result; }
 
@@ -469,6 +471,7 @@ namespace Js
 
     protected:
         Field(PromiseStatus) status;
+        Field(bool) isHandled;
         Field(Var) result;
         Field(JavascriptPromiseReactionList*) resolveReactions;
         Field(JavascriptPromiseReactionList*) rejectReactions;

test/es7/PromiseRejectionTracking.baseline

@@ -0,0 +1,43 @@
+Executing test #1 - Reject promise with no reactions.
+Uncaught promise rejection
+Rejection from test 1
+Executing test #2 - Reject promise with a catch reaction only.
+Executing test #3 - Reject promise with catch and then reactions.
+Executing test #4 - Reject promise then add  a catch afterwards.
+Uncaught promise rejection
+Rejection from test 4
+Promise rejection handled
+Rejection from test 4
+Executing test #5 - Reject promise then add two catches afterwards.
+Uncaught promise rejection
+Rejection from test 5
+Promise rejection handled
+Rejection from test 5
+Executing test #6 - Async function that throws.
+Uncaught promise rejection
+Rejection from test 6
+Executing test #7 - Async function that throws but is caught.
+Uncaught promise rejection
+Rejection from test 7
+Promise rejection handled
+Rejection from test 7
+Executing test #8 - Async function that awaits a function that throws.
+Uncaught promise rejection
+Rejection from test 8
+Promise rejection handled
+Rejection from test 8
+Executing test #9 - Reject a handled promise then handle one of the handles but not the other.
+Executing test #10 - Reject a handled promise and don't handle either path.
+Begin async results:
+Uncaught promise rejection
+Rejection from test 8
+Uncaught promise rejection
+Rejection from test 9
+Uncaught promise rejection
+Rejection from test 10
+Uncaught promise rejection
+Rejection from test 10
+Promise rejection handled
+Rejection from test 9
+Uncaught promise rejection
+Rejection from test 9