feat: Port in use checking for development server (#6437)

* Initial port in use check implementation * Added suggested port, initial prompt and ensured consistent logging styles * Removed excess style import * Changed to use node-portfinder, tidied console logging and added initial port suggestion usage * fix(deps): pin portfinder and dedupe yarn.lock * Improved conditionals Co-authored-by: Dominic Saadi <[email protected]> * Added port flag to the watch api-server * Added working port checking * Made ports consistently a number type, fixed typo in port use warning and removed stray console log * Added default port options to mock config and updated inline snapshot to include --port flag * Apply suggestions from code review Co-authored-by: Dominic Saadi <[email protected]> * Applying more suggestions from code review * Changed to a simple error warning * style: remove "=" after flag this is valid but we don't do it for debugPort so just chasing consistency * fix: substring test and snapshot * Apply suggestions from code review Co-authored-by: Dominic Saadi <[email protected]> * Remove unused function Co-authored-by: Dominic Saadi <[email protected]>
redwoodjs · Nov 21, 2022 · bf5d7f9 · bf5d7f9
1 parent 92500ef
commit bf5d7f9
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 15 deletions.
diff --git a/packages/api-server/src/watch.ts b/packages/api-server/src/watch.ts
@@ -22,6 +22,11 @@ const argv = yargs(hideBin(process.argv))
     description: 'Debugging port',
     type: 'number',
   })
+  .option('port', {
+    alias: 'p',
+    description: 'Port',
+    type: 'number',
+  })
   .help()
   .alias('help', 'h')
   .parseSync()
@@ -75,10 +80,12 @@ const rebuildApiServer = () => {
       forkOpts.execArgv = forkOpts.execArgv.concat([`--inspect=${debugPort}`])
     }
 
