From d1c7143f730ab98b48f1ed0e939ef6975821a99f Mon Sep 17 00:00:00 2001 From: "Alexander J. Lallier" Date: Wed, 18 Nov 2020 21:33:40 -0500 Subject: [PATCH] Finished up work started by @al5ina5 in a PR (#263) auto incrementing the port if it is already in use. This commit adds the following to the existing work: * Added new tests * Added the port to reload's return object so the calling program knows what port reload ending up using if the port auto increments * Added autoIncrementPort optional param to realod to turn off auto port incrementing. Default `true` * Tweaked the code to work correctly, like handling the promise return correctly * Small tweaks to how the generated client side code replace works. Removed a branch that wasn't hitting in coverage and made it more readable * Updated README with API changes --- README.md | 24 ++++++++++++----------- lib/reload.js | 47 ++++++++++++++++++++++++++++++--------------- test/api/api.js | 44 ++++++++++++++++++++++++++++++++++++++++++ test/api/verbose.js | 29 ++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 12753bf..f32035c 100644 --- a/README.md +++ b/README.md @@ -194,31 +194,33 @@ _Consult the [migration guide](MIGRATION_GUIDE.md) for help updating reload acro ##### Table of options for reload opts parameter -| Parameter Name | Type | Description | Optional | Default | -|--------------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|---------| +| Parameter Name | Type | Description | Optional | Default | +|:------------------------:|:-------:|:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------:|:--------:|:-------:| | port | number | Port to run reload on. | ✓ | 9856 | -| webSocketServerWaitStart | boolean | When enabled will delay starting and opening WebSocket server when requiring reload. After enabling use the startWebSocketServer function returned in the object provided by the API to start the WebSocket. Note: Failing to call the returned function with this option enabled will cause reload not to work. See return API for more information | ✓ | FALSE | +| autoIncrementPort | boolean | Auto increments the reload port if the port is already in use | ✓ | TRUE | +| webSocketServerWaitStart | boolean | When enabled will delay starting and opening WebSocket server when requiring reload. After enabling use the startWebSocketServer function returned in the object provided by the API to start the WebSocket. Note: Failing to call the returned function with this option enabled will cause reload not to work. See return API for more information | ✓ | FALSE | | route | string | Route that reload should use to serve the client side script file. Changing the route will require the script tag URL to change. Reload will always strip any occurrence of reload.js and append reload.js for you. This is to ensure case, order, and use of / is correct. For example specifying newRoutePath as the route will give reload a route of newRoutePath/reload.js. (Recommend not modifying). | ✓ | reload | -| forceWss | boolean | Forces reload client connections to always use `wss` (secure websockerts) even when the window location is HTTP | ✓ | FALSE | +| forceWss | boolean | Forces reload client connections to always use wss (secure websockerts) even when the window location is HTTP | ✓ | FALSE | | https | object | HTTP options object. When defined runs reload in HTTPS mode | ✓ | {} | | https.certAndKey | object | Object that holds configuration for HTTPS key and cert configuration | ✓ | {} | | https.certAndKey.key | string | File path to HTTP key (not optional when defining an HTTPS object) | | null | | https.certAndKey.cert | string | File path to HTTP cert (not optional when defining an HTTPS object) | | null | | https.p12 | object | Object that holds configuration for HTTPS P12 configuration | ✓ | {} | | https.p12.p12Path | string | File path or file contents as string (Not optional when using P12 configuration | | null | -| https.passphrase | string | Shared passphrase used for a single private key and/or p12. | ✓ | null | +| https.passphrase | string | Shared passphrase used for a single private key and/or p12. | ✓ | null | | verbose | boolean | If set to true, will show logging on the server and client side. | ✓ | FALSE | #### Returns An **object** containing: -| Name | Type | Description | -|----------------------|----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| reload | function | A function that when called reloads all connected clients. For more information see manually firing server-side reload events. | -| startWebSocketServer | function | Returns a promise. Starts and opens the WebSocket server required for reload. Only **defined** when using the optional parameter `webSocketServerWaitStart`. Read the [parameters](#parameters) for more information | -| closeServer | function | Returns a promise. Closes Reload WebSocket server | -| wss | object | Web socket server | +| Name | Type | Description | +|:--------------------:|:--------:|:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------:| +| reload | function | A function that when called reloads all connected clients. For more information see manually firing server-side reload events. | +| startWebSocketServer | function | Returns a promise. Starts and opens the WebSocket server required for reload. Only defined when using the optional parameter webSocketServerWaitStart. Read the parameters for more information | +| closeServer | function | Returns a promise. Closes Reload WebSocket server | +| port | number | Port reload is running on. Useful when reload auto increments the port if the default/specified port is in use | +| wss | object | Web socket server | Using reload as a command line application --- diff --git a/lib/reload.js b/lib/reload.js index 687604a..e0f89aa 100644 --- a/lib/reload.js +++ b/lib/reload.js @@ -12,10 +12,11 @@ module.exports = function reload (app, opts, server) { return new Promise(function (resolve, reject) { // Parameters variables - var port = opts.port || 9856 + const port = opts.port || 9856 const httpsOption = opts.https || null const httpServerOrPort = server || port const forceWss = opts.forceWss || false + const autoIncrementPort = opts.autoIncrementPort === false ? false : true // eslint-disable-line no-unneeded-ternary const verboseLogging = opts.verbose || false const webSocketServerWaitStart = opts.webSocketServerWaitStart || false @@ -28,9 +29,11 @@ module.exports = function reload (app, opts, server) { let wss // General variables - const socketPortSpecified = server ? null : port + const commandLineUsingServer = server const connections = {} let httpOrHttpsServer + let hadToIncrement = false + var incrementTimer if (argumentCache[0] === undefined) { return reject(new Error('Lack of/invalid arguments provided to reload')) @@ -57,7 +60,7 @@ module.exports = function reload (app, opts, server) { } // Application setup - // setupClientSideCode(port) + setupClientSideCode() setupExpressAppRouting().then(function () { if (!webSocketServerWaitStart) { @@ -96,9 +99,8 @@ module.exports = function reload (app, opts, server) { reloadCode = reloadCode.replace('verboseLogging = false', 'verboseLogging = true') } - const webSocketString = forceWss ? 'wss://$3' : 'ws$2://$3' - - reloadCode = reloadCode.replace('socketUrl.replace()', 'socketUrl.replace(/(^http(s?):\\/\\/)(.*:)(.*)/,' + (socketPortSpecified ? '\'' + webSocketString + (port || socketPortSpecified) : '\'' + webSocketString + '$4') + '\')') + const webSocketString = forceWss ? 'wss://$3' : 'ws$2://$3$4' + reloadCode = reloadCode.replace('socketUrl.replace()', `socketUrl.replace(/(^http(s?):\\/\\/)(.*:)(.*)/, '${webSocketString}')`) } // Websocket server setup @@ -111,7 +113,7 @@ module.exports = function reload (app, opts, server) { console.log('Starting WebSocket Server') } - if (socketPortSpecified) { // Use custom user specified port + if (!commandLineUsingServer) { // Default mode for reload. Starts server on default/specified port. The else is used for command line reload and attaches to a server wss = new WebSocketServer({ noServer: true }) if (httpsOption) { // HTTPS @@ -165,17 +167,18 @@ module.exports = function reload (app, opts, server) { httpOrHttpsServer = http.createServer() } - function runServer(port) { - httpOrHttpsServer.listen(port, function () { - resolve(getReloadReturn(),) - setupClientSideCode(port) - }) - } runServer(port) - httpOrHttpsServer.once('error', function (err) { - if (err.code === 'EADDRINUSE') { + // Wait to see if a EADDRINUSE port already in use error occurs so we can increment before resolving the promise. Time is restarted if error case hits. + incrementTimer = setTimeout(resolvePromise, 100) + + httpOrHttpsServer.on('error', function (err) { + /* istanbul ignore else */ + if (err.code === 'EADDRINUSE' && autoIncrementPort) { + hadToIncrement = true port = port + 1 + clearTimeout(incrementTimer) + incrementTimer = setTimeout(resolvePromise, 100) runServer(port) } }) @@ -202,6 +205,17 @@ module.exports = function reload (app, opts, server) { }) } + function resolvePromise () { + if (hadToIncrement && verboseLogging) { + console.log('Incremented port number. Server running on:', port) + } + resolve(getReloadReturn()) + } + + function runServer (port) { + httpOrHttpsServer.listen(port) + } + function sendMessage (message) { if (verboseLogging) { console.log('Sending message to ' + (wss.clients.size) + ' connection(s): ' + message) @@ -260,7 +274,8 @@ module.exports = function reload (app, opts, server) { } httpOrHttpsServer.close(resolve) }) - } + }, + port: port } // Only define the function and make it available if the WebSocket is waiting in the first place diff --git a/test/api/api.js b/test/api/api.js index 13bfe5c..8f4c4e3 100644 --- a/test/api/api.js +++ b/test/api/api.js @@ -128,6 +128,50 @@ describe('API', function () { assert.strictEqual(result, true, 'Could not connect to WebSocket') }) + it('Should increment if default/specified port is unavailable', async () => { + const net = require('net') + const server = net.createServer() + + server.listen(9856) + + var app = express() + + try { + var reloadReturned = await reload(app) + } catch (err) { + + } + + var result = await helperFunction.testWebSocket(reloadReturned.port) + + await helperFunction.closeReloadSocket(reloadReturned) + + assert.strictEqual(result, true, 'Could not connect to WebSocket') + + server.close() + }) + + it('Should *not* increment if default/specified port is unavailable and autoIncrementOption is set to false', async () => { + const net = require('net') + const server = net.createServer() + + server.listen(9856) + + var app = express() + + try { + var reloadReturned = await reload(app, { autoIncrementPort: false }) + } catch (err) { + + } + + await helperFunction.closeReloadSocket(reloadReturned) + + assert.strictEqual(reloadReturned.port, 9856, 'Should not increment') + + server.close() + }) + it('Should error if unable to attach route to express app', async () => { try { await reload(function () {}) diff --git a/test/api/verbose.js b/test/api/verbose.js index 5e9fdc9..703904a 100644 --- a/test/api/verbose.js +++ b/test/api/verbose.js @@ -77,6 +77,35 @@ describe('Verbose', function () { console.error.restore() }) + it('Should verbose log if increment was required if default/specified port is unavailable', async () => { + sinon.stub(console, 'log').returns(0) + sinon.stub(console, 'error').returns(0) + + const net = require('net') + const server = net.createServer() + + server.listen(9856) + + var app = express() + + try { + var reloadReturned = await reload(app, { verbose: true }) + } catch (err) { + + } + + const foundLog = helperFunction.checkForConsoleLog(console.log.args, 'Incremented port number. Server running on:', reloadReturned.port) + + await helperFunction.closeReloadSocket(reloadReturned) + + server.close() + + assert(foundLog, 'Incremented port number. Server running on: ' + reloadReturned.port + ' not found in console logging') + + console.log.restore() + console.error.restore() + }) + it('Should error if verbose logging option is not a boolean', async () => { var app = express()