Skip to content

Commit 543e0e1

Browse files
author
Mike Kaufman
committed
fixing issue where if a throw occurred in a promise without any attached rejection handlers, we wouldn't notify the debugger that an unhandled exception occurred
1 parent 756ee73 commit 543e0e1

19 files changed

+707
-4
lines changed

‎bin/ch/DbgController.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,8 @@ var controllerObj = (function () {
383383
if (bpName == "none") {
384384
exceptionAttributes = 0; // JsDiagBreakOnExceptionAttributeNone
385385
} else if (bpName == "uncaught") {
386+
exceptionAttributes = 0x1; // JsDiagBreakOnExceptionAttributeUncaught
387+
} else if (bpName == "firstchance") {
386388
exceptionAttributes = 0x2; // JsDiagBreakOnExceptionAttributeFirstChance
387389
} else if (bpName == "all") {
388390
exceptionAttributes = 0x1 | 0x2; // JsDiagBreakOnExceptionAttributeUncaught | JsDiagBreakOnExceptionAttributeFirstChance

‎lib/Runtime/Language/JavascriptExceptionOperators.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,22 @@ namespace Js
4040
}
4141
}
4242

43-
JavascriptExceptionOperators::AutoCatchHandlerExists::AutoCatchHandlerExists(ScriptContext* scriptContext)
43+
JavascriptExceptionOperators::AutoCatchHandlerExists::AutoCatchHandlerExists(ScriptContext* scriptContext, bool isPromiseHandled)
4444
{
4545
Assert(scriptContext);
4646
m_threadContext = scriptContext->GetThreadContext();
4747
Assert(m_threadContext);
4848
m_previousCatchHandlerExists = m_threadContext->HasCatchHandler();
4949
m_threadContext->SetHasCatchHandler(TRUE);
50+
51+
if (!isPromiseHandled)
52+
{
53+
// If this is created from a promise-specific code path, and we don't have a rejection
54+
// handler on the promise, then we want SetCatchHandler to be false so we report any
55+
// unhandled exceptions to any detached debuggers.
56+
m_threadContext->SetHasCatchHandler(false);
57+
}
58+
5059
m_previousCatchHandlerToUserCodeStatus = m_threadContext->IsUserCode();
5160
if (scriptContext->IsScriptContextInDebugMode())
5261
{

‎lib/Runtime/Language/JavascriptExceptionOperators.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ namespace Js
3939
void FetchNonUserCodeStatus(ScriptContext *scriptContext);
4040

4141
public:
42-
AutoCatchHandlerExists(ScriptContext* scriptContext);
42+
AutoCatchHandlerExists(ScriptContext* scriptContext, bool isPromiseHandled = true);
4343
~AutoCatchHandlerExists();
4444
};
4545

‎lib/Runtime/Library/JavascriptPromise.cpp

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,22 @@ namespace Js
932932
JavascriptExceptionObject* exception = nullptr;
933933

934934
{
935-
Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
935+
936+
bool isPromiseRejectionHandled = true;
937+
if (scriptContext->IsScriptContextInDebugMode())
938+
{
939+
// only necessary to determine if false if debugger is attached. This way we'll
940+
// correctly break on exceptions raised in promises that result in uhandled rejection
941+
// notifications
942+
Var promiseVar = promiseCapability->GetPromise();
943+
if (JavascriptPromise::Is(promiseVar))
944+
{
945+
JavascriptPromise* promise = JavascriptPromise::FromVar(promiseVar);
946+
isPromiseRejectionHandled = !promise->WillRejectionBeUnhandled();
947+
}
948+
}
949+
950+
Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext, isPromiseRejectionHandled);
936951
try
937952
{
938953
BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext())
@@ -960,6 +975,80 @@ namespace Js
960975
return TryCallResolveOrRejectHandler(promiseCapability->GetResolve(), handlerResult, scriptContext);
961976
}
962977

978+
979+
/**
980+
* Determine if at the current point in time, the given promise has a path of reactions that result
981+
* in an unhandled rejection. This doesn't account for potential of a rejection handler added later
982+
* in time.
983+
*/
984+
bool JavascriptPromise::WillRejectionBeUnhandled()
985+
{
986+
bool willBeUnhandled = !this->GetIsHandled();
987+
if (!willBeUnhandled)
988+
{
989+
// if this promise is handled, then we need to do a depth-first search over this promise's reject
990+
// reactions. If we find a reaction that
991+
// - associated promise is "unhandled" (ie, it's never been "then'd")
992+
// - AND its rejection handler is our default "thrower function"
993+
// then this promise results in an unhandled rejection path.
994+
995+
JsUtil::Stack<JavascriptPromise*, HeapAllocator> stack(&HeapAllocator::Instance);
996+
SimpleHashTable<JavascriptPromise*, int, HeapAllocator> visited(&HeapAllocator::Instance);
997+
stack.Push(this);
998+
visited.Add(this, 1);
999+
1000+
while (!willBeUnhandled && !stack.Empty())
1001+
{
1002+
JavascriptPromise * curr = stack.Pop();
1003+
{
1004+
JavascriptPromiseReactionList* reactions = curr->GetRejectReactions();
1005+
for (int i = 0; i < reactions->Count(); i++)
1006+
{
1007+
JavascriptPromiseReaction* reaction = reactions->Item(i);
1008+
Var promiseVar = reaction->GetCapabilities()->GetPromise();
1009+
1010+
if (JavascriptPromise::Is(promiseVar))
1011+
{
1012+
JavascriptPromise* p = JavascriptPromise::FromVar(promiseVar);
1013+
if (!p->GetIsHandled())
1014+
{
1015+
RecyclableObject* handler = reaction->GetHandler();
1016+
if (JavascriptFunction::Is(handler))
1017+
{
1018+
JavascriptFunction* func = JavascriptFunction::FromVar(handler);
1019+
FunctionInfo* functionInfo = func->GetFunctionInfo();
1020+
1021+
#ifdef DEBUG
1022+
if (!func->IsCrossSiteObject())
1023+
{
1024+
// assert that Thrower function's FunctionInfo hasn't changed
1025+
AssertMsg(func->GetScriptContext()->GetLibrary()->GetThrowerFunction()->GetFunctionInfo() == &JavascriptPromise::EntryInfo::Thrower, "unexpected FunctionInfo for thrower function!");
1026+
}
1027+
#endif
1028+
1029+
// If the function info is the default thrower function's function info, then assume that this is unhandled
1030+
// this will work across script contexts
1031+
if (functionInfo == &JavascriptPromise::EntryInfo::Thrower)
1032+
{
1033+
willBeUnhandled = true;
1034+
break;
1035+
}
1036+
}
1037+
}
1038+
AssertMsg(visited.HasEntry(p) == false, "Unexpected cycle in promise reaction tree!");
1039+
if (!visited.HasEntry(p))
1040+
{
1041+
stack.Push(p);
1042+
visited.Add(p, 1);
1043+
}
1044+
}
1045+
}
1046+
}
1047+
}
1048+
}
1049+
return willBeUnhandled;
1050+
}
1051+
9631052
Var JavascriptPromise::TryCallResolveOrRejectHandler(Var handler, Var value, ScriptContext* scriptContext)
9641053
{
9651054
Var undefinedVar = scriptContext->GetLibrary()->GetUndefined();
@@ -1092,7 +1181,15 @@ namespace Js
10921181
JavascriptExceptionObject* exception = nullptr;
10931182

10941183
{
1095-
Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
1184+
bool isPromiseRejectionHandled = true;
1185+
if (scriptContext->IsScriptContextInDebugMode())
1186+
{
1187+
// only necessary to determine if false if debugger is attached. This way we'll
1188+
// correctly break on exceptions raised in promises that result in uhandled rejections
1189+
isPromiseRejectionHandled = !promise->WillRejectionBeUnhandled();
1190+
}
1191+
1192+
Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext, isPromiseRejectionHandled);
10961193
try
10971194
{
10981195
BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext())

‎lib/Runtime/Library/JavascriptPromise.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,7 @@ namespace Js
563563

564564
private :
565565
static void AsyncSpawnStep(JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* nextFunction, JavascriptGenerator* gen, Var resolve, Var reject);
566+
bool WillRejectionBeUnhandled();
566567

567568
#if ENABLE_TTD
568569
public:

‎test/Debugger/JsDiagBreakOnUncaughtException.baseline

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Error: throw exception from throwFunction
2+
at throwFunction() (JsDiagBreakOnUncaughtException.js:18:4)
3+
at Global code (JsDiagBreakOnUncaughtException.js:20:1)

‎test/Debugger/JsDiagBreakOnUncaughtException.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
/**exception(uncaught):stack();**/
7+
8+
function noThrowFunction() {
9+
try {
10+
throw new Error("throw exception from noThrowFunction");
11+
} catch (err) {
12+
}
13+
}
14+
noThrowFunction();
15+
16+
// calling throwFunction() will terminate program, so this has to come last
17+
function throwFunction() {
18+
throw new Error("throw exception from throwFunction");
19+
}
20+
throwFunction();

‎test/Debugger/JsDiagBreakOnUncaughtException.js.dbg.baseline

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
[
2+
{
3+
"callStack": [
4+
{
5+
"line": 17,
6+
"column": 3,
7+
"sourceText": "throw new Error(\"throw exception from throwFunction\")",
8+
"function": "throwFunction"
9+
},
10+
{
11+
"line": 19,
12+
"column": 0,
13+
"sourceText": "throwFunction()",
14+
"function": "Global code"
15+
}
16+
]
17+
}
18+
]

‎test/Debugger/JsDiagExceptionsInAsyncFunctions_BreakOnUncaughtExceptions.baseline

Whitespace-only changes.

‎test/Debugger/JsDiagExceptionsInAsyncFunctions_BreakOnUncaughtExceptions.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
/**exception(uncaught):stack();**/
7+
8+
async function f1() {
9+
await null;
10+
throw new Error('error in f1');
11+
}
12+
f1();
13+
14+
async function f2() {
15+
16+
async function f2a() {
17+
throw "err";
18+
}
19+
20+
async function f2b() {
21+
try {
22+
var p = f2a();
23+
} catch (e) {
24+
console.log("caught " + e);
25+
}
26+
}
27+
28+
async function f2c() {
29+
var p = f2a();
30+
}
31+
32+
f2b();
33+
f2c();
34+
}
35+
f2();

0 commit comments

Comments
 (0)