Skip to content

Commit

Permalink
fix(server): pass bound port to preventEADDRINUSE issue. (#3065)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sergei-startsev @lusarz
sergei-startsev authored and lusarz committed Jul 11, 2018
1 parent ad820a1 commit 850a90b
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 78 deletions.
9 changes: 5 additions & 4 deletions lib/server.js
View file
Expand Up @@ -116,9 +116,10 @@ class Server extends KarmaEventEmitter {
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
.then(() => NetUtils.bindAvailablePort(config.port, config.listenAddress))
.then((boundServer) => {
config.port = boundServer.address().port
this._boundServer = boundServer
this._injector.invoke(this._start, this)
})
.catch(this.dieOnError.bind(this))
Expand Down Expand Up @@ -156,7 +157,7 @@ class Server extends KarmaEventEmitter {
this._injector.invoke(watcher.watch)
}

webServer.listen(config.port, config.listenAddress, () => {
webServer.listen(this._boundServer, () => {
this.log.info(`Karma v${constant.VERSION} server started at ${config.protocol}//${config.listenAddress}:${config.port}${config.urlRoot}`)

this.emit('listening', config.port)
Expand Down
33 changes: 14 additions & 19 deletions lib/utils/net-utils.js
View file
Expand Up @@ -4,29 +4,24 @@ const Promise = require('bluebird')
const net = require('net')

const NetUtils = {
isPortAvailable (port, listenAddress) {
bindAvailablePort (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))
})
server
.on('error', (err) => {
server.close()
if (err.code === 'EADDRINUSE' || err.code === 'EACCES') {
server.listen(++port, listenAddress)
} else {
reject(err)
}
})
.on('listening', () => {
resolve(server)
})
.listen(port, listenAddress)
})
},

getAvailablePort (port, listenAddress) {
return NetUtils.isPortAvailable(port, listenAddress)
.then((available) => available ? port : NetUtils.getAvailablePort(port + 1, listenAddress))
}
}

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Expand Up @@ -195,6 +195,7 @@
"Samuel Marks <samuelmarks@gmail.com>",
"Saugat Acharya <mesaugat@gmail.com>",
"Schmulik Raskin <schmuli@gmail.com>",
"Sergei Startsev <sergei.startsev@gmail.com>",
"Sergey Kruk <sergey.kruk@gmail.com>",
"Sergey Simonchik <sergey.simonchik@jetbrains.com>",
"Seth Rhodes <seth@thinkpixbit.com>",
Expand Down
61 changes: 39 additions & 22 deletions test/unit/server.spec.js
View file
Expand Up @@ -11,6 +11,7 @@ describe('server', () => {
var mockLauncher
var mockWebServer
var mockSocketServer
var mockBoundServer
var mockExecutor
var doneSpy
var server = mockConfig = browserCollection = webServerOnError = null
Expand Down Expand Up @@ -45,8 +46,7 @@ describe('server', () => {

sinon.stub(server._injector, 'invoke').returns([])

mockExecutor =
{schedule: () => {}}
mockExecutor = { schedule: () => {} }

mockFileList = {
refresh: sinon.spy(() => {
Expand Down Expand Up @@ -80,6 +80,13 @@ describe('server', () => {
}
}

mockBoundServer = {
on: sinon.spy((event, callback) => callback && callback()),
listen: sinon.spy((port, listenAddress, callback) => callback && callback()),
close: sinon.spy((callback) => callback && callback()),
address: () => { return { port: 9876 } }
}

mockWebServer = {
on (name, handler) {
if (name === 'error') {
Expand All @@ -99,18 +106,24 @@ describe('server', () => {
close: sinon.spy((callback) => callback && callback())
}

sinon.stub(server._injector, 'get')
sinon
.stub(server._injector, 'get')
.withArgs('webServer').returns(mockWebServer)
.withArgs('socketServer').returns(mockSocketServer)

webServerOnError = null
})

describe('start', () => {
var config
beforeEach(() => {
config = { port: 9876, listenAddress: '127.0.0.1' }
sinon.spy(BundleUtils, 'bundleResourceIfNotExist')
sinon.stub(NetUtils, 'getAvailablePort').resolves(9876)
sinon.stub(server, 'get').withArgs('config').returns({ port: 9876, listenAddress: '127.0.0.1' })
sinon.stub(NetUtils, 'bindAvailablePort').resolves(mockBoundServer)
sinon.stub(mockBoundServer, 'address').returns({ port: 9877 })
sinon
.stub(server, 'get')
.withArgs('config').returns(config)
})

it('should compile static resources', (done) => {
Expand All @@ -123,7 +136,17 @@ describe('server', () => {

it('should search for available port', (done) => {
server.start().then(() => {
expect(NetUtils.getAvailablePort).to.have.been.calledWith(9876, '127.0.0.1')
expect(NetUtils.bindAvailablePort).to.have.been.calledWith(9876, '127.0.0.1')
expect(mockBoundServer.address).to.have.been.called
done()
})
})

it('should change config.port to available', (done) => {
expect(config.port).to.be.equal(9876)
server.start().then(() => {
expect(config.port).to.be.equal(9877)
expect(server._boundServer).to.be.equal(mockBoundServer)
done()
})
})
Expand All @@ -133,6 +156,10 @@ describe('server', () => {
// server._start()
// ============================================================================
describe('_start', () => {
beforeEach(() => {
server._boundServer = mockBoundServer
})

it('should start the web server after all files have been preprocessed successfully', () => {
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)

Expand All @@ -142,7 +169,8 @@ describe('server', () => {
expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)

fileListOnResolve()
expect(mockWebServer.listen).to.have.been.called
expect(mockWebServer.listen).to.have.been.calledWith(mockBoundServer, sinon.match.func)
expect(webServerOnError).not.to.be.null
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
})

Expand All @@ -155,34 +183,23 @@ describe('server', () => {
expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)

fileListOnReject()
expect(mockWebServer.listen).to.have.been.called
expect(mockWebServer.listen).to.have.been.calledWith(mockBoundServer, sinon.match.func)
expect(webServerOnError).not.to.be.null
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
})

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(webServerOnError).not.to.be.null
expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)

fileListOnResolve()
expect(mockWebServer.listen).to.have.been.called
expect(mockWebServer.listen).to.have.been.calledWith(mockBoundServer, sinon.match.func)
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
})

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()
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', () => {
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)

Expand Down
53 changes: 20 additions & 33 deletions test/unit/utils/net-utils.spec.js
View file
Expand Up @@ -4,46 +4,33 @@ 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)
})
describe('NetUtils.bindAvailablePort', () => {
it('resolves server with bound port when it is available', (done) => {
NetUtils.bindAvailablePort(9876, '127.0.0.1').then((boundServer) => {
const port = boundServer.address().port
expect(port).to.be.equal(9876)
expect(boundServer).not.to.be.null
const server = net.createServer(connect()).listen(boundServer, () => {
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)
})
it('resolves with next available port', (done) => {
const server = net.createServer(connect()).listen(9876, '127.0.0.1', () => {
NetUtils.bindAvailablePort(9876, '127.0.0.1').then((boundServer) => {
const port = boundServer.address().port
expect(port).to.be.equal(9877)
expect(boundServer).not.to.be.null
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)
it('rejects if a critical error occurs', (done) => {
const incorrectAddress = '123.321'
NetUtils.bindAvailablePort(9876, incorrectAddress).catch((err) => {
expect(err).not.to.be.null
done()
})
})
Expand Down

0 comments on commit 850a90b

Please sign in to comment.