Added improved http request retry mechanism and better error reportin…

…g for api commands

lib/api/_loaders/command.js

@@ -160,6 +160,21 @@ class CommandLoader extends BaseCommandLoader {
             }
 
             throw err;
+          })
+          .then(result => {
+            let reportErrors = instance.client.settings.report_command_errors;
+
+            if (result && result.code && result.error && result.status === -1) {
+              // node.js errors, e.g. ECONNRESET
+              reportErrors = true;
+            }
+
+            if (result && result.status === -1 && instance.reportProtocolErrors(result) && reportErrors) {
+              const error = new Error(`Error while running .${this.commandName}(): ${result.error}`);
+              instance.client.reporter.registerTestError(error);
+            }
+
+            return result;
           });
 
         if (instance instanceof EventEmitter) {

lib/api/client-commands/_base-command.js

@@ -40,6 +40,10 @@ module.exports = class ClientCommand {
     return true;
   }
 
+  reportProtocolErrors(result) {
+    return true;
+  }
+
   command(userSuppliedCallback) {
     const {performAction} = this;
 

lib/api/protocol/_base-action.js

@@ -23,10 +23,18 @@ module.exports = class ProtocolAction {
     }
   }
 
+  reportProtocolErrors(result) {
+    return true;
+  }
+
   get elementLocator() {
     return this.client.elementLocator;
   }
 
+  /**
+   * @type {Nightwatch}
+   * @return {*}
+   */
   get client() {
     return this.__nightwatchInstance;
   }
@@ -39,8 +47,7 @@ module.exports = class ProtocolAction {
    * @override
    * @return {Promise}
    */
-  command() {
-  }
+  async command() {}
 
   /*!
    *

lib/api/protocol/element.js

@@ -36,6 +36,10 @@ const ProtocolAction = require('./_base-action.js');
 const Element = require('../../element');
 
 module.exports = class Session extends ProtocolAction {
+  reportProtocolErrors(result) {
+    return result.code && result.message;
+  }
+
   command(using, value, callback) {
     const commandName = 'element';
     if (using instanceof Element) {

lib/api/protocol/openNewWindow.js

@@ -32,6 +32,11 @@ const ProtocolAction = require('./_base-action.js');
  */
 module.exports = class Session extends ProtocolAction {
   command(type = 'tab', callback) {
+    if (typeof type == 'function' && callback === undefined) {
+      callback = arguments[0];
+      type = 'tab';
+    }
+
     return this.transportActions.openNewWindow(type, callback);
   }
 };

lib/core/session.js

@@ -97,10 +97,10 @@ class Session extends EventEmitter {
         return;
     }
 
-    if (Array.isArray(opts.args)) {
-      opts.args.push(arg);
-    } else {
+    if (!Array.isArray(opts.args)) {
       opts.args = [arg];
+    } else if (!opts.args.includes(arg)) {
+      opts.args.push(arg);
     }
 
     lodashMerge(this.settings.capabilities, this.desiredCapabilities);

lib/http/request.js

@@ -128,14 +128,25 @@ class HttpRequest extends EventEmitter {
     events.forEach(event => {
       originalIssuer.on(event, (...args) => {
         args.unshift(event);
+
+        if (event === 'error' && this.shouldRetryRequest()) {
+          this.isAborted = true;
+          this.socket.unref();
+          this.retryCount = this.retryCount + 1;
+          this.send();
+          this.retryAttempts = this.retryAttempts - 1;
+
+          return;
+        }
+
         this.emit.apply(this, args);
       });
     });
   }
 
   createHttpRequest() {
     try {
-      let req = (this.httpOpts.use_ssl ? https: http).request(this.reqOptions, response => {
+      const req = (this.httpOpts.use_ssl ? https: http).request(this.reqOptions, response => {
         this.httpResponse = new HttpResponse(response, this);
         this.httpResponse.on('complete', this.logResponse.bind(this));
         this.proxyEvents(this.httpResponse, ['response', 'complete', 'success', 'error']);
@@ -206,30 +217,23 @@ class HttpRequest extends EventEmitter {
     this.startTime = new Date();
     this.isAborted = false;
 
+    const {hostname, data} = this;
+    const {method, path, headers} = this.reqOptions;
+
     this.emit('send', {
-      method: this.reqOptions.method,
-      hostname: this.hostname,
-      path: this.reqOptions.path,
-      data: this.data,
-      headers: this.reqOptions.headers
+      hostname,
+      data,
+      method,
+      path,
+      headers
     });
 
     this.httpRequest = this.createHttpRequest();
     this.logRequest();
-
     this.proxyEvents(this.httpRequest, ['error']);
 
     this.httpRequest.setTimeout(this.httpOpts.timeout, () => {
       this.httpRequest.abort();
-
-      if (this.shouldRetryRequest()) {
-        this.socket.unref();
-        this.isAborted = true;
-        this.retryCount = this.retryCount + 1;
-
-        this.send();
-        this.retryAttempts = this.retryAttempts - 1;
-      }
     });
 
     this.httpRequest.write(this.data);

lib/transport/jsonwire.js

@@ -153,7 +153,7 @@ class JsonWireProtocol extends Transport {
     if (code && message) {
       errorMessage = `Error ${code}: ${message}`;
     } else {
-      errorMessage = response.statusCode === 404 ? 'Unknown command' : 'An unknown error has occurred.';
+      errorMessage = response.statusCode === 404 ? 'Unknown command' : 'An unknown error has occurred';
     }
 
     if (screenshotContent) {

test/lib/globals.js

@@ -4,6 +4,7 @@ const assert = require('assert');
 const lodashMerge = require('lodash.merge');
 const common = require('../common.js');
 const Nightwatch = require('../lib/nightwatch.js');
+const MockServer  = require('../lib/mockserver.js')
 const Settings = common.require('settings/settings.js');
 const Runner = common.require('runner/runner.js');
 const Reporter = common.require('reporter/index.js');
@@ -57,7 +58,11 @@ class Globals {
     }
   }
 
-  protocolBefore(opts = {}) {
+  protocolAfter(done) {
+    this.server.close(() => done());
+  }
+
+  protocolBefore(opts = {}, done) {
     this.client = Nightwatch.createClient(opts);
     this.wdClient = Nightwatch.createClient({
       selenium: {
@@ -71,6 +76,11 @@ class Globals {
 
     this.client.session.sessionId = this.client.api.sessionId = '1352110219202';
     this.wdClient.session.sessionId = this.wdClient.api.sessionId = '1352110219202';
+
+    if (typeof done == 'function') {
+      this.server = MockServer.init();
+      this.server.on('listening', () => done());
+    }
   }
 
   protocolTest(definition) {
@@ -110,6 +120,72 @@ class Globals {
     });
   }
 
+  async runProtocolTestWithError({commandName, url, message = 'test message', method, args = []}) {
+    const SimplifiedReporter = common.require('reporter/simplified.js');
+    class Reporter extends SimplifiedReporter {
+      constructor(settings) {
+        super(settings);
+
+        this.errors = 0;
+      }
+
+      registerTestError(err) {
+        this.errors++;
+      }
+    }
+
+    const reporter = new Reporter({});
+    const client = await Nightwatch.initClient({
+      output: false,
+      report_command_errors: true,
+      silent: false
+    }, reporter);
+
+    MockServer.addMock({
+      url,
+      method,
+      statusCode: 500,
+      response: {
+        sessionId: '1352110219202',
+        state: 'unhandled error',
+        value: {
+          message
+        },
+        status: 13
+      }
+    }, true);
+
+    args.push(function callback(result) {
+      assert.deepStrictEqual(result, {
+        code: '',
+        value: {
+          message,
+          error: []
+        },
+        error: 'An unknown server-side error occurred while processing the command. – ' + message,
+        errorStatus: 13,
+        httpStatusCode: 500,
+        state: 'unhandled error',
+        status: -1
+      });
+
+      return result;
+    });
+
+    client.api[commandName](...args);
+
+    await new Promise((resolve, reject) => {
+      client.start(err => {
+        if (err) {
+          reject(err);
+        } else {
+          resolve();
+        }
+      });
+    });
+    assert.strictEqual(client.reporter.errors, 1);
+  }
+
   runApiCommand(commandName, args, client = this.client) {
     let context;
     let commandFn;
@@ -169,7 +245,7 @@ function addSettings(settings) {
       retryAssertionTimeout: 5,
       waitForConditionPollInterval: 3
     },
-    output: true,
+    output: false,
     silent: false
   }, settings);
 }

test/lib/globals/commands.js

@@ -11,7 +11,7 @@ module.exports = {
         webdriver:{
           start_process: false
         },
-        output: false,
+        output: process.env.VERBOSE === '1' || false,
         silent: false
       })
         .then(client => {

test/sampletests/withcommanderrors/demoTest.js

@@ -0,0 +1,10 @@
+module.exports = {
+  'Demo test' : function (browser) {
+    browser
+      .url('http://localhost')
+      .waitForElementPresent('#weblogin')
+      .openNewWindow('window')
+      .pause(100)
+      .end();
+  }
+};

test/src/api/commands/testClearValue.js

@@ -3,17 +3,14 @@ const MockServer  = require('../../../lib/mockserver.js');
 const Nightwatch = require('../../../lib/nightwatch.js');
 
 describe('clearValue', function() {
+
   before(function(done) {
     this.server = MockServer.init();
-    this.server.on('listening', () => {
-      done();
-    });
+    this.server.on('listening', () => done());
   });
 
   after(function(done) {
-    this.server.close(function () {
-      done();
-    });
+    this.server.close(() => done());
   });
 
   it('client.clearValue()', function(done) {

test/src/api/commands/testCloseWindow.js

@@ -0,0 +1,23 @@
+const assert = require('assert');
+const MockServer  = require('../../../lib/mockserver.js');
+const CommandGlobals = require('../../../lib/globals/commands.js');
+
+describe('closeWindow', function() {
+  before(function(done) {
+    CommandGlobals.beforeEach.call(this, done);
+  });
+
+  after(function(done) {
+    CommandGlobals.afterEach.call(this, done);
+  });
+
+  it('client.closeWindow() success', function(done) {
+    const api = this.client.api;
+    this.client.api.closeWindow(function callback(result) {
+      assert.strictEqual(this, api);
+      assert.strictEqual(result.status, 0);
+    });
+
+    this.client.start(done);
+  });
+});