feat(server): remove socket reconnect support

client/karma.js

@@ -282,9 +282,6 @@ function Karma (updater, socket, iframe, opener, navigator, location, document)
     this.complete()
   }.bind(this))
 
-  // 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

client/main.js

@@ -12,8 +12,10 @@ var BROWSER_SOCKET_TIMEOUT = constants.BROWSER_SOCKET_TIMEOUT
 
 // Connect to the server using socket.io https://socket.io/
 var socket = io(location.host, {
-  reconnectionDelay: 500,
-  reconnectionDelayMax: Infinity,
+  // We can't support reconnection without message replay: any messages sent
+  // after disconnect are lost.
+  reconnection: false,
+  // At this timeout the client disconnects and the server sees 'transport close'
   timeout: BROWSER_SOCKET_TIMEOUT,
   path: KARMA_PROXY_PATH + KARMA_URL_ROOT.substr(1) + 'socket.io',
   'sync disconnect on unload': true

lib/browser.js

@@ -4,39 +4,36 @@ const BrowserResult = require('./browser_result')
 const helper = require('./helper')
 const logger = require('./logger')
 
-const CONNECTED = 'CONNECTED' // The browser is connected but not yet been commanded to execute tests.
+const CONNECTED = 'CONNECTED' // The browser is connected but not yet been commanded to execute tests, or finished testing.
 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 socket reconnecting).
-const DISCONNECTED = 'DISCONNECTED' // The browser got completely disconnected (e.g. browser crash) and can be only restored with a restart of execution.
+/* The browser got completely disconnected (e.g. browser crash) and can be only restored with a restart of execution. */
+const DISCONNECTED = 'DISCONNECTED'
 
 class Browser {
-  constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay,
-    noActivityTimeout, singleRun, clientConfig) {
+  constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, pingTimeout, singleRun, clientConfig) {
     this.id = id
     this.fullName = fullName
     this.name = helper.browserFullNameToShort(fullName)
     this.lastResult = new BrowserResult()
     this.disconnectsCount = 0
-    this.activeSockets = [socket]
-    this.noActivityTimeout = noActivityTimeout
     this.singleRun = singleRun
     this.clientConfig = clientConfig
     this.collection = collection
     this.emitter = emitter
     this.socket = socket
     this.timer = timer
     this.disconnectDelay = disconnectDelay
+    // Set onto the socket when it was created.
+    this.pingTimeout = pingTimeout
 
     this.log = logger.create(this.name)
 
-    this.noActivityTimeoutId = null
-    this.pendingDisconnect = null
     this.setState(CONNECTED)
   }
 
   init () {
-    this.log.info(`Connected on socket ${this.socket.id} with id ${this.id}`)
+    this.log.info(`Binding socket ${this.socket.id} to browser with id ${this.id}`)
 
     this.bindSocketEvents(this.socket)
     this.collection.add(this)
@@ -53,7 +50,6 @@ class Browser {
       this.lastResult.error = true
     }
     this.emitter.emit('browser_error', this, error)
-    this.refreshNoActivityTimeout()
   }
 
   onInfo (info) {
@@ -70,8 +66,6 @@ class Browser {
     } else if (!helper.isDefined(info.dump)) {
       this.emitter.emit('browser_info', this, info)
     }
-
-    this.refreshNoActivityTimeout()
   }
 
   onStart (info) {
@@ -82,78 +76,45 @@ class Browser {
     this.lastResult = new BrowserResult(info.total)
     this.setState(EXECUTING)
     this.emitter.emit('browser_start', this, info)
-    this.refreshNoActivityTimeout()
   }
 
   onComplete (result) {
-    if (this.isNotConnected()) {
-      this.setState(CONNECTED)
-      this.lastResult.totalTimeEnd()
-
-      this.emitter.emit('browsers_change', this.collection)
-      this.emitter.emit('browser_complete', this, result)
-
-      this.clearNoActivityTimeout()
+    if (this.state !== EXECUTING) {
+      this.log.warn(`Unexpected:'complete' event arrived while in state ${this.state}`)
     }
+    this.lastResult.totalTimeEnd()
+    this.setState(CONNECTED)
+    this.socket.disconnect(true)
   }
 
   onSocketDisconnect (reason, disconnectedSocket) {
-    helper.arrayRemove(this.activeSockets, disconnectedSocket)
-    if (this.activeSockets.length) {
-      this.log.debug(`Disconnected ${disconnectedSocket.id}, still have ${this.getActiveSocketsIds()}`)
+    if (this.state === DISCONNECTED) {
       return
     }
-
-    if (this.isConnected()) {
-      this.disconnect(`Client disconnected from CONNECTED state (${reason})`)
-    } else if ([CONFIGURING, EXECUTING].includes(this.state)) {
-      this.log.debug(`Disconnected during run, waiting ${this.disconnectDelay}ms for reconnecting.`)
-      this.setState(EXECUTING_DISCONNECTED)
-
-      this.pendingDisconnect = this.timer.setTimeout(() => {
-        this.lastResult.totalTimeEnd()
-        this.lastResult.disconnected = true
-        this.disconnect(`reconnect failed before timeout of ${this.disconnectDelay}ms (${reason})`)
-        this.emitter.emit('browser_complete', this)
-      }, this.disconnectDelay)
-
-      this.clearNoActivityTimeout()
-    }
-  }
-
-  reconnect (newSocket, clientSaysReconnect) {
-    if (!clientSaysReconnect || this.state === DISCONNECTED) {
-      this.log.info(`Disconnected browser returned on socket ${newSocket.id} with id ${this.id}.`)
-      this.setState(CONNECTED)
-
-      // The disconnected browser is already part of the collection.
-      // Update the collection view in the UI (header on client.html)
-      this.emitter.emit('browsers_change', this.collection)
-      // Notify the launcher
-      this.emitter.emit('browser_register', this)
-      // Execute tests if configured to do so.
-      if (this.singleRun) {
-        this.execute()
+    // Done with this socket
+    disconnectedSocket.disconnect()
+    disconnectedSocket.removeAllListeners()
+
+    if (this.state !== CONNECTED) {
+      // If we loaded the context or began testing, `disconnect` loses data.
+      this.emitter.emit('browser_error', this, `Disconnected from state ${this.state} with reason: ${reason || ''}`)
+      if (reason === 'ping timeout') {
+        this.log.info('Check for a test that crashes the browser or tab.')
+        this.log.info(`For large JS or tests that take longer than ${this.pingTimeout}ms, increase 'config.pingTimeout'`)
+      } else if (reason === 'transport close') {
+        this.log.info('The client may have disconnected.')
       }
-    } else if (this.state === EXECUTING_DISCONNECTED) {
-      this.log.debug('Lost socket connection, but browser continued to execute. Reconnected ' +
-        `on socket ${newSocket.id}.`)
-      this.setState(EXECUTING)
-    } else if ([CONNECTED, CONFIGURING, EXECUTING].includes(this.state)) {
-      this.log.debug(`Rebinding to new socket ${newSocket.id} (already have ` +
-        `${this.getActiveSocketsIds()})`)
-    }
-
-    if (!this.activeSockets.some((s) => s.id === newSocket.id)) {
-      this.activeSockets.push(newSocket)
-      this.bindSocketEvents(newSocket)
-    }
+      this.lastResult.totalTimeEnd()
+      this.lastResult.disconnected = true
 
-    if (this.pendingDisconnect) {
-      this.timer.clearTimeout(this.pendingDisconnect)
+      this.timer.setTimeout(() => {
+        // If we disconnect during a test the server can retry the browser
+        this.disconnectsCount++
+        this.finishDisconnect()
+      }, this.disconnectDelay)
+    } else {
+      this.finishDisconnect()
     }
-
-    this.refreshNoActivityTimeout()
   }
 
   onResult (result) {
@@ -163,23 +124,15 @@ class Browser {
       this.lastResult.add(result)
       this.emitter.emit('spec_complete', this, result)
     }
-    this.refreshNoActivityTimeout()
   }
 
   execute () {
-    this.activeSockets.forEach((socket) => socket.emit('execute', this.clientConfig))
+    this.socket.emit('execute', this.clientConfig)
     this.setState(CONFIGURING)
-    this.refreshNoActivityTimeout()
-  }
-
-  getActiveSocketsIds () {
-    return this.activeSockets.map((s) => s.id).join(', ')
   }
 
-  disconnect (reason) {
-    this.log.warn(`Disconnected (${this.disconnectsCount} times) ${reason || ''}`)
-    this.disconnectsCount++
-    this.emitter.emit('browser_error', this, `Disconnected ${reason || ''}`)
+  finishDisconnect () {
+    this.emitter.emit('browser_complete', this)
     this.remove()
   }
 
@@ -188,26 +141,6 @@ class Browser {
     this.collection.remove(this)
   }
 
-  refreshNoActivityTimeout () {
-    if (this.noActivityTimeout) {
-      this.clearNoActivityTimeout()
-
-      this.noActivityTimeoutId = this.timer.setTimeout(() => {
-        this.lastResult.totalTimeEnd()
-        this.lastResult.disconnected = true
-        this.disconnect(`, because no message in ${this.noActivityTimeout} ms.`)
-        this.emitter.emit('browser_complete', this)
-      }, this.noActivityTimeout)
-    }
-  }
-
-  clearNoActivityTimeout () {
-    if (this.noActivityTimeout && this.noActivityTimeoutId) {
-      this.timer.clearTimeout(this.noActivityTimeoutId)
-      this.noActivityTimeoutId = null
-    }
-  }
-
   bindSocketEvents (socket) {
     // TODO: check which of these events are actually emitted by socket
     socket.on('disconnect', (reason) => this.onSocketDisconnect(reason, socket))
@@ -246,7 +179,6 @@ class Browser {
       state: this.state,
       lastResult: this.lastResult,
       disconnectsCount: this.disconnectsCount,
-      noActivityTimeout: this.noActivityTimeout,
       disconnectDelay: this.disconnectDelay
     }
   }
@@ -255,17 +187,16 @@ class Browser {
 Browser.factory = function (
   id, fullName, /* capturedBrowsers */ collection, emitter, socket, timer,
   /* config.browserDisconnectTimeout */ disconnectDelay,
-  /* config.browserNoActivityTimeout */ noActivityTimeout,
+  /* config.pingTimeout */ pingTimeout,
   /* config.singleRun */ singleRun,
   /* config.client */ clientConfig) {
   return new Browser(id, fullName, collection, emitter, socket, timer,
-    disconnectDelay, noActivityTimeout, singleRun, clientConfig)
+    disconnectDelay, pingTimeout, singleRun, clientConfig)
 }
 
 Browser.STATE_CONNECTED = CONNECTED
 Browser.STATE_CONFIGURING = CONFIGURING
 Browser.STATE_EXECUTING = EXECUTING
-Browser.STATE_EXECUTING_DISCONNECTED = EXECUTING_DISCONNECTED
 Browser.STATE_DISCONNECTED = DISCONNECTED
 
 module.exports = Browser

lib/server.js

@@ -295,20 +295,19 @@ class Server extends KarmaEventEmitter {
         const knownBrowser = info.id ? (capturedBrowsers.getById(info.id) || singleRunBrowsers.getById(info.id)) : null
 
         if (knownBrowser) {
-          knownBrowser.reconnect(socket, info.isSocketReconnect)
-        } else {
-          const newBrowser = this._injector.createChild([{
-            id: ['value', info.id || null],
-            fullName: ['value', (helper.isDefined(info.displayName) ? info.displayName : info.name)],
-            socket: ['value', socket]
-          }]).invoke(Browser.factory)
-
-          newBrowser.init()
-
-          if (config.singleRun) {
-            newBrowser.execute()
-            singleRunBrowsers.add(newBrowser)
-          }
+          this.log.debug(`A previously captured browser has connected (${info.id} with reconnect ${info.isSocketReconnect})`)
+        }
+        const newBrowser = this._injector.createChild([{
+          id: ['value', info.id || null],
+          fullName: ['value', (helper.isDefined(info.displayName) ? info.displayName : info.name)],
+          socket: ['value', socket]
+        }]).invoke(Browser.factory)
+
+        newBrowser.init()
+
+        if (config.singleRun) {
+          newBrowser.execute()
+          singleRunBrowsers.add(newBrowser)
         }
 
         replySocketEvents()

static/karma.js

@@ -292,9 +292,6 @@ function Karma (updater, socket, iframe, opener, navigator, location, document)
     this.complete()
   }.bind(this))
 
-  // 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
@@ -334,8 +331,10 @@ var BROWSER_SOCKET_TIMEOUT = constants.BROWSER_SOCKET_TIMEOUT
 
 // Connect to the server using socket.io https://socket.io/
 var socket = io(location.host, {
-  reconnectionDelay: 500,
-  reconnectionDelayMax: Infinity,
+  // We can't support reconnection without message replay: any messages sent
+  // after disconnect are lost.
+  reconnection: false,
+  // At this timeout the client disconnects and the server sees 'transport close'
   timeout: BROWSER_SOCKET_TIMEOUT,
   path: KARMA_PROXY_PATH + KARMA_URL_ROOT.substr(1) + 'socket.io',
   'sync disconnect on unload': true