Skip to content

Commit

Permalink
fix(server): Add error handler for webserver socket. (#3300)
Browse files Browse the repository at this point in the history
Some times connection arrive, are given 404 responses, then disconnect,
resulting in ECONNRESET errors on the socket.  After we began monitoring
UncaughtExceptions, these errors show up. Suppress them with a handler
that ignores errors.
  • Loading branch information
johnjbarton authored Apr 23, 2019
1 parent 13ed695 commit fe9a1dd
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
26 changes: 21 additions & 5 deletions lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ class Server extends KarmaEventEmitter {
BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js')
])
this._boundServer = await NetUtils.bindAvailablePort(config.port, config.listenAddress)
this._boundServer.on('connection', (socket) => {
// Attach an error handler to avoid UncaughtException errors.
socket.on('error', (err) => {
// Errors on this socket are retried, ignore them
this.log.debug('Ignoring error on webserver connection: ' + err)
})
})
config.port = this._boundServer.address().port
this._injector.invoke(this._start, this)
} catch (err) {
Expand Down Expand Up @@ -228,6 +235,10 @@ class Server extends KarmaEventEmitter {

socket.on('complete', (data, ack) => ack())

socket.on('error', (err) => {
this.log.debug('karma server socket error: ' + err)
})

socket.on('register', (info) => {
let newBrowser = info.id ? (capturedBrowsers.getById(info.id) || singleRunBrowsers.getById(info.id)) : null

Expand Down Expand Up @@ -372,14 +383,19 @@ class Server extends KarmaEventEmitter {
processWrapper.on('SIGINT', () => disconnectBrowsers(process.exitCode))
processWrapper.on('SIGTERM', disconnectBrowsers)

processWrapper.on('uncaughtException', (error) => {
this.log.error(error)
const reportError = (error) => {
process.emit('infrastructure_error', error)
disconnectBrowsers(1)
})
}

processWrapper.on('unhandledRejection', (error) => {
this.log.error(error)
disconnectBrowsers(1)
this.log.error('UnhandledRejection')
reportError(error)
})

processWrapper.on('uncaughtException', (error) => {
this.log.error('UncaughtException')
reportError(error)
})
}

Expand Down
3 changes: 2 additions & 1 deletion test/unit/server.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ describe('server', () => {
}

mockBoundServer = {
on: sinon.spy((event, callback) => callback && callback()),
on: sinon.spy((event, callback) => callback && callback(mockServerSocket)),
listen: sinon.spy((port, listenAddress, callback) => callback && callback()),
close: sinon.spy((callback) => callback && callback()),
address: () => { return { port: 9876 } }
Expand Down Expand Up @@ -148,6 +148,7 @@ describe('server', () => {
server.start().then(() => {
expect(NetUtils.bindAvailablePort).to.have.been.calledWith(9876, '127.0.0.1')
expect(mockBoundServer.address).to.have.been.called
expect(typeof mockSocketEventListeners.get('error')).to.be.equal('function')
done()
})
})
Expand Down

0 comments on commit fe9a1dd

Please sign in to comment.