Skip to content

Commit

Permalink
fix(server): error early on incompatible config (apiHost and apiUrl) (#…
Browse files Browse the repository at this point in the history
…9808)

I was working on Redwood's server config with @Josh-Walker-GM. One of
the more unfortunate footguns is when you're serving the web side alone
while the api side is somewhere else, on a completely different host. If
you don't pass `apiHost`, and `apiUrl` isn't fully qualified, what
happens is that the web side requests itself, resulting in network
responses like this:

<img width="1532" alt="image"
src="https://github.com/redwoodjs/redwood/assets/32992335/21c6f3ec-5614-4336-9967-1ab4c25ea7db">

An easy fix that would go a long way is just erroring out if we detect
the config is incompatible and not starting the server at all:

```
# In redwood.toml, apiUrl is `/.redwood/functions`, a relative URL, and apiHost isn't passed the at the CLI:
% yarn rw serve web        
Error: If you don't provide apiHost, apiUrl needs to be a fully-qualified URL. But apiUrl is /.redwood/functions.
```

We used a specific exit code because `yarn rw serve web` is a bin that's
calling a bin `yarn rw-server-web`. We plan to fix that by deduping code
across packages, so this specific exit code is just a stopgap till we
get that work done. The reason we're doing this fix first is that again,
it's easy and would solve a big problem.

We referenced this doc on exit code conventions which stated that 64-113
was up for grabs: https://tldp.org/LDP/abs/html/exitcodes.html.
  • Loading branch information
jtoar authored Jan 7, 2024
1 parent 93b04f8 commit 5210e06
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 17 deletions.
47 changes: 31 additions & 16 deletions packages/cli/src/commands/serveWebHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import execa from 'execa'

import { getPaths } from '@redwoodjs/project-config'

import { exitWithError } from '../lib/exit'

export const webSsrServerHandler = async () => {
await execa('yarn', ['rw-serve-fe'], {
cwd: getPaths().web.base,
Expand All @@ -11,21 +13,34 @@ export const webSsrServerHandler = async () => {
}

export const webServerHandler = async (argv) => {
await execa(
'yarn',
[
'rw-web-server',
'--port',
argv.port,
'--socket',
argv.socket,
'--api-host',
argv.apiHost,
],
{
cwd: getPaths().base,
stdio: 'inherit',
shell: true,
try {
await execa(
'yarn',
[
'rw-web-server',
'--port',
argv.port,
'--socket',
argv.socket,
'--api-host',
argv.apiHost,
],
{
cwd: getPaths().base,
stdio: 'inherit',
shell: true,
}
)
} catch (e) {
// `@redwoodjs/web-server` uses a custom error exit code to tell this handler that an error has already been handled.
// While any other exit code than `0` is considered an error, there seems to be some conventions around some of them
// like `127`, etc. We chose 64 because it's in the range where there deliberately aren't any previous conventions.
// See https://tldp.org/LDP/abs/html/exitcodes.html.
if (e.exitCode === 64) {
process.exitCode = 1
return
}
)

exitWithError(e)
}
}
28 changes: 27 additions & 1 deletion packages/web-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,15 @@ interface Opts {
apiHost?: string
}

// no help option...
function isFullyQualifiedUrl(url: string) {
try {
// eslint-disable-next-line no-new
new URL(url)
return true
} catch (e) {
return false
}
}

async function serve() {
// Parse server file args
Expand All @@ -39,6 +47,24 @@ async function serve() {
const port = options.port ? parseInt(options.port) : redwoodConfig.web.port
const apiUrl = redwoodConfig.web.apiUrl

if (!options.apiHost && !isFullyQualifiedUrl(apiUrl)) {
console.error(
`${chalk.red('Error')}: If you don't provide ${chalk.magenta(
'apiHost'
)}, ${chalk.magenta(
'apiUrl'
)} needs to be a fully-qualified URL. But ${chalk.magenta(
'apiUrl'
)} is ${chalk.yellow(apiUrl)}.`
)
// We're using a custom error exit code here to tell `@redwoodjs/cli` that this error has been handled.
// While any other exit code than `0` is considered an error, there seems to be some conventions around some of them
// like `127`, etc. We chose 64 because it's in the range where there deliberately aren't any previous conventions.
// See https://tldp.org/LDP/abs/html/exitcodes.html.
process.exitCode = 64
return
}

const tsServer = Date.now()

// Load .env files
Expand Down

0 comments on commit 5210e06

Please sign in to comment.