Skip to content

Commit

Permalink
HostPromiseRejection tracker:
Browse files Browse the repository at this point in the history
1. Internal CC machinary for unhandled promise rejection.
2. Simplisitic implimentation in ch
3. Test case
  • Loading branch information
rhuanjl committed Feb 5, 2018
1 parent 97d5c20 commit 812e237
Show file tree
Hide file tree
Showing 16 changed files with 335 additions and 0 deletions.
2 changes: 2 additions & 0 deletions bin/ChakraCore/ChakraCore.def
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,5 @@ JsLessThan
JsLessThanOrEqual

JsCreateEnhancedFunction

JsSetHostPromiseRejectionTracker
1 change: 1 addition & 0 deletions bin/ch/ChakraRtInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
3 changes: 3 additions & 0 deletions bin/ch/ChakraRtInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -152,6 +153,7 @@ struct JsAPIHooks
JsrtGetValueType pfJsrtGetValueType;
JsrtSetIndexedPropertyPtr pfJsrtSetIndexedProperty;
JsrtSetPromiseContinuationCallbackPtr pfJsrtSetPromiseContinuationCallback;
JsrtSetHostPromiseRejectionTrackerPtr pfJsrtSetHostPromiseRejectionTracker;
JsrtGetContextOfObject pfJsrtGetContextOfObject;
JsrtDiagStartDebugging pfJsrtDiagStartDebugging;
JsrtDiagStopDebugging pfJsrtDiagStopDebugging;
Expand Down Expand Up @@ -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)); }
Expand Down
1 change: 1 addition & 0 deletions bin/ch/HostConfigFlagsList.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
29 changes: 29 additions & 0 deletions bin/ch/WScriptJsrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

1 change: 1 addition & 0 deletions bin/ch/WScriptJsrt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
5 changes: 5 additions & 0 deletions bin/ch/ch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
43 changes: 43 additions & 0 deletions lib/Jsrt/ChakraCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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_
9 changes: 9 additions & 0 deletions lib/Jsrt/Jsrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
26 changes: 26 additions & 0 deletions lib/Runtime/Library/JavascriptLibrary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5295,6 +5295,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(nativeHostPromiseRejectionTracker != nullptr)
{
BEGIN_LEAVE_SCRIPT(scriptContext);
try
{
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
Expand Down
6 changes: 6 additions & 0 deletions lib/Runtime/Library/JavascriptLibrary.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down
11 changes: 11 additions & 0 deletions lib/Runtime/Library/JavascriptPromise.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -660,6 +661,10 @@ namespace Js
{
reactions = this->GetRejectReactions();
newStatus = PromiseStatusCode_HasRejection;
if(!GetIsHandled())
{
scriptContext->GetLibrary()->CallNativeHostPromiseRejectionTracker(this, resolution, false);
}
}
else
{
Expand Down Expand Up @@ -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();
}

Expand Down
3 changes: 3 additions & 0 deletions lib/Runtime/Library/JavascriptPromise.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand All @@ -470,6 +472,7 @@ namespace Js
protected:
Field(PromiseStatus) status;
Field(Var) result;
Field(bool) isHandled;
Field(JavascriptPromiseReactionList*) resolveReactions;
Field(JavascriptPromiseReactionList*) rejectReactions;

Expand Down
43 changes: 43 additions & 0 deletions test/es7/PromiseRejectionTracking.baseline
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 812e237

Please sign in to comment.