-
Notifications
You must be signed in to change notification settings - Fork 5.4k
revert: Dynamic client path resolution #5303
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
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: SPA Fallback Route Missing Error Handling
The SPA fallback route's res.sendFile() call for index.html now lacks error handling. If the file is not found at the hardcoded path, users receive generic Express errors instead of the previously provided informative, structured JSON responses. This degrades user experience and complicates diagnosing client UI deployment issues.
packages/server/src/index.ts#L646-L648
eliza/packages/server/src/index.ts
Lines 646 to 648 in 65966dd
| // Client files are built into the CLI package's dist directory | |
| const cliDistPath = path.resolve(__dirname, '../../cli/dist'); | |
| res.sendFile(path.join(cliDistPath, 'index.html')); |
Bug: Server UI Path Hardcoding Causes Silent Failures
The server now hardcodes the client UI path to ../../cli/dist, removing the previous dynamic resolution and robust error handling. This makes the system less resilient, as it will silently fail to serve static assets (e.g., CSS, JS) if the client files are not located at this assumed path, resulting in a broken web UI without any diagnostic logging or warnings.
packages/server/src/index.ts#L572-L575
eliza/packages/server/src/index.ts
Lines 572 to 575 in 65966dd
| // Client files are built into the CLI package's dist directory | |
| const clientPath = path.resolve(__dirname, '../../cli/dist'); | |
| this.app.use(express.static(clientPath, staticOptions)); | |
Was this report helpful? Give feedback by reacting with 👍 or 👎
This PR reverts commit 9f50fda which introduced dynamic client path resolution.
The reverted commit attempted to resolve client paths from multiple locations, which may have introduced unexpected behavior. This PR restores the original fixed path resolution for client assets.