[js] Fix new session parsing for geckodriver 0.15.0 (breaks support for older

versions)

Fixes SeleniumHQ#3625

Author: jleyba

javascript/node/selenium-webdriver/CHANGES.md

@@ -1,11 +1,22 @@
 ## v.next
 
+### Notice
+
+This release requires [geckodriver 0.15.0](https://github.com/mozilla/geckodriver/releases/tag/v0.15.0) or newer.
+
+### API Changes
+
 * Added `Options#getTimeouts()` for retrieving the currently configured session
   timeouts (i.e. implicit wait). This method will only work with W3C compatible
   WebDriver implementations.
 * Deprecated the `Timeouts` class in favor of `Options#setTimeouts()`, which
   supports setting multiple timeouts at once.
 
+### Changes for W3C WebDriver Spec Compliance
+
+* Fix W3C response parsing, which expects response data to always be a JSON
+  object with a `value` key.
+
 
 ## v3.3.0
 

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

@@ -429,34 +429,29 @@ class Executor {
       return doSend(this, request).then(response => {
         this.log_.finer(() => `>>>\n${request}\n<<<\n${response}`);
 
-        let parsed =
-            parseHttpResponse(
-                command, /** @type {!Response} */ (response), this.w3c);
+        let httpResponse = /** @type {!Response} */(response);
+        let {isW3C, value} = parseHttpResponse(command, httpResponse);
 
         if (command.getName() === cmd.Name.NEW_SESSION
             || command.getName() === cmd.Name.DESCRIBE_SESSION) {
-          if (!parsed || !parsed['sessionId']) {
+          if (!value || !value.sessionId) {
             throw new error.WebDriverError(
-                'Unable to parse new session response: ' + response.body);
+                `Unable to parse new session response: ${response.body}`);
           }
 
           // The remote end is a W3C compliant server if there is no `status`
           // field in the response. This is not applicable for the DESCRIBE_SESSION
           // command, which is not defined in the W3C spec.
           if (command.getName() === cmd.Name.NEW_SESSION) {
-            this.w3c = this.w3c || !('status' in parsed);
+            this.w3c = this.w3c || isW3C;
           }
 
-          return new Session(parsed['sessionId'], parsed['value']);
+          // No implementations use the `capabilities` key yet...
+          let capabilities = value.capabilities || value.value;
+          return new Session(value.sessionId, capabilities);
         }
 
-        if (parsed
-            && typeof parsed === 'object'
-            && 'value' in parsed) {
-          let value = parsed['value'];
-          return typeof value === 'undefined' ? null : value;
-        }
-        return parsed;
+        return typeof value === 'undefined' ? null : value;
       });
     });
   }
