fix: restarted browsers not running tests

Currently whenever a browser disconnects completely (no socket.io connection loss), the launcher is instructed to "restart" the browser. Whenever the restarted browser now tries to "register" again, Karma considers the browser instance to be still executing and doesn't do anything about it (except setting the state to `EXECUTING` again).

This means that the browser is in the state of executing, but
practically it does nothing just waits. Resulting another disconnect
(repeat here).

client/karma.js

@@ -5,6 +5,7 @@ var util = require('../common/util')
 function Karma (socket, iframe, opener, navigator, location) {
   var startEmitted = false
   var reloadingContext = false
+  var socketReconnect = false
   var self = this
   var queryParams = util.parseQueryParams(location.search)
   var browserId = queryParams.id || util.generateId('manual-')
@@ -227,20 +228,27 @@ function Karma (socket, iframe, opener, navigator, location) {
     this.complete()
   }.bind(this))
 
-  // report browser name, id
+  // Report the browser name and Id. Note that this event can also fire if the connection has
+  // been temporarily lost, but the socket reconnected automatically. Read more in the docs:
+  // https://socket.io/docs/client-api/#Event-%E2%80%98connect%E2%80%99
   socket.on('connect', function () {
     socket.io.engine.on('upgrade', function () {
       resultsBufferLimit = 1
     })
     var info = {
       name: navigator.userAgent,
-      id: browserId
+      id: browserId,
+      isSocketReconnect: socketReconnect
     }
     if (displayName) {
       info.displayName = displayName
     }
     socket.emit('register', info)
   })
+
+  socket.on('reconnect', function () {
+    socketReconnect = true
+  })
 }
 
 module.exports = Karma

lib/browser.js

@@ -7,8 +7,8 @@ const logger = require('./logger')
 const CONNECTED = 'CONNECTED' // The browser is connected but not yet been commanded to execute tests.
 const CONFIGURING = 'CONFIGURING' // The browser has been told to execute tests; it is configuring before tests execution.
 const EXECUTING = 'EXECUTING' // The browser is executing the tests.
