fix(server): prefix port/host with api, fix logging (#10035)

Last fix for v7 I noticed while trying to get #10034 done. In talking to Josh about his experiences deploying to Fly etc and some of the pain points around that, I may introduce a `createServer` for the web side in a minor. If they're in the same file (which they will be for now), parsing argv with just port and host will be a problem. So to avoid that I've made it so that it's apiPort and apiHost.
redwoodjs · Feb 20, 2024 · 5b2e74a · 5b2e74a
1 parent 3e1f89c
commit 5b2e74a
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 49 deletions.
diff --git a/docs/docs/docker.md b/docs/docs/docker.md
@@ -637,10 +637,10 @@ await server.start()
 `start` is a thin wrapper around [`listen`](https://fastify.dev/docs/latest/Reference/Server/#listen).
 It takes the same arguments as `listen`, except for host and port. It computes those in the following way, in order of precedence:
 
-1. `--host` or `--port` flags:
+1. `--apiHost` or `--apiPort` flags:
 
   ```
-  yarn node api/dist/server.js --host 0.0.0.0 --port 8913
+  yarn node api/dist/server.js --apiHost 0.0.0.0 --apiPort 8913
   ```
 
 2. `REDWOOD_API_HOST` or `REDWOOD_API_PORT` env vars:

diff --git a/packages/api-server/src/__tests__/createServer.test.ts b/packages/api-server/src/__tests__/createServer.test.ts
@@ -239,8 +239,8 @@ describe('resolveOptions', () => {
           DEFAULT_CREATE_SERVER_OPTIONS.fastifyServerOptions.requestTimeout,
         logger: DEFAULT_CREATE_SERVER_OPTIONS.logger,
       },
-      port: 65501,
-      host: '::',
+      apiPort: 65501,
+      apiHost: '::',
     })
   })
 
@@ -309,17 +309,17 @@ describe('resolveOptions', () => {
     })
   })
 