+    const port = argv.port ?? getConfig().api.port
+
     // Start API server
     httpServerProcess = fork(
       path.join(__dirname, 'index.js'),
-      ['api', '--port', getConfig().api.port.toString()],
+      ['api', '--port', port.toString()],
       forkOpts
     )
   } catch (e) {

diff --git a/packages/cli/package.json b/packages/cli/package.json
@@ -55,6 +55,7 @@
     "param-case": "3.0.4",
     "pascalcase": "1.0.0",
     "pluralize": "8.0.0",
+    "portfinder": "1.0.32",
     "prettier": "2.7.1",
     "prisma": "4.6.1",
     "prompts": "2.4.2",

diff --git a/packages/cli/src/commands/__tests__/dev.test.js b/packages/cli/src/commands/__tests__/dev.test.js
@@ -70,8 +70,11 @@ describe('yarn rw dev', () => {
 
   it('Should run api and web dev servers, and  by default', async () => {
     getConfig.mockReturnValue({
-      web: {},
+      web: {
+        port: 8910,
+      },
       api: {
+        port: 8911,
         debugPort: 18911,
       },
     })
@@ -92,16 +95,19 @@ describe('yarn rw dev', () => {
     )
 
     expect(apiCommand.command).toMatchInlineSnapshot(
-      `"yarn cross-env NODE_ENV=development NODE_OPTIONS=--enable-source-maps yarn nodemon --quiet --watch "/mocked/project/redwood.toml" --exec "yarn rw-api-server-watch --debug-port 18911 | rw-log-formatter""`
+      `"yarn cross-env NODE_ENV=development NODE_OPTIONS=--enable-source-maps yarn nodemon --quiet --watch "/mocked/project/redwood.toml" --exec "yarn rw-api-server-watch --port 8911 --debug-port 18911 | rw-log-formatter""`
     )
 
     expect(generateCommand.command).toEqual('yarn rw-gen-watch')
   })
 
   it('Debug port passed in command line overrides TOML', async () => {
     getConfig.mockReturnValue({
-      web: {},
+      web: {
+        port: 8910,
+      },
       api: {
+        port: 8911,
         debugPort: 505050,
       },
     })
@@ -115,14 +121,17 @@ describe('yarn rw dev', () => {
     const apiCommand = find(concurrentlyArgs, { name: 'api' })
 
     expect(apiCommand.command).toContain(
-      'yarn rw-api-server-watch --debug-port 90909090'
+      'yarn rw-api-server-watch --port 8911 --debug-port 90909090'
     )
   })
 
   it('Can disable debugger by setting toml to false', async () => {
     getConfig.mockReturnValue({
-      web: {},
+      web: {
+        port: 8910,
+      },
       api: {
+        port: 8911,
         debugPort: false,
       },
     })

diff --git a/packages/cli/src/commands/devHandler.js b/packages/cli/src/commands/devHandler.js
@@ -11,6 +11,7 @@ import { errorTelemetry } from '@redwoodjs/telemetry'
 import { getPaths } from '../lib'
 import c from '../lib/colors'
 import { generatePrismaClient } from '../lib/generatePrismaClient'
+import { getFreePort } from '../lib/ports'
 
 const defaultApiDebugPort = 18911
 
@@ -23,6 +24,63 @@ export const handler = async ({
 }) => {
   const rwjsPaths = getPaths()
 
+  // Starting values of ports from config (redwood.toml)
+  let apiPreferredPort = parseInt(getConfig().api.port)
+  let webPreferredPort = parseInt(getConfig().web.port)
+
+  // Assume we can have the ports we want
+  let apiAvailablePort = apiPreferredPort
+  let apiPortChangeNeeded = false
+  let webAvailablePort = webPreferredPort
+  let webPortChangeNeeded = false
+
+  // Check api port
+  if (side.includes('api')) {
+    apiAvailablePort = await getFreePort(apiPreferredPort)
+    if (apiAvailablePort === -1) {
+      console.error(`Error could not determine a free port for the api server`)
+      process.exit(1)
+    }
+    apiPortChangeNeeded = apiAvailablePort !== apiPreferredPort
+  }
+
+  // Check web port
+  if (side.includes('web')) {
+    // Extract any ports the user forwarded to the webpack server and prefer that instead
+    const forwardedPortMatches = [
+      ...forward.matchAll(/\-\-port(\=|\s)(?<port>[^\s]*)/g),
+    ]
+    if (forwardedPortMatches.length) {
+      webPreferredPort = forwardedPortMatches.pop().groups.port
+    }
+
+    webAvailablePort = await getFreePort(webPreferredPort, [
+      apiPreferredPort,
+      apiAvailablePort,
+    ])
+    if (webAvailablePort === -1) {
+      console.error(`Error could not determine a free port for the web server`)
+      process.exit(1)
+    }
+    webPortChangeNeeded = webAvailablePort !== webPreferredPort
+  }
+
+  // Check for port conflict and exit with message if found
+  if (apiPortChangeNeeded || webPortChangeNeeded) {
+    let message = `The currently configured ports for the development server are unavailable. Suggested changes to your ports, which can be changed in redwood.toml, are:\n`
+    message += apiPortChangeNeeded
+      ? `  - API to use port ${apiAvailablePort} instead of your currently configured ${apiPreferredPort}\n`
+      : ``
+    message += webPortChangeNeeded
+      ? `  - Web to use port ${webAvailablePort} instead of your currently configured ${webPreferredPort}\n`
+      : ``
+    console.error(message)
+    console.error(
+      `Cannot run the development server until your configured ports are changed or become available.`
+    )
+    process.exit(1)
+  }
+
   if (side.includes('api')) {
     try {
       await generatePrismaClient({
@@ -39,7 +97,7 @@ export const handler = async ({
     }
 
     try {
-      await shutdownPort(getConfig().api.port)
+      await shutdownPort(apiAvailablePort)
     } catch (e) {
       errorTelemetry(process.argv, `Error shutting down "api": ${e.message}`)
       console.error(
@@ -50,7 +108,7 @@ export const handler = async ({
 
   if (side.includes('web')) {
     try {
-      await shutdownPort(getConfig().web.port)
+      await shutdownPort(webAvailablePort)
     } catch (e) {
       errorTelemetry(process.argv, `Error shutting down "web": ${e.message}`)
       console.error(
@@ -86,7 +144,7 @@ export const handler = async ({
   const jobs = {
     api: {
       name: 'api',
-      command: `yarn cross-env NODE_ENV=development NODE_OPTIONS=--enable-source-maps yarn nodemon --quiet --watch "${redwoodConfigPath}" --exec "yarn rw-api-server-watch ${getApiDebugFlag()} | rw-log-formatter"`,
+      command: `yarn cross-env NODE_ENV=development NODE_OPTIONS=--enable-source-maps yarn nodemon --quiet --watch "${redwoodConfigPath}" --exec "yarn rw-api-server-watch --port ${apiAvailablePort} ${getApiDebugFlag()} | rw-log-formatter"`,
       prefixColor: 'cyan',
       runWhen: () => fs.existsSync(rwjsPaths.api.src),
     },

diff --git a/packages/cli/src/lib/ports.js b/packages/cli/src/lib/ports.js
@@ -0,0 +1,21 @@
+import portfinder from 'portfinder'
+
+/**
+ * Finds a free port
+ * @param  {[number]}   requestedPort Port to start searching from
+ * @param  {[number[]]} excludePorts  Array of port numbers to exclude
+ * @return {[number]}                 A free port equal or higher than requestedPort but not within excludePorts. If no port can be found then returns -1
+ */
+export async function getFreePort(requestedPort, excludePorts = []) {
+  try {
+    let freePort = await portfinder.getPortPromise({
+      port: requestedPort,
+    })
+    if (excludePorts.includes(freePort)) {
+      freePort = await getFreePort(freePort + 1, excludePorts)
+    }
+    return freePort
+  } catch (error) {
+    return -1
+  }
+}
diff --git a/yarn.lock b/yarn.lock
@@ -6466,6 +6466,7 @@ __metadata:
     param-case: 3.0.4
     pascalcase: 1.0.0
     pluralize: 8.0.0
+    portfinder: 1.0.32
     prettier: 2.7.1
     prisma: 4.6.1
     prompts: 2.4.2
@@ -11073,6 +11074,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"async@npm:^2.6.4":
+  version: 2.6.4
+  resolution: "async@npm:2.6.4"
+  dependencies:
+    lodash: ^4.17.14
+  checksum: 0ebb3273ef96513389520adc88e0d3c45e523d03653cc9b66f5c46f4239444294899bfd13d2b569e7dbfde7da2235c35cf5fd3ece9524f935d41bbe4efccdad0
+  languageName: node
+  linkType: hard
+
 "async@npm:^3.1.0, async@npm:^3.2.0, async@npm:^3.2.3":
   version: 3.2.4
   resolution: "async@npm:3.2.4"
@@ -21981,7 +21991,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"lodash@npm:4.17.21, lodash@npm:>=4.17.21, lodash@npm:^4.0.0, lodash@npm:^4.11.2, lodash@npm:^4.17.15, lodash@npm:^4.17.19, lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:~4.17.0":
+"lodash@npm:4.17.21, lodash@npm:>=4.17.21, lodash@npm:^4.0.0, lodash@npm:^4.11.2, lodash@npm:^4.17.14, lodash@npm:^4.17.15, lodash@npm:^4.17.19, lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:~4.17.0":
   version: 4.17.21
   resolution: "lodash@npm:4.17.21"
   checksum: d8cbea072bb08655bb4c989da418994b073a608dffa608b09ac04b43a791b12aeae7cd7ad919aa4c925f33b48490b5cfe6c1f71d827956071dae2e7bb3a6b74c
@@ -22907,14 +22917,14 @@ __metadata:
   languageName: node
   linkType: hard
 
-"mkdirp@npm:^0.5.1, mkdirp@npm:^0.5.3":
-  version: 0.5.5
-  resolution: "mkdirp@npm:0.5.5"
+"mkdirp@npm:^0.5.1, mkdirp@npm:^0.5.3, mkdirp@npm:^0.5.6":
+  version: 0.5.6
+  resolution: "mkdirp@npm:0.5.6"
   dependencies:
-    minimist: ^1.2.5
+    minimist: ^1.2.6
   bin:
     mkdirp: bin/cmd.js
-  checksum: 4469faeeba703bc46b7cdbe3097d6373747a581eb8b556ce41c8fd25a826eb3254466c6522ba823c2edb0b6f0da7beb91cf71f040bc4e361534a3e67f0994bd0
+  checksum: e2e2be789218807b58abced04e7b49851d9e46e88a2f9539242cc8a92c9b5c3a0b9bab360bd3014e02a140fc4fbc58e31176c408b493f8a2a6f4986bd7527b01
   languageName: node
   linkType: hard
 
@@ -24966,6 +24976,17 @@ __metadata:
   languageName: node
   linkType: hard
 
+"portfinder@npm:1.0.32":
+  version: 1.0.32
+  resolution: "portfinder@npm:1.0.32"
+  dependencies:
+    async: ^2.6.4
+    debug: ^3.2.7
+    mkdirp: ^0.5.6
+  checksum: cef8b567b78aabccc59fe8e103bac8b394bb45a6a69be626608f099f454124c775aaf47b274c006332c07ab3f501cde55e49aaeb9d49d78d90362d776a565cbf
+  languageName: node
+  linkType: hard
+
 "posix-character-classes@npm:^0.1.0":
   version: 0.1.1
   resolution: "posix-character-classes@npm:0.1.1"