@@ -482,40 +477,45 @@ function tryParse(str) {
  *
  * @param {!cmd.Command} command The command the response is for.
  * @param {!Response} httpResponse The HTTP response to parse.
- * @param {boolean} w3c Whether the response should be processed using the
- *     W3C wire protocol.
- * @return {?} The parsed response.
+ * @return {{isW3C: boolean, value: ?}} An object describing the parsed
+ *     response. This object will have two fields: `isW3C` indicates whether
+ *     the response looks like it came from a remote end that conforms with the
+ *     W3C WebDriver spec, and `value`, the actual response value.
  * @throws {WebDriverError} If the HTTP response is an error.
  */
-function parseHttpResponse(command, httpResponse, w3c) {
-  let parsed = tryParse(httpResponse.body);
-  if (parsed !== undefined) {
-    if (httpResponse.status < 200) {
-      // This should never happen, but throw the raw response so
-      // users report it.
-      throw new error.WebDriverError(
-          `Unexpected HTTP response:\n${httpResponse}`);
-    }
+function parseHttpResponse(command, httpResponse) {
+  if (httpResponse.status < 200) {
+    // This should never happen, but throw the raw response so users report it.
+    throw new error.WebDriverError(
+        `Unexpected HTTP response:\n${httpResponse}`);
+  }
 
-    if (w3c) {
-      if (httpResponse.status > 399) {
-        error.throwDecodedError(parsed);
+  let parsed = tryParse(httpResponse.body);
+  if (parsed && typeof parsed === 'object') {
+    let value = parsed.value;
+    let isW3C =
+        value !== null && typeof value === 'object'
+            && typeof parsed.status === 'undefined';
+
+    if (!isW3C) {
+      error.checkLegacyResponse(parsed);
+
+      // Adjust legacy new session responses to look like W3C to simplify
+      // later processing.
+      if (command.getName() === cmd.Name.NEW_SESSION
+          || command.getName() == cmd.Name.DESCRIBE_SESSION) {
+        value = parsed;
       }
-      return parsed;
-    }
 
-    // If this is a new session command, we need to check for a W3C compliant
-    // error object. This is necessary since a successful new session command
-    // is what puts the executor into W3C mode.
-    if (httpResponse.status > 399
-        && (command.getName() == cmd.Name.NEW_SESSION
-            || command.getName() === cmd.Name.DESCRIBE_SESSION)
-        && error.isErrorResponse(parsed)) {
-      error.throwDecodedError(parsed);
+    } else if (httpResponse.status > 399) {
+      error.throwDecodedError(value);
     }
 
-    error.checkLegacyResponse(parsed);
-    return parsed;
+    return {isW3C, value};
+  }
+
+  if (parsed !== undefined) {
+    return {isW3C: false, value: parsed};
   }
 
   let value = httpResponse.body.replace(/\r\n/g, '\n');
@@ -528,7 +528,7 @@ function parseHttpResponse(command, httpResponse, w3c) {
     throw new error.WebDriverError(value);
   }
 
-  return value || null;
+  return {isW3C: false, value: value || null};
 }
 
 

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

@@ -50,6 +50,7 @@ var NATIVE_BROWSERS = [
 ];
 
 
+var noBuild = /^1|true$/i.test(process.env['SELENIUM_NO_BUILD']);
 var serverJar = process.env['SELENIUM_SERVER_JAR'];
 var remoteUrl = process.env['SELENIUM_REMOTE_URL'];
 var useLoopback = process.env['SELENIUM_USE_LOOP_BACK'] == '1';
@@ -225,7 +226,7 @@ function suite(fn, opt_options) {
   try {
 
     before(function() {
-      if (isDevMode) {
+      if (isDevMode && !noBuild) {
         return build.of(
             '//javascript/atoms/fragments:is-displayed',
             '//javascript/webdriver/atoms:getAttribute')

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

@@ -293,7 +293,14 @@ describe('http', function() {
         });
 
         it('auto-upgrades on W3C response', function() {
-          var rawResponse = {sessionId: 's123', value: {name: 'Bob'}};
+          let rawResponse = {
+            value: {
+              sessionId: 's123',
+              value: {
+                name: 'Bob'
+              }
+            }
+          };
 
           send.returns(Promise.resolve(
               new http.Response(200, {}, JSON.stringify(rawResponse))));
@@ -344,7 +351,8 @@ describe('http', function() {
         });
 
         it('handles w3c new session failures', function() {
-          let rawResponse = {error: 'no such element', message: 'oops'};
+          let rawResponse =
+              {value: {error: 'no such element', message: 'oops'}};
 
           send.returns(Promise.resolve(
               new http.Response(500, {}, JSON.stringify(rawResponse))));
@@ -429,7 +437,7 @@ describe('http', function() {
         });
 
         it('does not auto-upgrade on W3C response', function() {
-          var rawResponse = {sessionId: 's123', value: {name: 'Bob'}};
+          var rawResponse = {value: {sessionId: 's123', value: {name: 'Bob'}}};
 
           send.returns(Promise.resolve(
               new http.Response(200, {}, JSON.stringify(rawResponse))));