Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/docs/docker.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
18 changes: 9 additions & 9 deletions packages/api-server/src/__tests__/createServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,8 @@ describe('resolveOptions', () => {
DEFAULT_CREATE_SERVER_OPTIONS.fastifyServerOptions.requestTimeout,
logger: DEFAULT_CREATE_SERVER_OPTIONS.logger,
},
port: 65501,
host: '::',
apiPort: 65501,
apiHost: '::',
})
})

Expand Down Expand Up @@ -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]`
)
})

Expand All @@ -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')
})
})
4 changes: 4 additions & 0 deletions packages/api-server/src/apiCLIConfigHandler.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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: {
Expand Down
10 changes: 5 additions & 5 deletions packages/api-server/src/createServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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,
})
}

Expand Down
31 changes: 17 additions & 14 deletions packages/api-server/src/createServerHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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')
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/api-server/src/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ const buildAndRestart = async ({
if (serverFile) {
httpServerProcess = fork(
serverFile,
['--port', port.toString()],
['--apiPort', port.toString()],
forkOpts
)
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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',
Expand Down
25 changes: 10 additions & 15 deletions packages/cli/src/commands/serveApiHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,14 @@ import execa from 'execa'
import { getPaths } from '@redwoodjs/project-config'

export const apiServerFileHandler = async (argv) => {
await execa(
'yarn',
[
'node',
'server.js',
'--port',
argv.port,
'--apiRootPath',
argv.apiRootPath,
],
{
cwd: getPaths().api.dist,
stdio: 'inherit',
}
)
const args = ['node', 'server.js', '--apiRootPath', argv.apiRootPath]

if (argv.port) {
args.push('--apiPort', argv.port)
}

await execa('yarn', args, {
cwd: getPaths().api.dist,
stdio: 'inherit',
})
}
4 changes: 2 additions & 2 deletions packages/cli/src/commands/serveBothHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ export const bothServerFileHandler = async (argv) => {
[
{
name: 'api',
command: `yarn node ${path.join('dist', 'server.js')} --port ${
command: `yarn node ${path.join('dist', 'server.js')} --apiPort ${
argv.apiPort
} --host ${argv.apiHost} --api-root-path ${argv.apiRootPath}`,
} --apiHost ${argv.apiHost} --apiRootPath ${argv.apiRootPath}`,
cwd: getPaths().api.base,
prefixColor: 'cyan',
},
Expand Down
Loading