Skip to content

Commit a19b8d4

Browse files
lusarzlusarz
committed
fix(server): check available port before start server (fix #1476, fix #3011)
1 parent 05dd09a commit a19b8d4

4 files changed

Lines changed: 153 additions & 86 deletions

File tree

‎lib/server.js‎

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const tmp = require('tmp')
99
const fs = require('fs')
1010
const path = require('path')
1111
const BundleUtils = require('./utils/bundle-utils')
12+
const NetUtils = require('./utils/net-utils')
1213
const root = global || window || this
1314

1415
const cfg = require('./config')
@@ -97,8 +98,24 @@ class Server extends KarmaEventEmitter {
9798
this._injector = new di.Injector(modules)
9899
}
99100

101+
dieOnError (error) {
102+
this.log.error(error)
103+
process.exitCode = 1
104+
process.kill(process.pid, 'SIGINT')
105+
}
106+
100107
start () {
101-
this._injector.invoke(this._start, this)
108+
const config = this.get('config')
109+
return Promise.all([
110+
BundleUtils.bundleResourceIfNotExist('client/main.js', 'static/karma.js'),
111+
BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js')
112+
])
113+
.then(() => NetUtils.getAvailablePort(config.port, config.listenAddress))
114+
.then((port) => {
115+
config.port = port
116+
this._injector.invoke(this._start, this)
117+
})
118+
.catch(this.dieOnError.bind(this))
102119
}
103120

104121
get (token) {
@@ -126,47 +143,26 @@ class Server extends KarmaEventEmitter {
126143
const singleRunBrowsers = new BrowserCollection(new EventEmitter())
127144
let singleRunBrowserNotCaptured = false
128145

129-
webServer.on('error', (e) => {
130-
if (e.code === 'EADDRINUSE') {
131-
this.log.warn('Port %d in use', config.port)
132-
config.port++
133-
webServer.listen(config.port, config.listenAddress)
134-
} else {
135-
throw e
136-
}
137-
})
146+
webServer.on('error', this.dieOnError.bind(this))
138147

139148
const afterPreprocess = () => {
140149
if (config.autoWatch) {
141150
this._injector.invoke(watcher.watch)
142151
}
143152

144-
return Promise.all([
145-
BundleUtils.bundleResourceIfNotExist('client/main.js', 'static/karma.js'),
146-
BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js')
147-
])
148-
.then(() => {
149-
webServer.listen(config.port, config.listenAddress, () => {
150-
this.log.info(`Karma v${constant.VERSION} server started at ${config.protocol}//${config.listenAddress}:${config.port}${config.urlRoot}`)
151-
152-
this.emit('listening', config.port)
153-
if (config.browsers && config.browsers.length) {
154-
this._injector.invoke(launcher.launch, launcher).forEach((browserLauncher) => {
155-
singleRunDoneBrowsers[browserLauncher.id] = false
156-
})
157-
}
158-
if (this.loadErrors.length > 0) {
159-
this.log.error('Found %d load error%s', this.loadErrors.length, this.loadErrors.length === 1 ? '' : 's')
160-
process.exitCode = 1
161-
process.kill(process.pid, 'SIGINT')
162-
}
153+
webServer.listen(config.port, config.listenAddress, () => {
154+
this.log.info(`Karma v${constant.VERSION} server started at ${config.protocol}//${config.listenAddress}:${config.port}${config.urlRoot}`)
155+
156+
this.emit('listening', config.port)
157+
if (config.browsers && config.browsers.length) {
158+
this._injector.invoke(launcher.launch, launcher).forEach((browserLauncher) => {
159+
singleRunDoneBrowsers[browserLauncher.id] = false
163160
})
164-
})
165-
.catch((error) => {
166-
this.log.error('Front-end script compile failed with error: ' + error)
167-
process.exitCode = 1
168-
process.kill(process.pid, 'SIGINT')
169-
})
161+
}
162+
if (this.loadErrors.length > 0) {
163+
this.dieOnError(new Error(`Found ${this.loadErrors.length} load error${this.loadErrors.length === 1 ? '' : 's'}`))
164+
}
165+
})
170166
}
171167

172168
fileList.refresh().then(afterPreprocess, afterPreprocess)

‎lib/utils/net-utils.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
'use strict'
2+
3+
const Promise = require('bluebird')
4+
const net = require('net')
5+
6+
const NetUtils = {
7+
isPortAvailable (port, listenAddress) {
8+
return new Promise((resolve, reject) => {
9+
const server = net.createServer()
10+
11+
server.unref()
12+
server.on('error', (err) => {
13+
server.close()
14+
if (err.code === 'EADDRINUSE' || err.code === 'EACCES') {
15+
resolve(false)
16+
} else {
17+
reject(err)
18+
}
19+
})
20+
21+
server.listen(port, listenAddress, () => {
22+
server.close(() => resolve(true))
23+
})
24+
})
25+
},
26+
27+
getAvailablePort (port, listenAddress) {
28+
return NetUtils.isPortAvailable(port, listenAddress)
29+
.then((available) => available ? port : NetUtils.getAvailablePort(port + 1, listenAddress))
30+
}
31+
}
32+
33+
module.exports = NetUtils

‎test/unit/server.spec.js‎

Lines changed: 39 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
var Server = require('../../lib/server')
22
var BundleUtils = require('../../lib/utils/bundle-utils')
3+
var NetUtils = require('../../lib/utils/net-utils')
34
var BrowserCollection = require('../../lib/browser_collection')
45

56
describe('server', () => {
@@ -105,106 +106,93 @@ describe('server', () => {
105106
webServerOnError = null
106107
})
107108

108-
// ============================================================================
109-
// server._start()
110-
// ============================================================================
111-
describe('_start', () => {
112-
it('should compile static resources', (done) => {
109+
describe('start', () => {
110+
beforeEach(() => {
113111
sinon.spy(BundleUtils, 'bundleResourceIfNotExist')
112+
sinon.stub(NetUtils, 'getAvailablePort').resolves(9876)
113+
sinon.stub(server, 'get').withArgs('config').returns({ port: 9876, listenAddress: '127.0.0.1' })
114+
})
114115

115-
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
116-
117-
fileListOnResolve().then(() => {
116+
it('should compile static resources', (done) => {
117+
server.start().then(() => {
118118
expect(BundleUtils.bundleResourceIfNotExist).to.have.been.calledWith('client/main.js', 'static/karma.js')
119119
expect(BundleUtils.bundleResourceIfNotExist).to.have.been.calledWith('context/main.js', 'static/context.js')
120120
done()
121121
})
122122
})
123123

124-
it('should start the web server after all files have been preprocessed successfully', (done) => {
124+
it('should search for available port', (done) => {
125+
server.start().then(() => {
126+
expect(NetUtils.getAvailablePort).to.have.been.calledWith(9876, '127.0.0.1')
127+
done()
128+
})
129+
})
130+
})
131+
132+
// ============================================================================
133+
// server._start()
134+
// ============================================================================
135+
describe('_start', () => {
136+
it('should start the web server after all files have been preprocessed successfully', () => {
125137
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
126138

127139
expect(mockFileList.refresh).to.have.been.called
128140
expect(fileListOnResolve).not.to.be.null
129141
expect(mockWebServer.listen).not.to.have.been.called
130142
expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
131143

132-
fileListOnResolve().then(() => {
133-
expect(mockWebServer.listen).to.have.been.called
134-
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
135-
done()
136-
})
144+
fileListOnResolve()
145+
expect(mockWebServer.listen).to.have.been.called
146+
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
137147
})
138148

139-
it('should start the web server after all files have been preprocessed with an error', (done) => {
149+
it('should start the web server after all files have been preprocessed with an error', () => {
140150
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
141151

142152
expect(mockFileList.refresh).to.have.been.called
143153
expect(fileListOnReject).not.to.be.null
144154
expect(mockWebServer.listen).not.to.have.been.called
145155
expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
146156

147-
fileListOnReject().then(() => {
148-
expect(mockWebServer.listen).to.have.been.called
149-
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
150-
done()
151-
})
157+
fileListOnReject()
158+
expect(mockWebServer.listen).to.have.been.called
159+
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
152160
})
153161

154-
it('should launch browsers after the web server has started', (done) => {
162+
it('should launch browsers after the web server has started', () => {
155163
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
156164

157165
expect(mockWebServer.listen).not.to.have.been.called
158166
expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
159167

160-
fileListOnResolve().then(() => {
161-
expect(mockWebServer.listen).to.have.been.called
162-
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
163-
done()
164-
})
168+
fileListOnResolve()
169+
expect(mockWebServer.listen).to.have.been.called
170+
expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
165171
})
166172

167-
it('should listen on the listenAddress in the config', (done) => {
173+
it('should listen on the listenAddress in the config', () => {
168174
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
169175

170176
expect(mockWebServer.listen).not.to.have.been.called
171177
expect(webServerOnError).not.to.be.null
172178

173179
expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
174180

175-
fileListOnResolve().then(() => {
176-
expect(mockWebServer.listen).to.have.been.calledWith(9876, '127.0.0.1')
177-
expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
178-
done()
179-
})
180-
})
181-
182-
it('should try next port if already in use', () => {
183-
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
184-
185-
expect(mockWebServer.listen).not.to.have.been.called
186-
expect(webServerOnError).not.to.be.null
187-
188-
expect(mockConfig.port).to.be.equal(9876)
189-
190-
webServerOnError({code: 'EADDRINUSE'})
191-
192-
expect(mockWebServer.listen).to.have.been.calledWith(9877)
193-
expect(mockConfig.port).to.be.equal(9877)
181+
fileListOnResolve()
182+
expect(mockWebServer.listen).to.have.been.calledWith(9876, '127.0.0.1')
183+
expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
194184
})
195185

196-
it('should emit a listening event once server begin accepting connections', (done) => {
186+
it('should emit a listening event once server begin accepting connections', () => {
197187
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
198188

199189
var listening = sinon.spy()
200190
server.on('listening', listening)
201191

202192
expect(listening).not.to.have.been.called
203193

204-
fileListOnResolve().then(() => {
205-
expect(listening).to.have.been.calledWith(9876)
206-
done()
207-
})
194+
fileListOnResolve()
195+
expect(listening).to.have.been.calledWith(9876)
208196
})
209197

210198
it('should emit a browsers_ready event once all the browsers are captured', () => {

‎test/unit/utils/net-utils.spec.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
'use strict'
2+
3+
const NetUtils = require('../../../lib/utils/net-utils')
4+
const connect = require('connect')
5+
const net = require('net')
6+
7+
describe('NetUtils.isPortAvailable', () => {
8+
it('it is possible to run server on available port', (done) => {
9+
NetUtils.isPortAvailable(9876, '127.0.0.1').then((available) => {
10+
expect(available).to.be.true
11+
const server = net
12+
.createServer(connect())
13+
.listen(9876, '127.0.0.1', () => {
14+
server.close(done)
15+
})
16+
})
17+
})
18+
19+
it('resolves with false when port is used', (done) => {
20+
const server = net
21+
.createServer(connect())
22+
.listen(9876, '127.0.0.1', () => {
23+
NetUtils.isPortAvailable(9876, '127.0.0.1').then((available) => {
24+
expect(available).to.be.false
25+
server.close(done)
26+
})
27+
})
28+
})
29+
})
30+
31+
describe('NetUtils.getAvailablePort', () => {
32+
it('resolves with port when is available', (done) => {
33+
NetUtils.getAvailablePort(9876, '127.0.0.1').then((port) => {
34+
expect(port).to.equal(9876)
35+
done()
36+
})
37+
})
38+
39+
it('resolves with next available port', (done) => {
40+
const stub = sinon.stub(NetUtils, 'isPortAvailable')
41+
stub.withArgs(9876).resolves(false)
42+
stub.withArgs(9877).resolves(false)
43+
stub.withArgs(9878).resolves(true)
44+
45+
NetUtils.getAvailablePort(9876, '127.0.0.1').then((port) => {
46+
expect(port).to.equal(9878)
47+
done()
48+
})
49+
})
50+
})

0 commit comments

Comments
 (0)