Skip to content

fix(new-webui): Bind to host specified by host environment variable (fixes #951). #954

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.

Sign up for GitHub

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

Merged
davemarco merged 4 commits into from
Jun 3, 2025
Merged

fix(new-webui): Bind to host specified by host environment variable (fixes #951). #954

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0c8d64
latest
Jun 2, 2025
22c312b
latest
Jun 2, 2025
85c23d8
Merge branch 'main' into host
davemarco Jun 2, 2025
6a6ff2e
Merge branch 'main' into host
davemarco Jun 3, 2025
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
3 changes: 1 addition & 2 deletions components/log-viewer-webui/server/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import serviceApp from "./fastify-v2/app.js";


const DEFAULT_FASTIFY_CLOSE_GRACE_DELAY = 500;
const DEFAULT_PORT = 3000;

/**
* Generates logger configuration options based on the environment.
Expand Down Expand Up @@ -59,7 +58,7 @@ const init = async (): Promise<void> => {
await app.ready();

try {
await app.listen({port: Number(process.env.PORT || DEFAULT_PORT)});
await app.listen({host: app.config.HOST, port: app.config.PORT});

@coderabbitai coderabbitai Bot Jun 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify that app.config properties exist and add error handling.

The change correctly reads from app.config, but there's no validation that HOST and PORT properties exist. This could cause runtime errors if the configuration is incomplete.

Please verify that app.config.HOST and app.config.PORT are properly defined by running this script to check the configuration setup:

Consider adding defensive programming to handle missing config values:

-        await app.listen({host: app.config.HOST, port: app.config.PORT});
+        const host = app.config.HOST || '127.0.0.1';
+        const port = app.config.PORT || 3000;
+        await app.listen({host, port});

🏁 Script executed:

#!/bin/bash
# Description: Verify app.config properties are properly set in the Fastify configuration

# Search for config definitions and usage patterns
echo "=== Searching for config property definitions ==="
ast-grep --pattern 'config: {
  $$$
}'

echo "=== Searching for HOST and PORT config assignments ==="
rg -A 3 -B 3 'HOST.*=' 
rg -A 3 -B 3 'PORT.*='

echo "=== Searching for fastify config plugin registration ==="
rg -A 5 'fastify.*config'

Length of output: 4846

Please check how the config plugin is registered (and whether it enforces required HOST and PORT properties) by running:

🏁 Script executed:

#!/bin/bash
echo "=== Checking for @fastify/env import in app.ts ==="
rg -n "@fastify/env" components/log-viewer-webui/server/src/fastify-v2/app.ts

echo "=== Listing fastify.register calls in app.ts ==="
rg -n "fastify\.register" -A3 components/log-viewer-webui/server/src/fastify-v2/app.ts

Length of output: 1017

Add validation or fallbacks for HOST and PORT.

The config plugin isn’t enforcing these variables or providing defaults, so calling
await app.listen({ host: app.config.HOST, port: app.config.PORT })
may throw at runtime if either is undefined.

• In server/src/main.ts (line 62):

await app.listen({ host: app.config.HOST, port: app.config.PORT });

• In server/src/fastify-v2/app.ts: no import or registration of @fastify/env was detected.

Suggested fixes:

  1. Register @fastify/env with a schema requiring HOST and PORT (and sensible defaults):

    await fastify.register(import('@fastify/env'), {
  schema: {
    type: 'object',
    required: ['HOST', 'PORT'],
    properties: {
      HOST: { type: 'string', default: '127.0.0.1' },
      PORT: { type: 'integer', default: 3000 }
    }
  }
});

  2. Or apply inline fallbacks in main.ts:

    - await app.listen({ host: app.config.HOST, port: app.config.PORT });
+ const host = app.config.HOST ?? '127.0.0.1';
+ const port = app.config.PORT ?? 3000;
+ await app.listen({ host, port });
🤖 Prompt for AI Agents 
In components/log-viewer-webui/server/src/main.ts at line 62, the code uses
app.config.HOST and app.config.PORT without verifying their existence, which can
cause runtime errors if these config values are missing. To fix this, add
validation to ensure HOST and PORT are defined before calling app.listen, or
provide default fallback values inline. Alternatively, register the @fastify/env
plugin in server/src/fastify-v2/app.ts with a schema that requires HOST and PORT
and sets sensible defaults, ensuring these config properties are always
available.

@davemarco davemarco Jun 2, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shh

@coderabbitai coderabbitai Bot Jun 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davemarco Understood! I'll drop this concern. 🤐

} catch (err) {
app.log.error(err);
process.exit(1);
Expand Down