Skip to content

Commit

Permalink
Fixed a problem with the request retries in case of a timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
beatfactor committed Jan 1, 2017
1 parent 387726e commit 5c5534f
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 6 deletions.
11 changes: 9 additions & 2 deletions lib/http/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ module.exports = (function() {
HttpRequest.prototype.send = function() {
var self = this;
var startTime = new Date();
var isAborted = false;

this.request = (Settings.use_ssl ? https: http).request(this.reqOptions, function (response) {
response.setEncoding('utf8');
var redirected = false;
Expand Down Expand Up @@ -158,6 +160,9 @@ module.exports = (function() {
}

if (response.statusCode.toString().indexOf('2') === 0 || redirected) {
if (isAborted) {
return;
}
self.emit('success', result, response, redirected);
} else {
self.emit('error', result, response, screenshotContent);
Expand All @@ -170,11 +175,13 @@ module.exports = (function() {
});

this.request.setTimeout(this.timeout, function() {
self.request.abort();

if (self.retryAttempts) {
self.request.socket.unref();
isAborted = true; // prevent emitting of the success event multiple times.
self.retryAttempts = self.retryAttempts - 1;
self.send();
} else {
self.request.abort();
}
});

Expand Down
2 changes: 1 addition & 1 deletion lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ Nightwatch.prototype.setOptions = function(options) {
if (typeof timeoutOptions.timeout != 'undefined') {
HttpRequest.setTimeout(timeoutOptions.timeout);
}
if (typeof timeoutOptions.retry_attempts != 'undefined') {
if (typeof timeoutOptions.retry_attempts != 'undefined') {
HttpRequest.setRetryAttempts(timeoutOptions.retry_attempts);
}

Expand Down
11 changes: 10 additions & 1 deletion lib/runner/clientmanager.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
var util = require('util');
var http = require('http');
var events = require('events');
var Q = require('q');
var Nightwatch = require('../index.js');
Expand Down Expand Up @@ -49,9 +50,17 @@ ClientManager.prototype.start = function(done) {
});

this['@client'].once('error', function(error, data) {
var errorMsg = error || data;
if (errorMsg instanceof http.IncomingMessage) {
errorMsg = {
statusCode : error.statusCode,
headers : error.headers
};
}

var result = {
message: 'Connection refused! Is selenium server started?\n',
data : error || data
data : errorMsg
};

self.emit('error', result, false);
Expand Down
3 changes: 1 addition & 2 deletions test/src/http/testRequestTimeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ module.exports = {

request
.on('error', function(res, err) {
assert.strictEqual(request.retryAttempts, 0);
assert.ok(err instanceof Error); // the 'socket hang up' error, caused by aborting the request
done(err);
})
.on('success', function (result) {
assert.strictEqual(request.retryAttempts, 0);
Expand Down

0 comments on commit 5c5534f

Please sign in to comment.