Skip to content

Commit 6afe714

Browse files
shirish87shirish87
committed
Merge pull request #123 from shirish87/retry
Retry opening URL if worker-browser fails to send request within ackTimeout
2 parents b1a8b92 + b73192c commit 6afe714

File tree

2 files changed

+102
-38
lines changed

2 files changed

+102
-38
lines changed

‎bin/cli.js

Lines changed: 88 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ var Log = require('../lib/logger'),
2020
logger = new Log(global.logLevel),
2121
BrowserStack = require('browserstack'),
2222
fs = require('fs'),
23+
qs = require('querystring'),
2324
chalk = require('chalk'),
2425
config = require('../lib/config'),
2526
utils = require('../lib/utils'),
@@ -32,6 +33,7 @@ var Log = require('../lib/logger'),
3233
server,
3334
timeout,
3435
activityTimeout,
36+
ackTimeout,
3537
workers = {},
3638
workerKeys = {},
3739
tunnelingAgent,
@@ -43,6 +45,7 @@ function terminateAllWorkers(callback) {
4345
var worker = workers[key];
4446
if(worker) {
4547
logger.debug('[%s] Terminated', worker.string);
48+
clearTimeout(worker.ackTimeout);
4649
clearTimeout(worker.activityTimeout);
4750
clearTimeout(worker.testActivityTimeout);
4851
delete workers[key];
@@ -109,9 +112,24 @@ function getTestBrowserInfo(browserString, path) {
109112
if (config.multipleTest) {
110113
info += ', ' + path;
111114
}
115+
112116
return info;
113117
}
114118

119+
120+
function buildTestUrl(test_path, worker_key, browser_string) {
121+
var url = 'http://localhost:' + serverPort + '/' + test_path;
122+
123+
var querystring = qs.stringify({
124+
_worker_key: worker_key,
125+
_browser_string: browser_string
126+
});
127+
128+
url += ((url.indexOf('?') > 0) ? '&' : '?') + querystring;
129+
return url;
130+
}
131+
132+
115133
function launchServer() {
116134
logger.debug('Launching server on port:', serverPort);
117135

@@ -120,20 +138,12 @@ function launchServer() {
120138
}
121139

122140
function launchBrowser(browser, path) {
123-
var url = 'http://localhost:' + serverPort.toString() + '/' + path.replace(/\\/g, '/');
124-
var browserString = utils.browserString(browser);
125-
logger.debug('[%s] Launching', getTestBrowserInfo(browserString, path));
126-
127141
var key = utils.uuid();
142+
var browserString = utils.browserString(browser);
143+
var browserInfo = getTestBrowserInfo(browserString, path);
144+
logger.debug('[%s] Launching', browserInfo);
128145

129-
if (url.indexOf('?') > 0) {
130-
url += '&';
131-
} else {
132-
url += '?';
133-
}
134-
135-
url += '_worker_key=' + key + '&_browser_string=' + browserString;
136-
browser['url'] = url;
146+
browser.url = buildTestUrl(path.replace(/\\/g, '/'), key, browserString);
137147

138148
if (config.project) {
139149
browser.project = config.project;
@@ -153,6 +163,7 @@ function launchBrowser(browser, path) {
153163
timeout = 300;
154164
}
155165
activityTimeout = timeout - 10;
166+
ackTimeout = parseInt(config.ackTimeout) || 60;
156167

157168
client.createWorker(browser, function (err, worker) {
158169
if (err || typeof worker !== 'object') {
@@ -169,10 +180,13 @@ function launchBrowser(browser, path) {
169180
worker.string = browserString;
170181
worker.test_path = path;
171182
worker.path_index = 0;
183+
184+
// attach helper methods to manage worker state
185+
attachWorkerHelpers(worker);
186+
172187
workers[key] = worker;
173188
workerKeys[worker.id] = {key: key, marked: false};
174189
});
175-
176190
}
177191

178192
function launchBrowsers(config, browser) {
@@ -187,6 +201,63 @@ function launchBrowsers(config, browser) {
187201
}, 100);
188202
}
189203

204+
205+
function attachWorkerHelpers(worker) {
206+
// TODO: Consider creating instances of a proper 'Worker' class
207+
208+
worker.buildUrl = function buildUrl(test_path) {
209+
return buildTestUrl(test_path || this.test_path, this._worker_key, this.getTestBrowserInfo());
210+
};
211+
212+
worker.getTestBrowserInfo = function getTestBrowserInfo(test_path) {
213+
var info = this.string;
214+
if (config.multipleTest) {
215+
info += ', ' + (test_path || this.test_path);
216+
}
217+
return info;
218+
};
219+
220+
worker.awaitAck = function awaitAck() {
221+
var self = this;
222+
223+
if (this.ackTimeout) {
224+
// Already awaiting ack, or awaited ack once and failed
225+
return;
226+
}
227+
228+
this.ackTimeout = setTimeout(function () {
229+
if (self.isAckd) {
230+
// Already ack'd
231+
return;
232+
}
233+
234+
// worker has not acknowledged itself in 60 sec, reopen url
235+
client.changeUrl(self.id, { url: self.buildUrl() }, function () {
236+
logger.debug("[%s] Sent Request to reload url", self.getTestBrowserInfo());
237+
});
238+
239+
}, ackTimeout * 1000);
240+
241+
logger.debug('[%s] Awaiting ack', this.getTestBrowserInfo());
242+
};
243+
244+
worker.markAckd = function markAckd() {
245+
this.resetAck();
246+
this.isAckd = true;
247+
248+
logger.debug('[%s] Received ack', this.getTestBrowserInfo());
249+
};
250+
251+
worker.resetAck = function resetAck() {
252+
clearTimeout(this.ackTimeout);
253+
this.ackTimeout = null;
254+
this.isAckd = false;
255+
};
256+
257+
return worker;
258+
}
259+
260+
190261
var statusPoller = {
191262
poller: null,
192263

@@ -208,10 +279,13 @@ var statusPoller = {
208279

209280
if (_worker.status === 'running') {
210281
//clearInterval(statusPoller);
211-
logger.debug('[%s] Launched', getTestBrowserInfo(worker.string, worker.test_path));
282+
logger.debug('[%s] Launched', worker.getTestBrowserInfo());
212283
worker.launched = true;
213284
workerData.marked = true;
214285

286+
// Await ack from browser-worker
287+
worker.awaitAck();
288+
215289
worker.activityTimeout = setTimeout(function () {
216290
if (!worker.acknowledged) {
217291
var subject = 'Worker inactive for too long: ' + worker.string;

‎lib/server.js

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,6 @@ var mimeTypes = {
2020
'css': 'text/css'
2121
};
2222

23-
function getTestBrowserInfo(worker) {
24-
var info = worker.string;
25-
if(config.multipleTest) {
26-
info += ', ' + worker.test_path;
27-
}
28-
return info;
29-
}
30-
3123

3224
exports.Server = function Server(bsClient, workers) {
3325

@@ -37,8 +29,7 @@ exports.Server = function Server(bsClient, workers) {
3729

3830
if (query._worker_key && workers[query._worker_key]) {
3931
var worker = workers[query._worker_key];
40-
worker.acknowledged = true;
41-
logger.debug('[%s] Acknowledged', getTestBrowserInfo(worker));
32+
worker.markAckd();
4233
}
4334

4435
var getReporterPatch = function (mimeType) {
@@ -187,18 +178,14 @@ exports.Server = function Server(bsClient, workers) {
187178
function checkAndTerminateWorker(worker, callback) {
188179
var next_path = getNextTestPath(worker);
189180
if (next_path) {
190-
var url = 'http://localhost:' + 8888 + '/' + next_path;
191-
if (url.indexOf('?') > 0) {
192-
url += '&';
193-
} else {
194-
url += '?';
195-
}
196-
url += '_worker_key=' + worker._worker_key + '&_browser_string=' + getTestBrowserInfo(worker) ;
181+
var url = worker.buildUrl(next_path);
197182
worker.test_path = next_path;
198183
worker.config.url = next_path;
199-
bsClient.changeUrl(worker.id, {url: url}, function() {
184+
185+
bsClient.changeUrl(worker.id, { url: url }, function () {
200186
callback(true);
201187
});
188+
202189
} else {
203190
bsClient.terminateWorker(worker.id, callback);
204191
}
@@ -225,7 +212,7 @@ exports.Server = function Server(bsClient, workers) {
225212

226213
if (query.tracebacks) {
227214
query.tracebacks.forEach(function(traceback) {
228-
logger.info('[%s] ' + chalk.red('Error:'), getTestBrowserInfo(worker), formatTraceback(traceback));
215+
logger.info('[%s] ' + chalk.red('Error:'), worker.getTestBrowserInfo(), formatTraceback(traceback));
229216
});
230217
}
231218
response.end();
@@ -244,19 +231,19 @@ exports.Server = function Server(bsClient, workers) {
244231
logger.info('[%s] Null response from remote Browser', request.headers['x-browser-string']);
245232
} else {
246233
if (query.tracebacks && query.tracebacks.length > 0) {
247-
logger.info('[%s] ' + chalk['red']('Tracebacks:'), getTestBrowserInfo(worker));
234+
logger.info('[%s] ' + chalk['red']('Tracebacks:'), worker.getTestBrowserInfo());
248235
query.tracebacks.forEach(function(traceback) {
249236
logger.info(traceback);
250237
});
251238
}
252239
var color = query.failed ? 'red' : 'green';
253-
logger.info('[%s] ' + chalk[color](query.failed ? 'Failed:' : 'Passed:') + ' %d tests, %d passed, %d failed; ran for %dms', getTestBrowserInfo(worker), query.total, query.passed, query.failed, query.runtime);
240+
logger.info('[%s] ' + chalk[color](query.failed ? 'Failed:' : 'Passed:') + ' %d tests, %d passed, %d failed; ran for %dms', worker.getTestBrowserInfo(), query.total, query.passed, query.failed, query.runtime);
254241
config.status += query.failed;
255242
}
256243

257244
bsClient.takeScreenshot(worker.id, function(error, screenshot) {
258245
if (!error && screenshot.url && query && query.failed) {
259-
logger.info('[%s] ' + chalk.yellow('Screenshot:') + ' %s', getTestBrowserInfo(worker), screenshot.url);
246+
logger.info('[%s] ' + chalk.yellow('Screenshot:') + ' %s', worker.getTestBrowserInfo(), screenshot.url);
260247
}
261248

262249
checkAndTerminateWorker(worker, function(reusedWorker) {
@@ -265,12 +252,15 @@ exports.Server = function Server(bsClient, workers) {
265252
}
266253

267254
if (reusedWorker) {
268-
logger.debug('[%s] Reused', getTestBrowserInfo(worker));
255+
logger.debug('[%s] Reused', worker.getTestBrowserInfo());
256+
worker.resetAck();
257+
worker.awaitAck();
269258
return;
270259
}
271260

272-
logger.debug('[%s] Terminated', getTestBrowserInfo(worker));
261+
logger.debug('[%s] Terminated', worker.getTestBrowserInfo());
273262

263+
clearTimeout(workers[uuid].ackTimeout);
274264
clearTimeout(workers[uuid].activityTimeout);
275265
clearTimeout(workers[uuid].testActivityTimeout);
276266
delete workers[uuid];

0 commit comments

Comments
 (0)