fix(server): check available port before start server (fix #1476, fix #…

…3011)
karma-runner · Jun 15, 2018 · a19b8d4 · a19b8d4
1 parent 05dd09a
commit a19b8d4
Show file tree

Hide file tree

Showing 4 changed files with 153 additions and 86 deletions.
diff --git a/lib/server.js b/lib/server.js
@@ -9,6 +9,7 @@ const tmp = require('tmp')
 const fs = require('fs')
 const path = require('path')
 const BundleUtils = require('./utils/bundle-utils')
+const NetUtils = require('./utils/net-utils')
 const root = global || window || this
 
 const cfg = require('./config')
@@ -97,8 +98,24 @@ class Server extends KarmaEventEmitter {
     this._injector = new di.Injector(modules)
   }
 
+  dieOnError (error) {
+    this.log.error(error)
+    process.exitCode = 1
+    process.kill(process.pid, 'SIGINT')
+  }
+
   start () {
-    this._injector.invoke(this._start, this)
+    const config = this.get('config')
+    return Promise.all([
+      BundleUtils.bundleResourceIfNotExist('client/main.js', 'static/karma.js'),
+      BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js')
+    ])
+      .then(() => NetUtils.getAvailablePort(config.port, config.listenAddress))
+      .then((port) => {
+        config.port = port
+        this._injector.invoke(this._start, this)
+      })
+      .catch(this.dieOnError.bind(this))
   }
 
   get (token) {
@@ -126,47 +143,26 @@ class Server extends KarmaEventEmitter {
     const singleRunBrowsers = new BrowserCollection(new EventEmitter())
     let singleRunBrowserNotCaptured = false
 
-    webServer.on('error', (e) => {
-      if (e.code === 'EADDRINUSE') {
-        this.log.warn('Port %d in use', config.port)
-        config.port++
-        webServer.listen(config.port, config.listenAddress)
-      } else {
-        throw e
-      }
-    })
+    webServer.on('error', this.dieOnError.bind(this))
 
     const afterPreprocess = () => {
       if (config.autoWatch) {
         this._injector.invoke(watcher.watch)
       }
 
-      return Promise.all([
-        BundleUtils.bundleResourceIfNotExist('client/main.js', 'static/karma.js'),
-        BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js')
-      ])
-        .then(() => {
-          webServer.listen(config.port, config.listenAddress, () => {
-            this.log.info(`Karma v${constant.VERSION} server started at ${config.protocol}//${config.listenAddress}:${config.port}${config.urlRoot}`)
-
-            this.emit('listening', config.port)
-            if (config.browsers && config.browsers.length) {
-              this._injector.invoke(launcher.launch, launcher).forEach((browserLauncher) => {
-                singleRunDoneBrowsers[browserLauncher.id] = false
-              })
-            }
-            if (this.loadErrors.length > 0) {
-              this.log.error('Found %d load error%s', this.loadErrors.length, this.loadErrors.length === 1 ? '' : 's')
-              process.exitCode = 1
-              process.kill(process.pid, 'SIGINT')
-            }
+      webServer.listen(config.port, config.listenAddress, () => {
+        this.log.info(`Karma v${constant.VERSION} server started at ${config.protocol}//${config.listenAddress}:${config.port}${config.urlRoot}`)
+
+        this.emit('listening', config.port)
+        if (config.browsers && config.browsers.length) {
+          this._injector.invoke(launcher.launch, launcher).forEach((browserLauncher) => {
+            singleRunDoneBrowsers[browserLauncher.id] = false
           })
-        })
-        .catch((error) => {
-          this.log.error('Front-end script compile failed with error: ' + error)
-          process.exitCode = 1
-          process.kill(process.pid, 'SIGINT')
-        })
+        }
+        if (this.loadErrors.length > 0) {
+          this.dieOnError(new Error(`Found ${this.loadErrors.length} load error${this.loadErrors.length === 1 ? '' : 's'}`))
+        }
+      })
     }
 
     fileList.refresh().then(afterPreprocess, afterPreprocess)

diff --git a/lib/utils/net-utils.js b/lib/utils/net-utils.js
@@ -0,0 +1,33 @@
+'use strict'
+
+const Promise = require('bluebird')
+const net = require('net')
+
+const NetUtils = {
+  isPortAvailable (port, listenAddress) {
+    return new Promise((resolve, reject) => {
+      const server = net.createServer()
+
+      server.unref()
+      server.on('error', (err) => {
+        server.close()
+        if (err.code === 'EADDRINUSE' || err.code === 'EACCES') {
+          resolve(false)
+        } else {
+          reject(err)
+        }
+      })
+
+      server.listen(port, listenAddress, () => {
+        server.close(() => resolve(true))
+      })
+    })
+  },
+
+  getAvailablePort (port, listenAddress) {
+    return NetUtils.isPortAvailable(port, listenAddress)
+      .then((available) => available ? port : NetUtils.getAvailablePort(port + 1, listenAddress))
+  }
+}
+
+module.exports = NetUtils
diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js
@@ -1,5 +1,6 @@
 var Server = require('../../lib/server')
 var BundleUtils = require('../../lib/utils/bundle-utils')
+var NetUtils = require('../../lib/utils/net-utils')
 var BrowserCollection = require('../../lib/browser_collection')
 
 describe('server', () => {
@@ -105,106 +106,93 @@ describe('server', () => {
     webServerOnError = null
   })
 
-  // ============================================================================
-  // server._start()
-  // ============================================================================
-  describe('_start', () => {
-    it('should compile static resources', (done) => {
+  describe('start', () => {
+    beforeEach(() => {
       sinon.spy(BundleUtils, 'bundleResourceIfNotExist')
+      sinon.stub(NetUtils, 'getAvailablePort').resolves(9876)
+      sinon.stub(server, 'get').withArgs('config').returns({ port: 9876, listenAddress: '127.0.0.1' })
+    })
 
-      server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
-
-      fileListOnResolve().then(() => {
+    it('should compile static resources', (done) => {
+      server.start().then(() => {
         expect(BundleUtils.bundleResourceIfNotExist).to.have.been.calledWith('client/main.js', 'static/karma.js')
         expect(BundleUtils.bundleResourceIfNotExist).to.have.been.calledWith('context/main.js', 'static/context.js')
         done()
       })
     })
 
-    it('should start the web server after all files have been preprocessed successfully', (done) => {
+    it('should search for available port', (done) => {
+      server.start().then(() => {
+        expect(NetUtils.getAvailablePort).to.have.been.calledWith(9876, '127.0.0.1')
+        done()
+      })
+    })
+  })
+
+  // ============================================================================
+  // server._start()
+  // ============================================================================
+  describe('_start', () => {
+    it('should start the web server after all files have been preprocessed successfully', () => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       expect(mockFileList.refresh).to.have.been.called
       expect(fileListOnResolve).not.to.be.null
       expect(mockWebServer.listen).not.to.have.been.called
       expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
 
-      fileListOnResolve().then(() => {
-        expect(mockWebServer.listen).to.have.been.called
-        expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
-        done()
-      })
+      fileListOnResolve()
+      expect(mockWebServer.listen).to.have.been.called
+      expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
     })
 
-    it('should start the web server after all files have been preprocessed with an error', (done) => {
+    it('should start the web server after all files have been preprocessed with an error', () => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       expect(mockFileList.refresh).to.have.been.called
       expect(fileListOnReject).not.to.be.null
       expect(mockWebServer.listen).not.to.have.been.called
       expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
 
-      fileListOnReject().then(() => {
-        expect(mockWebServer.listen).to.have.been.called
-        expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
-        done()
-      })
+      fileListOnReject()
+      expect(mockWebServer.listen).to.have.been.called
+      expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
     })
 
-    it('should launch browsers after the web server has started', (done) => {
+    it('should launch browsers after the web server has started', () => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       expect(mockWebServer.listen).not.to.have.been.called
       expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
 
-      fileListOnResolve().then(() => {
-        expect(mockWebServer.listen).to.have.been.called
-        expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
-        done()
-      })
+      fileListOnResolve()
+      expect(mockWebServer.listen).to.have.been.called
+      expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
     })
 
-    it('should listen on the listenAddress in the config', (done) => {
+    it('should listen on the listenAddress in the config', () => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       expect(mockWebServer.listen).not.to.have.been.called
       expect(webServerOnError).not.to.be.null
 
       expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
 
-      fileListOnResolve().then(() => {
-        expect(mockWebServer.listen).to.have.been.calledWith(9876, '127.0.0.1')
-        expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
-        done()
-      })
-    })
-
-    it('should try next port if already in use', () => {
-      server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
-
-      expect(mockWebServer.listen).not.to.have.been.called
-      expect(webServerOnError).not.to.be.null
-
-      expect(mockConfig.port).to.be.equal(9876)
-
-      webServerOnError({code: 'EADDRINUSE'})
-
-      expect(mockWebServer.listen).to.have.been.calledWith(9877)
-      expect(mockConfig.port).to.be.equal(9877)
+      fileListOnResolve()
+      expect(mockWebServer.listen).to.have.been.calledWith(9876, '127.0.0.1')
+      expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
     })
 
-    it('should emit a listening event once server begin accepting connections', (done) => {
+    it('should emit a listening event once server begin accepting connections', () => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       var listening = sinon.spy()
       server.on('listening', listening)
 
       expect(listening).not.to.have.been.called
 
-      fileListOnResolve().then(() => {
-        expect(listening).to.have.been.calledWith(9876)
-        done()
-      })
+      fileListOnResolve()
+      expect(listening).to.have.been.calledWith(9876)
     })
 
     it('should emit a browsers_ready event once all the browsers are captured', () => {

diff --git a/test/unit/utils/net-utils.spec.js b/test/unit/utils/net-utils.spec.js
@@ -0,0 +1,50 @@
+'use strict'
+
+const NetUtils = require('../../../lib/utils/net-utils')
+const connect = require('connect')
+const net = require('net')
+
+describe('NetUtils.isPortAvailable', () => {
+  it('it is possible to run server on available port', (done) => {
+    NetUtils.isPortAvailable(9876, '127.0.0.1').then((available) => {
+      expect(available).to.be.true
+      const server = net
+        .createServer(connect())
+        .listen(9876, '127.0.0.1', () => {
+          server.close(done)
+        })
+    })
+  })
+
+  it('resolves with false when port is used', (done) => {
+    const server = net
+      .createServer(connect())
+      .listen(9876, '127.0.0.1', () => {
+        NetUtils.isPortAvailable(9876, '127.0.0.1').then((available) => {
+          expect(available).to.be.false
+          server.close(done)
+        })
+      })
+  })
+})
+
+describe('NetUtils.getAvailablePort', () => {
+  it('resolves with port when is available', (done) => {
+    NetUtils.getAvailablePort(9876, '127.0.0.1').then((port) => {
+      expect(port).to.equal(9876)
+      done()
+    })
+  })
+
+  it('resolves with next available port', (done) => {
+    const stub = sinon.stub(NetUtils, 'isPortAvailable')
+    stub.withArgs(9876).resolves(false)
+    stub.withArgs(9877).resolves(false)
+    stub.withArgs(9878).resolves(true)
+
+    NetUtils.getAvailablePort(9876, '127.0.0.1').then((port) => {
+      expect(port).to.equal(9878)
+      done()
+    })
+  })
+})