Retry opening URL if worker-browser fails to send request within ackTimeout

[shirish87]

• Attempts to set the URL one additional time just in case a newly created (or reused) worker fails to set it
• Similar in approach to the other PR on this issue
• Incorporates changes requested and minor code cleanup

Run with --verbose to see ack check and re-request behaviour.

[jzaefferer]

Boolean flags like this are difficult to understand without looking at the method signature. How about a separate resetAck() method that clears the timeout? Then you'd call both resetAck() and awaitAck() here.

[shirish87]

Agreed. I was contemplating adding a worker.reset() to reset state and timers for reused workers, but then thought of keeping the changes as local to this issue as possible. : )

[jzaefferer]

Looks much better, still a few things to address.

[shirish87]

Modified as per your suggestions. Let me know if you'd still like to use .bind instead of self refs : )

[jzaefferer]

Looks good.

[jzaefferer]

Should rebase/squash the whole thing into a single commit before landing it in master.

[shirish87]

Done.

[jzaefferer]

👍

[nording]

After some digging I have found out that this pull request does not fix the blank page issue.
The reason for that is because of the following things:

• 1. The worker acknowledge-state is not good enough to listen for, because it is not handle correct, sometimes you get "Received ack" before even the browser is launched. And when that happens and the page fails to load correct, the page won't reload again, because it check if the worker.isAckd is false before the reload. See ackTimeout function below:

 this.ackTimeout = setTimeout(function () {
      if (self.isAckd) { // Then if the page fails to load, this expression is getting true and then it return. And nothing else will happen
        // Already ack'd
        return;
      }

      // worker has not acknowledged itself in 60 sec, reopen url
      client.changeUrl(self.id, { url: self.buildUrl() }, function () {
        logger.debug("[%s] Sent Request to reload url", self.getTestBrowserInfo());
      });

    }, ackTimeout * 1000);

Example in the log:

[OS X Yosemite, Safari 8.0] Received ack
[OS X Yosemite, Safari 8.0] Launched
[OS X Yosemite, Safari 8.0] Awaiting ack

• 2. When trying to reload the page, it does not include the worker_key data and therefore showing "404 Not found" when trying to reload.

Example:

00:01 - Open url http://localhost:8888/test.html?_worker_key=1539fd49-2783-4e91-8cfd-e192afec9f2c&_browser_string=OS%20X%20Yosemite%2C%20Safari%208.0
01:08 - Open url http://localhost:8888/test.html?_worker_key=&_browser_string=OS%20X%20Yosemite%2C%20Safari%208.0

• 3. The function testActivityTimeout (in cli.js) will never execute anymore, because the property "acknowledged" on the worker object is never set to true. Instead another boolean is used to indicate if worker is acknowledge called isAckd. But that is not updated everywhere in the code.

I have done a temporary fix local, where I in the function "testActivityTimeout" try to reload the page 1 time if the test is not completed within the specified timeout.
It works and no more session timeout is visible.

Another solution I tried that worked was to listen if any request was made to "_/report" within the specified timeout, and if not reload the page, that also works.

Let me know if you have any questions.