Skip to content

Commit

Permalink
Restart browser if it became unresponsive (closes #1815) (#2800)
Browse files Browse the repository at this point in the history 
* [WIP]Restart browser if it became unresponsive (closes #1815)

* tests

* change approach

* fix server tests

* fix for headless browsers

* fix functional test

* refactor and fix tests

* refactoring and tests

* refactoring

* refactor

* refactoring

* fix and refactoring

* add listenered to connection only when testRun is started

* rename and fix typos
  • Loading branch information
@AlexKamaev
AlexKamaev authored Sep 11, 2018
1 parent 7ac5ac6 commit 462606c
Show file tree
Hide file tree
Showing 9 changed files with 292 additions and 62 deletions.
2 changes: 1 addition & 1 deletion Gulpfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ function testFunctional (fixturesDir, testingEnvironmentName, browserProviderNam
.pipe(mocha({
ui: 'bdd',
reporter: 'spec',
timeout: typeof v8debug === 'undefined' ? 30000 : Infinity // NOTE: disable timeouts in debug
timeout: typeof v8debug === 'undefined' ? 180000 : Infinity // NOTE: disable timeouts in debug
}));
}

Expand Down
109 changes: 81 additions & 28 deletions src/browser/connection/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,22 @@ import { GeneralError } from '../../errors/runtime';
import MESSAGE from '../../errors/runtime/message';

const IDLE_PAGE_TEMPLATE = read('../../client/browser/idle-page/index.html.mustache');
const connections = {};

var connections = {};

export default class BrowserConnection extends EventEmitter {
constructor (gateway, browserInfo, permanent) {
super();

this.HEARTBEAT_TIMEOUT = 2 * 60 * 1000;
this.HEARTBEAT_TIMEOUT = 2 * 60 * 1000;
this.BROWSER_RESTART_TIMEOUT = 60 * 1000;

this.id = BrowserConnection._generateId();
this.jobQueue = [];
this.initScriptsQueue = [];
this.browserConnectionGateway = gateway;
this.errorSuppressed = false;
this.testRunAborted = false;

this.browserInfo = browserInfo;
this.browserInfo.userAgent = '';
Expand Down Expand Up @@ -63,33 +66,30 @@ export default class BrowserConnection extends EventEmitter {

this.browserConnectionGateway.startServingConnection(this);

this._runBrowser();
process.nextTick(() => this._runBrowser());
}

static _generateId () {
return nanoid(7);
}

_runBrowser () {
// NOTE: Give caller time to assign event listeners
process.nextTick(async () => {
try {
await this.provider.openBrowser(this.id, this.url, this.browserInfo.browserName);
async _runBrowser () {
try {
await this.provider.openBrowser(this.id, this.url, this.browserInfo.browserName);

if (!this.ready)
await promisifyEvent(this, 'ready');
if (!this.ready)
await promisifyEvent(this, 'ready');

this.opened = true;
this.emit('opened');
}
catch (err) {
this.emit('error', new GeneralError(
MESSAGE.unableToOpenBrowser,
this.browserInfo.providerName + ':' + this.browserInfo.browserName,
err.stack
));
}
});
this.opened = true;
this.emit('opened');
}
catch (err) {
this.emit('error', new GeneralError(
MESSAGE.unableToOpenBrowser,
this.browserInfo.providerName + ':' + this.browserInfo.browserName,
err.stack
));
}
}

async _closeBrowser () {
Expand All @@ -112,14 +112,28 @@ export default class BrowserConnection extends EventEmitter {
}
}

_createBrowserDisconnectedError () {
return new GeneralError(MESSAGE.browserDisconnected, this.userAgent);
}

_waitForHeartbeat () {
this.heartbeatTimeout = setTimeout(() => {
this.emit('error', new GeneralError(MESSAGE.browserDisconnected, this.userAgent));
const err = this._createBrowserDisconnectedError();

this.opened = false;
this.errorSuppressed = false;
this.testRunAborted = true;

this.emit('disconnected', err);

if (!this.errorSuppressed)
this.emit('error', err);

}, this.HEARTBEAT_TIMEOUT);
}

async _getTestRunUrl (isTestDone) {
if (isTestDone || !this.pendingTestRunUrl)
async _getTestRunUrl (needPopNext) {
if (needPopNext || !this.pendingTestRunUrl)
this.pendingTestRunUrl = await this._popNextTestRunUrl();

return this.pendingTestRunUrl;
Expand All @@ -136,6 +150,43 @@ export default class BrowserConnection extends EventEmitter {
return connections[id] || null;
}

async restartBrowser () {
this.ready = false;

this._forceIdle();

let resolveTimeout = null;
let isTimeoutExpired = false;
let timeout = null;

const restartPromise = this._closeBrowser()
.then(() => this._runBrowser());

const timeoutPromise = new Promise(resolve => {
resolveTimeout = resolve;

timeout = setTimeout(() => {
isTimeoutExpired = true;

resolve();
}, this.BROWSER_RESTART_TIMEOUT);
});

Promise.race([ restartPromise, timeoutPromise ])
.then(() => {
clearTimeout(timeout);

if (isTimeoutExpired)
this.emit('error', this._createBrowserDisconnectedError());
else
resolveTimeout();
});
}

suppressError () {
this.errorSuppressed = true;
}

addWarning (...args) {
if (this.currentJob)
this.currentJob.warningLog.addWarning(...args);
Expand All @@ -146,7 +197,7 @@ export default class BrowserConnection extends EventEmitter {
}

get userAgent () {
var userAgent = this.browserInfo.userAgent;
let userAgent = this.browserInfo.userAgent;

if (this.browserInfo.userAgentProviderMetaInfo)
userAgent += ` (${this.browserInfo.userAgentProviderMetaInfo})`;
Expand Down Expand Up @@ -228,13 +279,13 @@ export default class BrowserConnection extends EventEmitter {
}

getInitScript () {
var initScriptPromise = this.initScriptsQueue[0];
const initScriptPromise = this.initScriptsQueue[0];

return { code: initScriptPromise ? initScriptPromise.code : null };
}

handleInitScriptResult (data) {
var initScriptPromise = this.initScriptsQueue.shift();
const initScriptPromise = this.initScriptsQueue.shift();

if (initScriptPromise)
initScriptPromise.resolve(JSON.parse(data));
Expand All @@ -251,7 +302,9 @@ export default class BrowserConnection extends EventEmitter {
}

if (this.opened) {
var testRunUrl = await this._getTestRunUrl(isTestDone);
const testRunUrl = await this._getTestRunUrl(isTestDone || this.testRunAborted);

this.testRunAborted = false;

if (testRunUrl) {
this.idle = false;
Expand Down
6 changes: 3 additions & 3 deletions src/runner/browser-job.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default class BrowserJob extends EventEmitter {
}

_createTestRunController (test, index) {
var testRunController = new TestRunController(test, index + 1, this.proxy, this.screenshots, this.warningLog,
const testRunController = new TestRunController(test, index + 1, this.proxy, this.screenshots, this.warningLog,
this.fixtureHookController, this.opts);

testRunController.on('test-run-start', () => this.emit('test-run-start', testRunController.testRun));
Expand Down Expand Up @@ -100,7 +100,7 @@ export default class BrowserJob extends EventEmitter {
if (this.testRunControllerQueue[0].blocked)
break;

var testRunController = this.testRunControllerQueue.shift();
const testRunController = this.testRunControllerQueue.shift();

this._addToCompletionQueue(testRunController);

Expand All @@ -109,7 +109,7 @@ export default class BrowserJob extends EventEmitter {
this.emit('start');
}

var testRunUrl = await testRunController.start(connection);
const testRunUrl = await testRunController.start(connection);

if (testRunUrl)
return testRunUrl;
Expand Down
29 changes: 24 additions & 5 deletions src/runner/test-run-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import TestRun from '../test-run';
import SessionController from '../test-run/session-controller';

const QUARANTINE_THRESHOLD = 3;
const DISCONNECT_THRESHOLD = 3;

class Quarantine {
constructor () {
Expand Down Expand Up @@ -38,9 +39,10 @@ export default class TestRunController extends EventEmitter {

this.TestRunCtor = TestRunController._getTestRunCtor(test, opts);

this.testRun = null;
this.done = false;
this.quarantine = null;
this.testRun = null;
this.done = false;
this.quarantine = null;
this.disconnectionCount = 0;

if (this.opts.quarantineMode)
this.quarantine = new Quarantine();
Expand All @@ -54,8 +56,8 @@ export default class TestRunController extends EventEmitter {
}

_createTestRun (connection) {
var screenshotCapturer = this.screenshots.createCapturerFor(this.test, this.index, this.quarantine, connection, this.warningLog);
var TestRunCtor = this.TestRunCtor;
const screenshotCapturer = this.screenshots.createCapturerFor(this.test, this.index, this.quarantine, connection, this.warningLog);
const TestRunCtor = this.TestRunCtor;

this.testRun = new TestRunCtor(this.test, connection, screenshotCapturer, this.warningLog, this.opts);

Expand Down Expand Up @@ -89,6 +91,10 @@ export default class TestRunController extends EventEmitter {
}

_keepInQuarantine () {
this._restartTest();
}

_restartTest () {
this.emit('test-run-restart');
}

Expand Down Expand Up @@ -116,6 +122,18 @@ export default class TestRunController extends EventEmitter {
this.emit('test-run-done');
}

async _testRunDisconnected (connection) {
this.disconnectionCount++;

if (this.disconnectionCount < DISCONNECT_THRESHOLD) {
connection.suppressError();

await connection.restartBrowser();

this._restartTest();
}
}

get blocked () {
return this.fixtureHookController.isTestBlocked(this.test);
}
Expand All @@ -133,6 +151,7 @@ export default class TestRunController extends EventEmitter {

testRun.once('start', () => this.emit('test-run-start'));
testRun.once('done', () => this._testRunDone());
testRun.once('disconnected', () => this._testRunDisconnected(connection));

testRun.start();

Expand Down
Loading

0 comments on commit 462606c

Please sign in to comment.