-  it('parses `--port`', () => {
+  it('parses `--apiPort`', () => {
     expect(
-      resolveOptions({ parseArgs: true }, ['--port', '8930']).port
+      resolveOptions({ parseArgs: true }, ['--apiPort', '8930']).apiPort
     ).toEqual(8930)
   })
 
-  it("throws if `--port` can't be converted to an integer", () => {
+  it("throws if `--apiPort` can't be converted to an integer", () => {
     expect(() => {
-      resolveOptions({ parseArgs: true }, ['--port', 'eight-nine-ten'])
+      resolveOptions({ parseArgs: true }, ['--apiPort', 'eight-nine-ten'])
     }).toThrowErrorMatchingInlineSnapshot(
-      `[Error: \`port\` must be an integer]`
+      `[Error: \`apiPort\` must be an integer]`
     )
   })
 
@@ -338,9 +338,9 @@ describe('resolveOptions', () => {
     ).toEqual('/bar/')
   })
 
-  it('parses `--host`', () => {
+  it('parses `--apiHost`', () => {
     expect(
-      resolveOptions({ parseArgs: true }, ['--host', '127.0.0.1']).host
+      resolveOptions({ parseArgs: true }, ['--apiHost', '127.0.0.1']).apiHost
     ).toEqual('127.0.0.1')
   })
 })
diff --git a/packages/api-server/src/apiCLIConfigHandler.ts b/packages/api-server/src/apiCLIConfigHandler.ts
@@ -1,5 +1,7 @@
 import chalk from 'chalk'
 
+import { coerceRootPath } from '@redwoodjs/fastify-web'
+
 import { getAPIPort, getAPIHost } from './cliHelpers'
 import createFastifyInstance from './fastify'
 import { redwoodFastifyAPI } from './plugins/api'
@@ -9,6 +11,8 @@ export async function handler(options: APIParsedOptions) {
   const timeStart = Date.now()
   console.log(chalk.dim.italic('Starting API Server...'))
 
+  options.apiRootPath = coerceRootPath(options.apiRootPath ?? '/')
+
   const fastify = await createFastifyInstance()
   fastify.register(redwoodFastifyAPI, {
     redwood: {

diff --git a/packages/api-server/src/createServer.ts b/packages/api-server/src/createServer.ts
@@ -64,7 +64,7 @@ if (!process.env.REDWOOD_ENV_FILES_LOADED) {
  * ```
  */
 export async function createServer(options: CreateServerOptions = {}) {
-  const { apiRootPath, fastifyServerOptions, port, host } =
+  const { apiRootPath, fastifyServerOptions, apiPort, apiHost } =
     resolveOptions(options)
 
   // Warn about `api/server.config.js`
@@ -154,18 +154,18 @@ export async function createServer(options: CreateServerOptions = {}) {
   })
 
   /**
-   * A wrapper around `fastify.listen` that handles `--port`, `REDWOOD_API_PORT` and [api].port in redwood.toml
+   * A wrapper around `fastify.listen` that handles `--apiPort`, `REDWOOD_API_PORT` and [api].port in redwood.toml (same for host)
    *
    * The order of precedence is:
-   * - `--port`
+   * - `--apiPort`
    * - `REDWOOD_API_PORT`
    * - [api].port in redwood.toml
    */
   server.start = (options: StartOptions = {}) => {
     return server.listen({
       ...options,
-      port,
-      host,
+      port: apiPort,
+      host: apiHost,
     })
   }
 

diff --git a/packages/api-server/src/createServerHelpers.ts b/packages/api-server/src/createServerHelpers.ts
@@ -51,15 +51,17 @@ export const DEFAULT_CREATE_SERVER_OPTIONS: DefaultCreateServerOptions = {
 type ResolvedOptions = Required<
   Omit<CreateServerOptions, 'logger' | 'fastifyServerOptions' | 'parseArgs'> & {
     fastifyServerOptions: FastifyServerOptions
-    port: number
-    host: string
+    apiPort: number
+    apiHost: string
   }
 >
 
 export function resolveOptions(
   options: CreateServerOptions = {},
   args?: string[]
 ) {
+  options.parseArgs ??= true
+
   options.logger ??= DEFAULT_CREATE_SERVER_OPTIONS.logger
 
   // Set defaults.
@@ -73,8 +75,8 @@ export function resolveOptions(
       logger: options.logger ?? DEFAULT_CREATE_SERVER_OPTIONS.logger,
     },
 
-    host: getAPIHost(),
-    port: getAPIPort(),
+    apiHost: getAPIHost(),
+    apiPort: getAPIPort(),
   }
 
   // Merge fastifyServerOptions.
@@ -85,32 +87,33 @@ export function resolveOptions(
   if (options.parseArgs) {
     const { values } = parseArgs({
       options: {
-        host: {
+        apiHost: {
           type: 'string',
         },
-        port: {
+        apiPort: {
           type: 'string',
           short: 'p',
         },
         apiRootPath: {
           type: 'string',
         },
       },
+      strict: false,
       ...(args && { args }),
     })
 
-    if (values.host && typeof values.host !== 'string') {
-      throw new Error('`host` must be a string')
+    if (values.apiHost && typeof values.apiHost !== 'string') {
+      throw new Error('`apiHost` must be a string')
     }
-    if (values.host) {
-      resolvedOptions.host = values.host
+    if (values.apiHost) {
+      resolvedOptions.apiHost = values.apiHost
     }
 
-    if (values.port) {
-      resolvedOptions.port = +values.port
+    if (values.apiPort) {
+      resolvedOptions.apiPort = +values.apiPort
 
-      if (isNaN(resolvedOptions.port)) {
-        throw new Error('`port` must be an integer')
+      if (isNaN(resolvedOptions.apiPort)) {
+        throw new Error('`apiPort` must be an integer')
       }
     }
 

diff --git a/packages/api-server/src/watch.ts b/packages/api-server/src/watch.ts
@@ -138,7 +138,7 @@ const buildAndRestart = async ({
     if (serverFile) {
       httpServerProcess = fork(
         serverFile,
-        ['--port', port.toString()],
+        ['--apiPort', port.toString()],
         forkOpts
       )
     } else {

diff --git a/packages/cli/src/commands/serve.js b/packages/cli/src/commands/serve.js
@@ -25,7 +25,7 @@ export const builder = async (yargs) => {
     .command({
       command: '$0',
       description: bothServerCLIConfig.description,
-      builder: bothServerCLIConfig.builder(yargs),
+      builder: bothServerCLIConfig.builder,
       handler: async (argv) => {
         recordTelemetryAttributes({
           command: 'serve',

diff --git a/packages/cli/src/commands/serveHandler.js b/packages/cli/src/commands/serveHandler.js
@@ -32,9 +32,9 @@ export const bothServerFileHandler = async (options) => {
     [
       {
         name: 'api',
-        command: `yarn node ${path.join('dist', 'server.js')} --port ${
+        command: `yarn node ${path.join('dist', 'server.js')} --apiPort ${
           options.apiPort
-        } --host ${options.apiHost} --api-root-path ${options.apiRootPath}`,
+        } --apiHost ${options.apiHost} --apiRootPath ${options.apiRootPath}`,
         cwd: getPaths().api.base,
         prefixColor: 'cyan',
       },
@@ -66,19 +66,14 @@ export const bothServerFileHandler = async (options) => {
 }
 
 export const apiServerFileHandler = async (options) => {
-  await execa(
-    'yarn',
-    [
-      'node',
-      'server.js',
-      '--port',
-      options.port,
-      '--apiRootPath',
-      options.apiRootPath,
-    ],
-    {
-      cwd: getPaths().api.dist,
-      stdio: 'inherit',
-    }
-  )
+  const args = ['node', 'server.js', '--apiRootPath', options.apiRootPath]
+
+  if (options.port) {
+    args.push('--apiPort', options.port)
+  }
+
+  await execa('yarn', args, {
+    cwd: getPaths().api.dist,
+    stdio: 'inherit',
+  })
 }