chore(refactor): Pull proxy logic out and add unit tests around it.

lib/blockingproxy.ts

@@ -244,7 +244,7 @@ export class BlockingProxy implements WebDriverBarrier {
       return;
     }
 
-    this.proxy.requestListener(originalRequest, response);
+    this.proxy.handleRequest(originalRequest, response);
   }
 
   onCommand(command: WebDriverCommand): Promise<void> {

lib/webdriver_commands.ts

@@ -2,7 +2,6 @@
  * Utilities for parsing WebDriver commands from HTTP Requests.
  */
 import * as events from 'events';
-import * as http from 'http';
 
 type HttpMethod = 'GET'|'POST'|'DELETE';
 export type paramKey = 'sessionId' | 'elementId' | 'name' | 'propertyName';

lib/webdriver_proxy.ts

@@ -4,9 +4,9 @@ import * as url from 'url';
 import {parseWebDriverCommand, WebDriverCommand} from './webdriver_commands';
 
 /**
- * A proxy that understands WebDriver commands. Users can add middleware (similar to middleware in
- * express) that will be called before
- * forwarding the request to WebDriver or forwarding the response to the client.
+ * A proxy that understands WebDriver commands. Users can add barriers * (similar to middleware in
+ * express) that will be called before forwarding the request to WebDriver. The proxy will wait for
+ * each barrier to finish, calling them in the order in which they were added.
  */
 export class WebDriverProxy {
   barriers: WebDriverBarrier[];
@@ -21,7 +21,7 @@ export class WebDriverProxy {
     this.barriers.push(barrier);
   }
 
-  requestListener(originalRequest: http.IncomingMessage, response: http.ServerResponse) {
+  async handleRequest(originalRequest: http.IncomingMessage, response: http.ServerResponse) {
     let command = parseWebDriverCommand(originalRequest.url, originalRequest.method);
 
     let replyWithError = (err) => {
@@ -30,47 +30,58 @@ export class WebDriverProxy {
       response.end();
     };
 
-    // TODO: What happens when barriers error? return a client error?
-    let barrierPromises = this.barriers.map((b) => b.onCommand(command));
+    // Process barriers in order, one at a time.
+    try {
+      for (let barrier of this.barriers) {
+        await barrier.onCommand(command);
+      }
+    } catch (err) {
+      replyWithError(err);
+      // Don't call through if a barrier fails.
+      return;
+    }
 
-    Promise.all(barrierPromises).then(() => {
-      let parsedUrl = url.parse(this.seleniumAddress);
-      let options: http.RequestOptions = {};
-      options.method = originalRequest.method;
-      options.path = parsedUrl.path + originalRequest.url;
-      options.hostname = parsedUrl.hostname;
-      options.port = parseInt(parsedUrl.port);
-      options.headers = originalRequest.headers;
+    let parsedUrl = url.parse(this.seleniumAddress);
+    let options: http.RequestOptions = {};
+    options.method = originalRequest.method;
+    options.path = parsedUrl.path + originalRequest.url;
+    options.hostname = parsedUrl.hostname;
+    options.port = parseInt(parsedUrl.port);
+    options.headers = originalRequest.headers;
 
-      let forwardedRequest = http.request(options);
+    let forwardedRequest = http.request(options);
 
-      // clang-format off
-      let reqData = '';
-      originalRequest.on('data', (d) => {
-        reqData += d;
-        forwardedRequest.write(d);
-      }).on('end', () => {
-        command.handleData(reqData);
-        forwardedRequest.end();
-      }).on('error', replyWithError);
-
-      forwardedRequest.on('response', (seleniumResponse) => {
-        response.writeHead(seleniumResponse.statusCode, seleniumResponse.headers);
+    // clang-format off
+    let reqData = '';
+    originalRequest.on('data', (d) => {
+      reqData += d;
+      forwardedRequest.write(d);
+    }).on('end', () => {
+      command.handleData(reqData);
+      forwardedRequest.end();
+    }).on('error', replyWithError);
 
-        let respData = '';
-        seleniumResponse.on('data', (d) => {
-          respData += d;
-          response.write(d);
-        }).on('end', () => {
-          command.handleResponse(seleniumResponse.statusCode, respData);
-          response.end();
-        }).on('error', replyWithError);
+    forwardedRequest.on('response', (seleniumResponse) => {
+      response.writeHead(seleniumResponse.statusCode, seleniumResponse.headers);
 
+      let respData = '';
+      seleniumResponse.on('data', (d) => {
+        respData += d;
+        response.write(d);
+      }).on('end', () => {
+        command.handleResponse(seleniumResponse.statusCode, respData);
+        response.end();
       }).on('error', replyWithError);
-      // clang-format on
 
-    }, replyWithError);
+    }).on('error', replyWithError);
+    // clang-format on
   }
 }
 
-export interface WebDriverBarrier { onCommand(command: WebDriverCommand): Promise<void>; }
+/**
+ * When the proxy receives a WebDriver command, it will call onCommand() for each of it's barriers.
+ * Barriers may return a promise for the proxy to wait for before proceeding. If the promise is
+ * rejected, the proxy will reply with an error code and the result of the promise and the command
+ * will not be forwarded to Selenium.
+ */
+export interface WebDriverBarrier { onCommand(command: WebDriverCommand): Promise<void>; }

package.json

@@ -38,7 +38,7 @@
     "selenium-mock": "^0.1.5",
     "selenium-webdriver": "2.53.3",
     "ts-node": "^2.0.0",
-    "tslint": "^4.0.2",
+    "tslint": "^4.3.1",
     "tslint-eslint-rules": "^3.1.0",
     "typescript": "^2.0.3",
     "vrsource-tslint-rules": "^0.14.1",

spec/unit/proxy_spec.ts

@@ -5,10 +5,4 @@ describe('BlockingProxy', () => {
     let proxy = new BlockingProxy(8111);
     expect(proxy.waitEnabled).toBe(true);
   });
-
-  it('should provide hooks when relaying commands',
-     () => {
-
-
-     });
 });

spec/unit/webdriver_commands_spec.ts

@@ -22,7 +22,7 @@ describe('WebDriver command parser', () => {
     proxy = new WebDriverProxy(`http://localhost:${mockPort}/wd/hub`);
     testBarrier = new TestBarrier;
     proxy.addBarrier(testBarrier);
-    server = http.createServer(proxy.requestListener.bind(proxy));
+    server = http.createServer(proxy.handleRequest.bind(proxy));
     server.listen(0);
     let port = server.address().port;
 

spec/unit/webdriver_proxy_spec.ts

@@ -1,6 +1,6 @@
 import * as nock from 'nock';
 
-import {CommandName, WebDriverCommand} from '../../lib/webdriver_commands';
+import {WebDriverCommand} from '../../lib/webdriver_commands';
 import {WebDriverProxy} from '../../lib/webdriver_proxy';
 
 import {InMemoryReader, InMemoryWriter, TestBarrier} from './util';
@@ -9,7 +9,7 @@ describe('WebDriver Proxy', () => {
   let proxy: WebDriverProxy;
 
   beforeEach(() => {
-    proxy = new WebDriverProxy(`http://localhost:4444/wd/hub`);
+    proxy = new WebDriverProxy('http://test_webdriver_url/wd/hub');
   });
 
   it('proxies to WebDriver', (done) => {
@@ -22,13 +22,13 @@ describe('WebDriver Proxy', () => {
 
     let scope = nock(proxy.seleniumAddress).get('/session/sessionId/get').reply(200, responseData);
 
-    proxy.requestListener(req, resp);
+    proxy.handleRequest(req, resp);
 
     resp.onEnd((data) => {
       // Verify that all nock endpoints were called.
       expect(resp.writeHead.calls.first().args[0]).toBe(200);
       expect(data).toEqual(JSON.stringify(responseData));
-      scope.isDone();
+      scope.done();
       done();
     });
   });
@@ -60,11 +60,88 @@ describe('WebDriver Proxy', () => {
     });
 
     proxy.addBarrier(barrier);
-    proxy.requestListener(req, resp);
+    proxy.handleRequest(req, resp);
 
     resp.onEnd(() => {
       expect(barrierDone).toBeTruthy();
-      scope.isDone();
+      scope.done();
+      done();
+    });
+  });
+
+  it('waits for multiple barriers in order', (done) => {
+    const WD_URL = '/session/sessionId/url';
+
+    let req = new InMemoryReader() as any;
+    let resp = new InMemoryWriter() as any;
+    resp.writeHead = jasmine.createSpy('spy');
+    req.url = WD_URL;
+    req.method = 'POST';
+
+    let barrier1 = new TestBarrier();
+    let barrier1Done = false;
+    barrier1.onCommand = (): Promise<void> => {
+      return new Promise<void>((res) => {
+        setTimeout(() => {
+          expect(barrier2Done).toBeFalsy();
+          barrier1Done = true;
+          res();
+        }, 150);
+      });
+    };
+    let barrier2 = new TestBarrier();
+    let barrier2Done = false;
+    barrier2.onCommand = (): Promise<void> => {
+      return new Promise<void>((res) => {
+        setTimeout(() => {
+          expect(barrier1Done).toBeTruthy();
+          barrier2Done = true;
+          res();
+        }, 50);
+      });
+    };
+
+    let scope = nock(proxy.seleniumAddress).post(WD_URL).reply(200);
+
+    proxy.addBarrier(barrier1);
+    proxy.addBarrier(barrier2);
+    proxy.handleRequest(req, resp);
+
+    resp.onEnd(() => {
+      expect(barrier2Done).toBeTruthy();
+      scope.done();
+      done();
+    });
+  });
+
+  it('returns an error if a barrier fails', (done) => {
+    const WD_URL = '/session/sessionId/url';
+
+    let req = new InMemoryReader() as any;
+    let resp = new InMemoryWriter() as any;
+    resp.writeHead = jasmine.createSpy('spy');
+    req.url = WD_URL;
+    req.method = 'GET';
+
+    let barrier = new TestBarrier();
+    barrier.onCommand = (): Promise<void> => {
+      return new Promise<void>((res, rej) => {
+        rej('Barrier failed');
+      });
+    };
+
+    let scope = nock(proxy.seleniumAddress).get(WD_URL).reply(200);
+
+    proxy.addBarrier(barrier);
+    proxy.handleRequest(req, resp);
+
+    resp.onEnd((respData) => {
+      expect(resp.writeHead.calls.first().args[0]).toBe(500);
+      expect(respData).toEqual('Barrier failed');
+
+      // Should not call the selenium server.
+      expect(scope.isDone()).toBeFalsy();
+      nock.cleanAll();
       done();
     });
   });
@@ -85,13 +162,13 @@ describe('WebDriver Proxy', () => {
     barrier.onCommand = (command: WebDriverCommand): Promise<void> => {
       command.on('response', () => {
         expect(command.responseData['url']).toEqual(RESPONSE.url);
-        scope.isDone();
+        scope.done();
         done();
       });
       return undefined;
     };
     proxy.addBarrier(barrier);
-    proxy.requestListener(req, resp);
+    proxy.handleRequest(req, resp);
   });
 
   it('propagates http errors', (done) => {
@@ -106,13 +183,13 @@ describe('WebDriver Proxy', () => {
 
     let scope = nock(proxy.seleniumAddress).post(WD_URL).replyWithError(ERR);
 
-    proxy.requestListener(req, resp);
+    proxy.handleRequest(req, resp);
 
     resp.onEnd((data) => {
       expect(resp.writeHead.calls.first().args[0]).toBe(500);
       expect(data).toEqual(ERR.toString());
-      scope.isDone();
+      scope.done();
       done();
     });
   });
-});
+});

tsconfig.json

@@ -3,6 +3,7 @@
     "target": "es6",
     "module": "commonjs",
     "moduleResolution": "node",
+    "noUnusedLocals": true,
     "sourceMap": true,
     "declaration": true,
     "removeComments": false,