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

Refactor bootstrapping process and browser.waitForAngular to make them interruptible #4052

Open
@sjelin

Description

@sjelin

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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions