Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit bbeb7fc

Browse files
committed
Bug 1092922 - Only pause once when unwinding a particular exception, r=loganfsmyth.
1 parent f12656e commit bbeb7fc

5 files changed

Lines changed: 142 additions & 15 deletions

File tree

devtools/client/debugger/test/mochitest/browser_dbg-pause-exceptions.js

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ function caughtException() {
1717
4. skip a caught error
1818
*/
1919
add_task(async function() {
20-
const dbg = await initDebugger("doc-exceptions.html");
20+
const dbg = await initDebugger("doc-exceptions.html", "exceptions.js");
21+
const source = findSource(dbg, "exceptions.js");
2122

2223
log("1. test skipping an uncaught exception");
2324
await uncaughtException();
@@ -31,6 +32,14 @@ add_task(async function() {
3132
await resume(dbg);
3233
await waitForActive(dbg);
3334

35+
log("2.b Test throwing the same uncaught exception pauses again");
36+
await togglePauseOnExceptions(dbg, true, true);
37+
uncaughtException();
38+
await waitForPaused(dbg);
39+
assertPausedLocation(dbg);
40+
await resume(dbg);
41+
await waitForActive(dbg);
42+
3443
log("3. Test pausing on a caught Error");
3544
caughtException();
3645
await waitForPaused(dbg);
@@ -50,4 +59,51 @@ add_task(async function() {
5059
await waitForPaused(dbg);
5160
assertPausedLocation(dbg);
5261
await resume(dbg);
62+
63+
await togglePauseOnExceptions(dbg, true, true);
64+
65+
log("5. Only pause once when throwing deep in the stack");
66+
invokeInTab("deepError");
67+
await waitForPaused(dbg);
68+
assertPausedAtSourceAndLine(dbg, source.id, 16);
69+
await resume(dbg);
70+
await waitForPaused(dbg);
71+
assertPausedAtSourceAndLine(dbg, source.id, 22);
72+
await resume(dbg);
73+
74+
log("6. Only pause once on an exception when pausing in a finally block");
75+
invokeInTab("deepErrorFinally");
76+
await waitForPaused(dbg);
77+
assertPausedAtSourceAndLine(dbg, source.id, 34);
78+
await resume(dbg);
79+
await waitForPaused(dbg);
80+
assertPausedAtSourceAndLine(dbg, source.id, 31);
81+
await resume(dbg);
82+
await waitForPaused(dbg);
83+
assertPausedAtSourceAndLine(dbg, source.id, 40);
84+
await resume(dbg);
85+
86+
log("7. Only pause once on an exception when it is rethrown from a catch");
87+
invokeInTab("deepErrorCatch");
88+
await waitForPaused(dbg);
89+
assertPausedAtSourceAndLine(dbg, source.id, 53);
90+
await resume(dbg);
91+
await waitForPaused(dbg);
92+
assertPausedAtSourceAndLine(dbg, source.id, 49);
93+
await resume(dbg);
94+
await waitForPaused(dbg);
95+
assertPausedAtSourceAndLine(dbg, source.id, 59);
96+
await resume(dbg);
97+
98+
log("8. Pause on each exception thrown while unwinding");
99+
invokeInTab("deepErrorThrowDifferent");
100+
await waitForPaused(dbg);
101+
assertPausedAtSourceAndLine(dbg, source.id, 71);
102+
await resume(dbg);
103+
await waitForPaused(dbg);
104+
assertPausedAtSourceAndLine(dbg, source.id, 68);
105+
await resume(dbg);
106+
await waitForPaused(dbg);
107+
assertPausedAtSourceAndLine(dbg, source.id, 77);
108+
await resume(dbg);
53109
});

devtools/client/debugger/test/mochitest/browser_dbg-returnvalues.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ async function testThrowValue(dbg, val) {
4848
// displays values, so this test works when `val` is a string.
4949
is(getValue(dbg, 2), uneval(val), `check exception is ${uneval(val)}`);
5050

51-
await resume(dbg);
52-
await waitForPaused(dbg);
5351
await resume(dbg);
5452
assertNotPaused(dbg);
5553
}

devtools/client/debugger/test/mochitest/examples/exceptions.js

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,77 @@ function uncaughtException() {
22
throw "unreachable"
33
}
44

5-
function caughtError() {
5+
function caughtException() {
66
try {
7-
throw new Error("error");
7+
throw "reachable";
88
} catch (e) {
99
debugger;
1010
}
1111
}
1212

13-
function caughtException() {
13+
function deepError() {
14+
function a() { b(); }
15+
function b() { c(); }
16+
function c() { throw new Error(); }
17+
1418
try {
15-
throw "reachable";
16-
} catch (e) {
17-
debugger;
19+
a();
20+
} catch (e) {}
21+
22+
debugger;
23+
}
24+
25+
function deepErrorFinally() {
26+
function a() { b(); }
27+
function b() {
28+
try {
29+
c();
30+
} finally {
31+
debugger;
32+
}
1833
}
34+
function c() { throw new Error(); }
35+
36+
try {
37+
a();
38+
} catch (e) {}
39+
40+
debugger;
41+
}
42+
43+
function deepErrorCatch() {
44+
function a() { b(); }
45+
function b() {
46+
try {
47+
c();
48+
} catch (e) {
49+
debugger;
50+
throw e;
51+
}
52+
}
53+
function c() { throw new Error(); }
54+
55+
try {
56+
a();
57+
} catch (e) {}
58+
59+
debugger;
60+
}
61+
62+
function deepErrorThrowDifferent() {
63+
function a() { b(); }
64+
function b() {
65+
try {
66+
c();
67+
} catch (e) {
68+
throw new Error();
69+
}
70+
}
71+
function c() { throw new Error(); }
72+
73+
try {
74+
a();
75+
} catch (e) {}
76+
77+
debugger;
1978
}

devtools/server/actors/thread.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
7676
// the debugger instance.
7777
this._onLoadBreakpointURLs = new Set();
7878

79+
// A WeakMap from Debugger.Frame to an exception value which will be ignored
80+
// when deciding to pause if the value is thrown by the frame. When we are
81+
// pausing on exceptions then we only want to pause when the youngest frame
82+
// throws a particular exception, instead of for all older frames as well.
83+
this._handledFrameExceptions = new WeakMap();
84+
7985
this.global = global;
8086

8187
this.onNewSourceEvent = this.onNewSourceEvent.bind(this);
@@ -1565,6 +1571,11 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
15651571
return undefined;
15661572
}
15671573

1574+
if (this._handledFrameExceptions.has(youngestFrame) &&
1575+
this._handledFrameExceptions.get(youngestFrame) === value) {
1576+
return undefined;
1577+
}
1578+
15681579
// NS_ERROR_NO_INTERFACE exceptions are a special case in browser code,
15691580
// since they're almost always thrown by QueryInterface functions, and
15701581
// handled cleanly by native code.
@@ -1585,6 +1596,12 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
15851596
return undefined;
15861597
}
15871598

1599+
// Now that we've decided to pause, ignore this exception if it's thrown by
1600+
// any older frames.
1601+
for (let frame = youngestFrame.older; frame != null; frame = frame.older) {
1602+
this._handledFrameExceptions.set(frame, value);
1603+
}
1604+
15881605
try {
15891606
const packet = this._paused(youngestFrame);
15901607
if (!packet) {

devtools/server/tests/unit/test_ignore_no_interface_exceptions.js

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,9 @@
1010

1111
add_task(threadClientTest(async ({ threadClient, debuggee, client }) => {
1212
await threadClient.pauseOnExceptions(true, false);
13-
await executeOnNextTickAndWaitForPause(() => evaluateTestCode(debuggee), client);
14-
15-
await resume(threadClient);
16-
const paused = await waitForPause(client);
17-
Assert.equal(paused.why.type, "exception");
18-
equal(paused.frame.where.line, 12, "paused at throw");
13+
const paused =
14+
await executeOnNextTickAndWaitForPause(() => evaluateTestCode(debuggee), client);
15+
equal(paused.frame.where.line, 6, "paused at throw");
1916

2017
await resume(threadClient);
2118
}, {

0 commit comments

Comments
 (0)