-const EXECUTING_DISCONNECTED = 'EXECUTING_DISCONNECTED' // The browser is executing the tests, but temporarily disconnect (waiting for reconnecting).
-const DISCONNECTED = 'DISCONNECTED' // The browser got permanently disconnected (being removed from the collection and destroyed).
+const EXECUTING_DISCONNECTED = 'EXECUTING_DISCONNECTED' // The browser is executing the tests, but temporarily disconnect (waiting for socket reconnecting).
+const DISCONNECTED = 'DISCONNECTED' // The browser got completely disconnected (e.g. browser crash) and can be only restored with a restart of execution.
 
 class Browser {
   constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout) {
@@ -121,15 +121,26 @@ class Browser {
 
   reconnect (newSocket) {
     if (this.state === EXECUTING_DISCONNECTED) {
+      // In case the socket just lost connection and the browser continued the execution,
+      // we just update the state back to "EXECUTING" so that it matches the current state.
       this.log.debug(`Reconnected on ${newSocket.id}.`)
       this.setState(EXECUTING)
     } else if ([CONNECTED, CONFIGURING, EXECUTING].includes(this.state)) {
+      // In case for some reason the socket has changed, we just bind the events of the
+      // new socket (see below)
       this.log.debug(`New connection ${newSocket.id} (already have ${this.getActiveSocketsIds()})`)
     } else if (this.state === DISCONNECTED) {
-      this.log.info(`Connected on socket ${newSocket.id} with id ${this.id}`)
+      // In case the browser disconnected completely, we want to ensure that the browser will be
+      // registered and starts execution again. This can happen for example if the browser
+      // instance crashed and cannot continue from it's own execution state.
+      this.log.info(`Reconnected on socket ${newSocket.id} with id ${this.id}`)
       this.setState(CONNECTED)
 
-      this.collection.add(this)
+      // Since the disconnected browser is already part of the collection and we want to
+      // make sure that the server can properly handle the browser like it's the first time
+      // connecting this browser (as we want a complete new execution), we need to emit the
+      // following events:
+      this.emitter.emit('browsers_change', this.collection)
       this.emitter.emit('browser_register', this)
     }
 

lib/server.js

@@ -232,10 +232,20 @@ class Server extends KarmaEventEmitter {
         let newBrowser = info.id ? (capturedBrowsers.getById(info.id) || singleRunBrowsers.getById(info.id)) : null
 
         if (newBrowser) {
+          // By default if a browser disconnects while still executing, we assume that the test
+          // execution still continues because just the socket connection has been terminated. Now
+          // since we know whether this is just a socket reconnect or full client reconnect, we
+          // need to update the browser state accordingly. This is necessary because in case a
+          // browser crashed and has been restarted, we need to start with a fresh execution.
+          if (!info.isSocketReconnect) {
+            newBrowser.setState(Browser.STATE_DISCONNECTED)
+          }
+
           newBrowser.reconnect(socket)
 
-          // We are restarting a previously disconnected browser.
-          if (newBrowser.state === Browser.STATE_DISCONNECTED && config.singleRun) {
+          // In case we are in single run and the reconnected browser was not able to restore
+          // or continue with its previous execution, we start the execution again.
+          if (newBrowser.state === Browser.STATE_CONNECTED && config.singleRun) {
             newBrowser.execute(config.client)
           }
         } else {

test/client/karma.spec.js

@@ -147,6 +147,15 @@ describe('Karma', function () {
     assert(spyInfo.called)
   })
 
+  it.only('should mark "register" event for reconnected socket', function () {
+    socket.on('register', sinon.spy(function (info) {
+      assert(info.isSocketReconnect === true)
+    }))
+
+    socket.emit('reconnect')
+    socket.emit('connect')
+  })
+
   it('should report browser id', function () {
     windowLocation.search = '?id=567'
     socket = new MockSocket()

test/unit/browser.spec.js

@@ -297,6 +297,19 @@ describe('Browser', () => {
 
       expect(browser.isConnected()).to.equal(true)
     })
+
+    it('should not add a disconnected browser to the collection multiple times', () => {
+      browser = new Browser('id', 'Chrome 25.0', collection, emitter, socket, null, 10)
+      browser.init()
+
+      expect(collection.length).to.equal(1)
+
+      browser.state = Browser.STATE_DISCONNECTED
+
+      browser.reconnect(mkSocket())
+
+      expect(collection.length).to.equal(1)
+    })
   })
 
   describe('onResult', () => {

yarn.lock

@@ -2795,6 +2795,11 @@ flat-cache@^1.2.1:
     graceful-fs "^4.1.2"
     write "^0.2.1"
 
+flatted@^2.0.0:
+  version "2.0.0"
+  resolved "https://registry.yarnpkg.com/flatted/-/flatted-2.0.0.tgz#55122b6536ea496b4b44893ee2608141d10d9916"
+  integrity sha512-R+H8IZclI8AAkSBRQJLVOsxwAoHd6WC40b4QTNWIjzAa6BXOBfQcM587MXDTVPeYaopFNWHUFLx7eNmHDSxMWg==
+
 for-in@^1.0.1, for-in@^1.0.2:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/for-in/-/for-in-1.0.2.tgz#81068d295a8142ec0ac726c6e2200c30fb6d5e80"
@@ -4216,11 +4221,6 @@ json-stringify-safe@^5.0.1, json-stringify-safe@~5.0.1:
   resolved "https://registry.yarnpkg.com/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz#1296a2d58fd45f19a0f6ce01d65701e2c735b6eb"
   integrity sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=
 
-json3@^3.3.2:
-  version "3.3.2"
-  resolved "https://registry.yarnpkg.com/json3/-/json3-3.3.2.tgz#3c0434743df93e2f5c42aee7b19bcb483575f4e1"
-  integrity sha1-PAQ0dD35Pi9cQq7nsZvLSDV19OE=
-
 jsonfile@^3.0.0:
   version "3.0.1"
   resolved "https://registry.yarnpkg.com/jsonfile/-/jsonfile-3.0.1.tgz#a5ecc6f65f53f662c4415c7675a0331d0992ec66"
@@ -4583,6 +4583,11 @@ lodash@^4.0.0, lodash@^4.14.0, lodash@^4.16.6, lodash@^4.17.2, lodash@^4.17.4, l
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.4.tgz#78203a4d1c328ae1d86dca6460e369b57f4055ae"
   integrity sha1-eCA6TRwyiuHYbcpkYONptX9AVa4=
 
+lodash@^4.17.5:
+  version "4.17.11"
+  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.11.tgz#b39ea6229ef607ecd89e2c8df12536891cac9b8d"
+  integrity sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg==
+
 lodash@~4.3.0:
   version "4.3.0"
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.3.0.tgz#efd9c4a6ec53f3b05412429915c3e4824e4d25a4"
@@ -4641,10 +4646,13 @@ lower-case@^1.1.1:
   resolved "https://registry.yarnpkg.com/lower-case/-/lower-case-1.1.4.tgz#9a2cabd1b9e8e0ae993a4bf7d5875c39c42e8eac"
   integrity sha1-miyr0bno4K6ZOkv31YdcOcQujqw=
 
-lru-cache@2.2.x:
-  version "2.2.4"
-  resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-2.2.4.tgz#6c658619becf14031d0d0b594b16042ce4dc063d"
-  integrity sha1-bGWGGb7PFAMdDQtZSxYELOTcBj0=
+lru-cache@4.1.x:
+  version "4.1.5"
+  resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-4.1.5.tgz#8bbe50ea85bed59bc9e33dcab8235ee9bcf443cd"
+  integrity sha512-sWZlbEP2OsHNkXrMl5GYk/jKk70MBng6UU4YI/qGDYbgf6YbP4EvmqISbXCoJiRKs+1bSpFHVgQxvJ17F2li5g==
+  dependencies:
+    pseudomap "^1.0.2"
+    yallist "^2.1.2"
 
 lru-cache@^4.0.1:
   version "4.1.1"
@@ -6277,10 +6285,10 @@ signal-exit@^3.0.0, signal-exit@^3.0.2:
   resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.2.tgz#b5fdc08f1287ea1178628e415e25132b73646c6d"
   integrity sha1-tf3AjxKH6hF4Yo5BXiUTK3NkbG0=
 
-sinon-chai@^2.7.0:
-  version "2.14.0"
-  resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-2.14.0.tgz#da7dd4cc83cd6a260b67cca0f7a9fdae26a1205d"
-  integrity sha512-9stIF1utB0ywNHNT7RgiXbdmen8QDCRsrTjw+G9TgKt1Yexjiv8TOWZ6WHsTPz57Yky3DIswZvEqX8fpuHNDtQ==
+sinon-chai@^3.0.0:
+  version "3.3.0"
+  resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-3.3.0.tgz#8084ff99451064910fbe2c2cb8ab540c00b740ea"
+  integrity sha512-r2JhDY7gbbmh5z3Q62pNbrjxZdOAjpsqW/8yxAZRSqLZqowmfGZPGUZPFf3UX36NLis0cv8VEM5IJh9HgkSOAA==
 
 sinon@^6.1.5:
   version "6.3.5"
@@ -7149,12 +7157,12 @@ use@^3.1.0:
   dependencies:
     kind-of "^6.0.2"
 
-useragent@2.2.1:
-  version "2.2.1"
-  resolved "https://registry.yarnpkg.com/useragent/-/useragent-2.2.1.tgz#cf593ef4f2d175875e8bb658ea92e18a4fd06d8e"
-  integrity sha1-z1k+9PLRdYdei7ZY6pLhik/QbY4=
+useragent@2.3.0:
+  version "2.3.0"
+  resolved "https://registry.yarnpkg.com/useragent/-/useragent-2.3.0.tgz#217f943ad540cb2128658ab23fc960f6a88c9972"
+  integrity sha512-4AoH4pxuSvHCjqLO04sU6U/uE65BYza8l/KKBS0b0hnUPWi+cQ2BpeTEwejCSx9SPV5/U03nniDTrWx5NrmKdw==
   dependencies:
-    lru-cache "2.2.x"
+    lru-cache "4.1.x"
     tmp "0.0.x"
 
 util-arity@^1.0.2: