[js] Move all logic for interpreting responses from the remote end to the

command executor.

Author: jleyba

javascript/node/selenium-webdriver/CHANGES.md

@@ -2,6 +2,8 @@
 
 ### Change Summary
 
+* Moved all logic for parsing and interpreting responses from the remote end
+   into the individual `command.Executor` implementations.
 * For consistency with the other Selenium language bindings,
    `WebDriver#isElementPresent()` and `WebElement#isElementPresent()` have
    been deprecated. These methods will be removed in v3.0. Use the findElements

javascript/node/selenium-webdriver/error.js

@@ -577,7 +577,16 @@ function checkLegacyResponse(responseObj) {
     if (!value || typeof value !== 'object') {
       throw new ctor(value + '');
     } else {
-      throw new ctor(value['message'] + '');
+      let message = value['message'] + '';
+      if (ctor !== UnexpectedAlertOpenError) {
+        throw new ctor(message);
+      }
+
+      let text = '';
+      if (value['alert'] && typeof value['alert']['text'] === 'string') {
+        text = value['alert']['text'];
+      }
+      throw new UnexpectedAlertOpenError(message, text);
     }
   }
   return responseObj;

javascript/node/selenium-webdriver/http/index.js

@@ -29,6 +29,7 @@ const error = require('../error');
 const cmd = require('../lib/command');
 const logging = require('../lib/logging');
 const promise = require('../lib/promise');
+const Session = require('../lib/session').Session;
 
 
 
@@ -432,40 +433,62 @@ class Executor {
     log.finer(() => '>>>\n' + request);
     return this.client_.send(request).then(function(response) {
       log.finer(() => '<<<\n' + response);
-      return parseHttpResponse(/** @type {!HttpResponse} */ (response));
+
+      let value = parseHttpResponse(/** @type {!HttpResponse} */ (response));
+      let isResponseObj = (value && typeof value === 'object');
+      if (command.getName() === cmd.Name.NEW_SESSION) {
+        if (!isResponseObj) {
+          throw new error.WebDriverError(
+              'Unable to parse new session response: ' + response.body);
+        }
+        return new Session(value['sessionId'], value['value']);
+      }
+      return isResponseObj ? value['value'] : value;
     });
   }
 }
 
 
+/**
+ * @param {string} str .
+ * @return {?} .
+ */
+function tryParse(str) {
+  try {
+    return JSON.parse(str);
+  } catch (ignored) {
+    // Do nothing.
+  }
+}
+
+
 /**
  * Callback used to parse {@link HttpResponse} objects from a
  * {@link HttpClient}.
  * @param {!HttpResponse} httpResponse The HTTP response to parse.
  * @return {!Object} The parsed response.
  */
 function parseHttpResponse(httpResponse) {
-  try {
-    return /** @type {!Object} */ (JSON.parse(httpResponse.body));
-  } catch (ignored) {
-    // Whoops, looks like the server sent us a malformed response. We'll need
-    // to manually build a response object based on the response code.
+  let parsed = tryParse(httpResponse.body);
+  if (parsed !== undefined) {
+    error.checkLegacyResponse(parsed);
+    return parsed;
+  }
+
+  let value = httpResponse.body.replace(/\r\n/g, '\n');
+  if (!value) {
+    return null;
   }
 
-  let response = {
-    'status': error.ErrorCode.SUCCESS,
-    'value': httpResponse.body.replace(/\r\n/g, '\n')
-  };
-
-  if (httpResponse.status >= 400) {
-    // 404 represents an unknown command; anything else is a generic unknown
-    // error.
-    response['status'] = httpResponse.status == 404 ?
-        error.ErrorCode.UNKNOWN_COMMAND :
-        error.ErrorCode.UNKNOWN_ERROR;
+  // 404 represents an unknown command; anything else is a generic unknown
+  // error.
+  if (httpResponse.status == 404) {
+    throw new error.UnsupportedOperationError(value);
+  } else if (httpResponse.status >= 400) {
+    throw new error.WebDriverError(value);
   }
 
-  return response;
+  return value;
 }
 
 

javascript/node/selenium-webdriver/http/util.js

@@ -21,8 +21,7 @@
 
 'use strict';
 
-const error = require('../error'),
-    Executor = require('./index').Executor,
+const Executor = require('./index').Executor,
     HttpClient = require('./index').HttpClient,
     HttpRequest = require('./index').Request,
     Command = require('../lib/command').Command,
@@ -41,10 +40,7 @@ function getStatus(url) {
   var client = new HttpClient(url);
   var executor = new Executor(client);
   var command = new Command(CommandName.GET_SERVER_STATUS);
-  return executor.execute(command).then(function(responseObj) {
-    error.checkLegacyResponse(responseObj);
-    return responseObj['value'];
-  });
+  return executor.execute(command);
 }
 
 

javascript/node/selenium-webdriver/lib/command.js

@@ -227,7 +227,7 @@ class Executor {
    * response object.
    *
    * @param {!Command} command The command to execute.
-   * @return {!promise.Promise<!Object>} A promise that will be fulfilled with
+   * @return {!promise.Promise<?>} A promise that will be fulfilled with
    *     the command result.
    */
   execute(command) {}

javascript/node/selenium-webdriver/lib/webdriver.js

@@ -60,10 +60,7 @@ const until = require('./until');
 function acquireSession(executor, command, description, opt_flow) {
   let flow = opt_flow || promise.controlFlow();
   let session = flow.execute(function() {
-    return executeCommand(executor, command).then(function(response) {
-      error.checkLegacyResponse(response);
-      return new Session(response['sessionId'], response['value']);
-    });
+    return executeCommand(executor, command);
   }, description);
   return new WebDriver(session, executor, flow);
 }
@@ -354,24 +351,8 @@ class WebDriver {
       return prepCommand.then(function(parameters) {
         command.setParameters(parameters);
         return executor.execute(command);
-      });
-    }, description).then(function(response) {
-      try {
-        error.checkLegacyResponse(response);
-      } catch (ex) {
-        if (ex instanceof error.UnexpectedAlertOpenError) {
-          let text = '';
-          if (response['value']
-              && response['value']['alert']
-              && typeof response['value']['alert']['text'] === 'string') {
-            text = response['value']['alert']['text'];
-          }
-          throw new error.UnexpectedAlertOpenError(ex.message, text);
-        }
-        throw ex;
-      }
-      return fromWireValue(self, response['value']);
-    });
+      }).then(value => fromWireValue(self, value));
+    }, description);
 
     function checkHasNotQuit() {
       if (!self.session_) {

javascript/node/selenium-webdriver/safari.js

@@ -34,6 +34,7 @@ const url = require('url');
 const util = require('util');
 const ws = require('ws');
 
+const error = require('./error');
 const io = require('./io');
 const exec = require('./io/exec');
 const isDevMode = require('./lib/devmode');
@@ -303,8 +304,7 @@ class CommandExecutor {
           break;
 
         case command.Name.QUIT:
-          self.destroySession_()
-              .then(() => fulfill({status: 0, value: null}), reject);
+          self.destroySession_().then(() => fulfill(null), reject);
           break;
 
         default:
@@ -344,7 +344,13 @@ class CommandExecutor {
           reject(Error('Failed to parse driver message: ' + data));
           return;
         }
-        fulfill(data['response']);
+
+        try {
+          error.checkLegacyResponse(data['response']);
+          fulfill(data['response']['value']);
+        } catch (ex) {
+          reject(ex);
+        }
       });
     });
   }

javascript/node/selenium-webdriver/test/error_test.js

@@ -109,6 +109,43 @@ describe('error', function() {
     test('INVALID_XPATH_SELECTOR_RETURN_TYPE', error.InvalidSelectorError);
     test('METHOD_NOT_ALLOWED', error.UnsupportedOperationError);
 
+    describe('UnexpectedAlertOpenError', function() {
+      it('includes alert text from the response object', function() {
+        let response = {
+          status: error.ErrorCode.UNEXPECTED_ALERT_OPEN,
+          value: {
+            message: 'hi',
+            alert: {text: 'alert text here'}
+          }
+        };
+        assert.throws(
+            () => error.checkLegacyResponse(response),
+            (e) => {
+              assert.equal(error.UnexpectedAlertOpenError, e.constructor);
+              assert.equal(e.message, 'hi');
+              assert.equal(e.getAlertText(), 'alert text here');
+              return true;
+            });
+      });
+
+      it('uses an empty string if alert text omitted', function() {
+        let response = {
+          status: error.ErrorCode.UNEXPECTED_ALERT_OPEN,
+          value: {
+            message: 'hi'
+          }
+        };
+        assert.throws(
+            () => error.checkLegacyResponse(response),
+            (e) => {
+              assert.equal(error.UnexpectedAlertOpenError, e.constructor);
+              assert.equal(e.message, 'hi');
+              assert.equal(e.getAlertText(), '');
+              return true;
+            });
+      });
+    });
+
     function test(codeKey, expectedType) {
       it(`${codeKey} => ${expectedType.name}`, function() {
         let code = error.ErrorCode[codeKey];