Skip to content

Commit ceaf3da

Browse files
committed
[js] Fire the "uncaughtException" event in a new turn of the JS event loop.
As a result of this change, any errors thrown by an event listener will propagate to the global error handler. Previously, this event was fired with in the context of a (native) promise callback, causing errors to be silently suppressed in the promise chain. This was tested manually. I'm not sure how to test interaction with node's global error handler using mocha (without crashing the process) Fixes #2770
1 parent 254c77b commit ceaf3da

File tree

3 files changed

+21
-21
lines changed

3 files changed

+21
-21
lines changed

javascript/node/selenium-webdriver/CHANGES.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
via tha API, `safari.Options#useLegacyDriver`, to use the safari
99
extension driver.
1010
* Updated the `lib/proxy` module to support configuring a SOCKS proxy.
11+
* For the `promise.ControlFlow`, fire the "uncaughtException" event in a new
12+
turn of the JS event loop. As a result of this change, any errors thrown by
13+
an event listener will propagate to the global error handler. Previously,
14+
this event was fired with in the context of a (native) promise callback,
15+
causing errors to be silently suppressed in the promise chain.
1116

1217
### API Changes
1318

javascript/node/selenium-webdriver/lib/promise.js

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -644,18 +644,6 @@ function asyncRun(fn) {
644644
}
645645

646646

647-
/**
648-
* Throws an error asynchronously so it is reported to the global error handler.
649-
*
650-
* @param {!Error} error The error to throw.
651-
*/
652-
function asyncThrow(error) {
653-
setTimeout(function() {
654-
throw error;
655-
}, 0);
656-
}
657-
658-
659647
/**
660648
* @param {number} level What level of verbosity to log with.
661649
* @param {(string|function(this: T): string)} loggable The message to log.
@@ -2297,13 +2285,14 @@ class ControlFlow extends events.EventEmitter {
22972285
this.cancelShutdown_();
22982286
this.cancelHold_();
22992287

2300-
var listeners = this.listeners(
2301-
ControlFlow.EventType.UNCAUGHT_EXCEPTION);
2302-
if (!listeners.size) {
2303-
asyncThrow(/** @type {!Error} */(error));
2304-
} else {
2305-
this.reportUncaughtException_(error);
2306-
}
2288+
setTimeout(() => {
2289+
let listeners = this.listeners(ControlFlow.EventType.UNCAUGHT_EXCEPTION);
2290+
if (!listeners.size) {
2291+
throw error;
2292+
} else {
2293+
this.reportUncaughtException_(error);
2294+
}
2295+
}, 0);
23072296
}
23082297

23092298
/**

javascript/node/selenium-webdriver/test/lib/promise_test.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,10 @@ describe('promise', function() {
9696
// so tearDown() doesn't throw
9797
app.removeAllListeners();
9898
app.on(promise.ControlFlow.EventType.UNCAUGHT_EXCEPTION, handler);
99-
return NativePromise.resolve().then(() => handler.assertCalled());
99+
return NativePromise.resolve()
100+
// Macro yield so the uncaught exception has a chance to trigger.
101+
.then(() => new NativePromise(resolve => setTimeout(resolve, 0)))
102+
.then(() => handler.assertCalled());
100103
});
101104
});
102105

@@ -312,7 +315,10 @@ describe('promise', function() {
312315
app.removeAllListeners();
313316
app.on(promise.ControlFlow.EventType.UNCAUGHT_EXCEPTION, handler);
314317

315-
return NativePromise.resolve().then(handler.assertCalled);
318+
return NativePromise.resolve()
319+
// Macro yield so the uncaught exception has a chance to trigger.
320+
.then(() => new NativePromise(resolve => setTimeout(resolve, 0)))
321+
.then(handler.assertCalled);
316322
});
317323

318324
it('cannotResolveADeferredWithItself', function() {

0 commit comments

Comments
 (0)