Refactor bootstrapping process and browser.waitForAngular
to make them interruptible #4052
Description
In the bootstrapping process (everything after navigation in browser.get
) and browser.waitForAngular
we send a few separate commands to selenium-webdriver
. Browser initiated navigation (e.g. clicking a link) may cause the first few commands to be executed on one page, and the remaining commands to be executed on a different page. This could end up with us trying to bootstrap or synchronize with entirely the wrong page.
What's more, in #3857 we want to start running bootstrap checks periodically. With the control flow disabled, this could easily lead of race conditions.
Luckily, this is easy enough to solve. We can stick some metadata on the window
object at the start of bootstrapping/waitForAngular
, and if that metadata goes away, we know navigation occurred. If navigation occurs, we silently give up on the remainder of the bootstrapping/waitForAngular
.
Proposed Implementation
We should write a helper function safeExecuteScriptFactory
:
interface SafeExecuteScript extends Function {
(script: string|Function, description: string, ...scriptArgs: any[]): wdpromise.Promise<any>;
context: string;
id: string;
deleteMetadata: () => wdpromise.Promise<void>;
}
class InterruptionError extends Error {
constructor(public context: string, public id: string) {
super(`Navigation occurred during ${context} (call id: ${id})`);
}
}
private safeExecuteScriptFactory(context: string): wdpromise.Promise<SafeExecuteScript> {
const id = uuid.v4();
const safeExec = function(script: string|Function, description: string, ...scriptArgs: any[]) {
// Wrap script
if (typeof script === 'function') {
script = `return (${script}).apply(null, arguments);`;
}
script = clientSideScripts.wrapScriptForSafeCall(script, id);
// Run wrapped script & unwrap return value
scriptArgs.unshift(script, description, id);
return this.executeScriptWithDescription.apply(this, scriptArgs).then((result: any) => {
if (result.value) {
return result.value;
} else {
throw new InterruptionError(context, id);
}
});
} as SafeExecuteScript;
safeExec.bind(this);
safeExec.context = context;
safeExec.id = id;
safeExec.deleteMetadata = () => {
return this.executeScriptWithDescription(clientSideScripts.deleteSafeCallMetadata,
`Deleting metadata for ${context} (call id: ${id})`, id);
};
return this.executeScriptWithDescription(clientSideScripts.initSafeCallMetadata,
`Initializing metadata for ${context} (call id: ${id})`, id).then(() => safeExec);
}
With the following additions to clientsidescripts.js
:
functions.initSafeCallMetadata = function(safeCallUUID) {
window['__PROTRACTOR_METADATA_' + safeCallUUID] = {};
}
functions.deleteSafeCallMetadata = function(safeCallUUID) {
delete window['__PROTRACTOR_METADATA_' + safeCallUUID];
}
exports.wrapScriptForSafeCall = function(script, safeCallUUID) {
var unwrappedFn = 'function unwrappedFn() {' + script + '}';
var getCallMetadata = 'function getCallMetadata() {' +
'return window["__PROTRACTOR_METADATA_' + safeCallUUID + '"];}'
return wrapWithHelpers(function() {
if (getCallMetadata()) {
return { value: unwrappedFn.apply(this, arguments) };
} else {
return { error: 'No metadata for ' + id };
}
}, unwrappedFn, getCallMetadata);
}
Then, in at the start of bootstrapping in browser.get
(code link in v5.1) we do:
.then(() => {
return this.safeExecuteScriptFactory(msg('bootstrapping'));
})
.then((safeExec) => {
safeExecuteScriptWithDescription = safeExec; // declared at the top of the file
})
... // Proceed as before but use `safeExecuteScriptWithDescription`
... // instead of `this.executeScriptWithDescription`
...
// at the end of the promise chain, delete the metadata
.then(() => {
return safeExecuteScriptWithDescription.deleteMetadata();
}, (error) => {
return safeExecuteScriptWithDescription.deleteMetadata().then(() => {
if (!(error instanceOf InterruptionError)) {
throw error;
}
}, (newError) => {
throw (error instanceOf InterruptionError) ? newError : error;
});
});
And then we do essentially the same thing in waitForAngular
as well. This way, if some kind of navigation happens, we just stop.
Edge Case: browser.executeAsyncScript_
We can't use safeExecuteScriptWithDescription
in place of browser.executeAsyncScript_
because safeExecuteScriptWithDescription
doesn't take asynchronous scripts. However, as per #4053, we probably want to get rid of browser.executeAsyncScript_
anyway.
Edge Case: Blocking Proxy
Blocking Proxy has its own GitHub issue (angular/blocking-proxy#32) about sending clear error messages when navigation interrupts stabilization. We will simply catch this error to detect interruptions.
We could have a race condition in waitForAngular
where navigation could occur in between plugin stability checks and blocking proxy checks. However, waitForAngular
is only called at the start of commands like el.click()
, and so well written tests shouldn't run into this problem.
Edge Case: Plugins
The plugin functions onPageLoad
, onPageStable
, onPageLoad
and onPageStable
also need to be interruptible. This is mostly going to be up to the plugin authors, but we can help them out by providing them with safeExecuteScriptWithDescription
.