Skip to content

Commit 654bf24

Browse files
committed
Fix an error propagation bug when a command fails from bad inputs.
This fixes a bug where commands that had rejected promises as inputs could not catch errors from those inputs. One example is trying to catch a NoSuchElementError through a chained call to click: driver.findElement(By.id('not-there')).click().thenCatch(function(e) { if (e.code === NO_SUCH_ELEMENT) { // Handle element not found } });
1 parent e0dbda0 commit 654bf24

File tree

3 files changed

+230
-17
lines changed

3 files changed

+230
-17
lines changed

javascript/webdriver/promise.js

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -423,24 +423,15 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
423423
}
424424

425425
if (!handled && state == webdriver.promise.Deferred.State_.REJECTED) {
426-
pendingRejectionKey = propagateError(value);
426+
flow.pendingRejections_ += 1;
427+
pendingRejectionKey = flow.timer.setTimeout(function() {
428+
pendingRejectionKey = null;
429+
flow.pendingRejections_ -= 1;
430+
flow.abortFrame_(value);
431+
}, 0);
427432
}
428433
}
429434

430-
/**
431-
* Propagates an unhandled rejection to the parent ControlFlow in a
432-
* future turn of the JavaScript event loop.
433-
* @param {*} error The error value to report.
434-
* @return {number} The key for the registered timeout.
435-
*/
436-
function propagateError(error) {
437-
flow.pendingRejections_ += 1;
438-
return flow.timer.setTimeout(function() {
439-
flow.pendingRejections_ -= 1;
440-
flow.abortFrame_(error);
441-
}, 0);
442-
}
443-
444435
/**
445436
* Notifies a single listener of this Deferred's change in state.
446437
* @param {!webdriver.promise.Deferred.Listener_} listener The listener to
@@ -485,9 +476,10 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
485476
// The moment a listener is registered, we consider this deferred to be
486477
// handled; the callback must handle any rejection errors.
487478
handled = true;
488-
if (pendingRejectionKey) {
479+
if (pendingRejectionKey !== null) {
489480
flow.pendingRejections_ -= 1;
490481
flow.timer.clearTimeout(pendingRejectionKey);
482+
pendingRejectionKey = null;
491483
}
492484

493485
var deferred = new webdriver.promise.Deferred(cancel, flow);

javascript/webdriver/test/webdriver_test.js

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,6 +1575,36 @@ function testExecuteScript_scriptReturnsAnError() {
15751575
}
15761576

15771577

1578+
function testExecuteScript_failsIfArgumentIsARejectedPromise() {
1579+
var testHelper = TestHelper.expectingSuccess().replayAll();
1580+
1581+
var callback = callbackHelper(assertIsStubError);
1582+
1583+
var arg = webdriver.promise.rejected(STUB_ERROR);
1584+
arg.thenCatch(goog.nullFunction); // Suppress default handler.
1585+
1586+
var driver = testHelper.createDriver();
1587+
driver.executeScript(goog.nullFunction, arg).thenCatch(callback);
1588+
testHelper.execute();
1589+
callback.assertCalled();
1590+
}
1591+
1592+
1593+
function testExecuteAsyncScript_failsIfArgumentIsARejectedPromise() {
1594+
var testHelper = TestHelper.expectingSuccess().replayAll();
1595+
1596+
var callback = callbackHelper(assertIsStubError);
1597+
1598+
var arg = webdriver.promise.rejected(STUB_ERROR);
1599+
arg.thenCatch(goog.nullFunction); // Suppress default handler.
1600+
1601+
var driver = testHelper.createDriver();
1602+
driver.executeAsyncScript(goog.nullFunction, arg).thenCatch(callback);
1603+
testHelper.execute();
1604+
callback.assertCalled();
1605+
}
1606+
1607+
15781608
function testFindElement_elementNotFound() {
15791609
var testHelper = TestHelper.
15801610
expectingFailure(expectedError(ECode.NO_SUCH_ELEMENT, 'Unable to find element')).
@@ -2058,6 +2088,23 @@ function testElementEquals_sendsRpcIfElementsHaveDifferentIds() {
20582088
callback.assertCalled();
20592089
}
20602090

2091+
2092+
function testElementEquals_failsIfAnInputElementCouldNotBeFound() {
2093+
var testHelper = TestHelper.expectingSuccess().replayAll();
2094+
2095+
var callback = callbackHelper(assertIsStubError);
2096+
var id = webdriver.promise.rejected(STUB_ERROR);
2097+
id.thenCatch(goog.nullFunction); // Suppress default handler.
2098+
2099+
var driver = testHelper.createDriver();
2100+
var a = new webdriver.WebElement(driver, {'ELEMENT': 'foo'});
2101+
var b = new webdriver.WebElementPromise(driver, id);
2102+
2103+
webdriver.WebElement.equals(a, b).thenCatch(callback);
2104+
testHelper.execute();
2105+
callback.assertCalled();
2106+
}
2107+
20612108
function testWaiting_waitSucceeds() {
20622109
var testHelper = TestHelper.expectingSuccess().
20632110
expect(CName.FIND_ELEMENTS, {'using':'id', 'value':'foo'}).
@@ -2245,3 +2292,122 @@ function testFetchingLogs() {
22452292
testHelper.execute();
22462293
pair.assertCallback();
22472294
}
2295+
2296+
2297+
function testCommandsFailIfInitialSessionCreationFailed() {
2298+
var testHelper = TestHelper.expectingSuccess().replayAll();
2299+
var navigateResult = callbackPair(null, assertIsStubError);
2300+
var quitResult = callbackPair(null, assertIsStubError);
2301+
2302+
var session = webdriver.promise.rejected(STUB_ERROR);
2303+
2304+
var driver = testHelper.createDriver(session);
2305+
driver.get('some-url').then(navigateResult.callback, navigateResult.errback);
2306+
driver.quit().then(quitResult.callback, quitResult.errback);
2307+
2308+
testHelper.execute();
2309+
navigateResult.assertErrback();
2310+
quitResult.assertErrback();
2311+
}
2312+
2313+
2314+
function testWebElementCommandsFailIfInitialDriverCreationFailed() {
2315+
var testHelper = TestHelper.expectingSuccess().replayAll();
2316+
2317+
var session = webdriver.promise.rejected(STUB_ERROR);
2318+
var callback = callbackHelper(assertIsStubError);
2319+
2320+
var driver = testHelper.createDriver(session);
2321+
driver.findElement(By.id('foo')).click().thenCatch(callback);
2322+
testHelper.execute();
2323+
callback.assertCalled();
2324+
}
2325+
2326+
2327+
function testWebElementCommansFailIfElementCouldNotBeFound() {
2328+
var testHelper = TestHelper.
2329+
expectingSuccess().
2330+
expect(CName.FIND_ELEMENT, {'using':'id', 'value':'foo'}).
2331+
andReturnError(ECode.NO_SUCH_ELEMENT,
2332+
{'message':'Unable to find element'}).
2333+
replayAll();
2334+
2335+
var callback = callbackHelper(
2336+
expectedError(ECode.NO_SUCH_ELEMENT, 'Unable to find element'));
2337+
2338+
var driver = testHelper.createDriver();
2339+
driver.findElement(By.id('foo')).click().thenCatch(callback);
2340+
testHelper.execute();
2341+
callback.assertCalled();
2342+
}
2343+
2344+
2345+
function testCannotFindChildElementsIfParentCouldNotBeFound() {
2346+
var testHelper = TestHelper.
2347+
expectingSuccess().
2348+
expect(CName.FIND_ELEMENT, {'using':'id', 'value':'foo'}).
2349+
andReturnError(ECode.NO_SUCH_ELEMENT,
2350+
{'message':'Unable to find element'}).
2351+
replayAll();
2352+
2353+
var callback = callbackHelper(
2354+
expectedError(ECode.NO_SUCH_ELEMENT, 'Unable to find element'));
2355+
2356+
var driver = testHelper.createDriver();
2357+
driver.findElement(By.id('foo'))
2358+
.findElement(By.id('bar'))
2359+
.findElement(By.id('baz'))
2360+
.thenCatch(callback);
2361+
testHelper.execute();
2362+
callback.assertCalled();
2363+
}
2364+
2365+
2366+
function testActionSequenceFailsIfInitialDriverCreationFailed() {
2367+
var testHelper = TestHelper.expectingSuccess().replayAll();
2368+
2369+
var session = webdriver.promise.rejected(STUB_ERROR);
2370+
2371+
// Suppress the default error handler so we can verify it propagates
2372+
// to the perform() call below.
2373+
session.thenCatch(goog.nullFunction);
2374+
2375+
var callback = callbackHelper(assertIsStubError);
2376+
2377+
var driver = testHelper.createDriver(session);
2378+
driver.actions().
2379+
mouseDown().
2380+
mouseUp().
2381+
perform().
2382+
thenCatch(callback);
2383+
testHelper.execute();
2384+
callback.assertCalled();
2385+
}
2386+
2387+
2388+
function testAlertCommandsFailIfAlertNotPresent() {
2389+
var testHelper = TestHelper
2390+
.expectingSuccess()
2391+
.expect(CName.GET_ALERT_TEXT)
2392+
.andReturnError(ECode.NO_SUCH_ALERT, {'message': 'no alert'})
2393+
.replayAll();
2394+
2395+
var driver = testHelper.createDriver();
2396+
var alert = driver.switchTo().alert();
2397+
2398+
var expectError = expectedError(ECode.NO_SUCH_ALERT, 'no alert');
2399+
var callbacks = [];
2400+
for (var key in webdriver.Alert.prototype) {
2401+
if (webdriver.Alert.prototype.hasOwnProperty(key)) {
2402+
var helper = callbackHelper(expectError);
2403+
callbacks.push(key, helper);
2404+
alert[key].call(alert).thenCatch(helper);
2405+
}
2406+
}
2407+
2408+
testHelper.execute();
2409+
for (var i = 0; i < callbacks.length - 1; i += 2) {
2410+
callbacks[i + 1].assertCalled(
2411+
'Error did not propagate for ' + callbacks[i]);
2412+
}
2413+
}

javascript/webdriver/webdriver.js

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,14 +281,39 @@ webdriver.WebDriver.prototype.schedule = function(command, description) {
281281
checkHasNotQuit();
282282
command.setParameter('sessionId', this.session_);
283283

284+
// If any of the command parameters are rejected promises, those
285+
// rejections may be reported as unhandled before the control flow
286+
// attempts to execute the command. To ensure parameters errors
287+
// propagate through the command itself, we resolve all of the
288+
// command parameters now, but suppress any errors until the ControlFlow
289+
// actually executes the command. This addresses scenarios like catching
290+
// an element not found error in:
291+
//
292+
// driver.findElement(By.id('foo')).click().thenCatch(function(e) {
293+
// if (e.code === bot.ErrorCode.NO_SUCH_ELEMENT) {
294+
// // Do something.
295+
// }
296+
// });
297+
var prepCommand = webdriver.WebDriver.toWireValue_(command.getParameters());
298+
prepCommand.thenCatch(goog.nullFunction);
299+
284300
var flow = this.flow_;
301+
var executor = this.executor_;
285302
return flow.execute(function() {
286303
// A call to WebDriver.quit() may have been scheduled in the same event
287304
// loop as this |command|, which would prevent us from detecting that the
288305
// driver has quit above. Therefore, we need to make another quick check.
289306
// We still check above so we can fail as early as possible.
290307
checkHasNotQuit();
291-
return webdriver.WebDriver.executeCommand_(self.executor_, command);
308+
309+
// Retrieve resolved command parameters; any previously suppressed errors
310+
// will now propagate up through the control flow as part of the command
311+
// execution.
312+
return prepCommand.then(function(parameters) {
313+
command.setParameters(parameters);
314+
return webdriver.promise.checkedNodeCall(
315+
goog.bind(executor.execute, executor, command));
316+
});
292317
}, description).then(function(response) {
293318
try {
294319
bot.response.checkResponse(response);
@@ -2186,6 +2211,36 @@ webdriver.AlertPromise = function(driver, alert) {
21862211
return alert.getText();
21872212
});
21882213
};
2214+
2215+
/**
2216+
* Defers action until the alert has been located.
2217+
* @override
2218+
*/
2219+
this.accept = function() {
2220+
return alert.then(function(alert) {
2221+
return alert.accept();
2222+
});
2223+
};
2224+
2225+
/**
2226+
* Defers action until the alert has been located.
2227+
* @override
2228+
*/
2229+
this.dismiss = function() {
2230+
return alert.then(function(alert) {
2231+
return alert.dismiss();
2232+
});
2233+
};
2234+
2235+
/**
2236+
* Defers action until the alert has been located.
2237+
* @override
2238+
*/
2239+
this.sendKeys = function(text) {
2240+
return alert.then(function(alert) {
2241+
return alert.sendKeys(text);
2242+
});
2243+
};
21892244
};
21902245
goog.inherits(webdriver.AlertPromise, webdriver.Alert);
21912246

0 commit comments

Comments
 (0)