fix(cli): handle error for proxy server

[jiashengguo]

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling for proxy startup: when the chosen port is in use, users receive a clear, red-colored message explaining the issue and how to change the port; other startup errors now show informative messaging.

• Chores

  • Updated the default proxy port from 8008 to 2311.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[claude]

Claude finished @jiashengguo's task —— View job

---

PR Review: Handle Error for Proxy Server

Todo List:

• Read and analyze the modified proxy.ts file
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage needs
• Provide comprehensive feedback

---

Review Summary

This PR adds error handling for the Express server in the proxy action, specifically handling the EADDRINUSE error when the specified port is already in use. The implementation is solid and follows existing patterns in the codebase.

✅ Positives

1. Good Error Handling: The addition properly handles the common EADDRINUSE error case with a user-friendly message
2. Consistent with Codebase: Uses CliError class and colors for output formatting, matching existing patterns
3. Type Safety: Correctly types the error handler parameter as NodeJS.ErrnoException
4. Clean Code: The added code is well-formatted and readable
5. Proper Exit Codes: Uses process.exit(1) for error cases, consistent with other CLI actions

🔍 Code Quality & Best Practices

File: packages/cli/src/actions/proxy.ts:169-178

The implementation follows Node.js best practices:

• Proper error event listener registration
• Graceful error handling with user-friendly messages
• Appropriate process exit codes

⚠️ Areas for Consideration

1. Error Handling Pattern Inconsistency

The current approach mixes two error handling strategies:

• For EADDRINUSE: Uses console.error() + process.exit(1)
• For other errors: Uses throw new CliError() + process.exit(1)

Consideration: The throw new CliError() followed by process.exit(1) is redundant since the thrown error won't be caught (process exits immediately). Consider either:

• Use only console.error() + process.exit(1) for both cases
• Or use only throw new CliError() (remove process.exit(1)) and let the CLI's main error handler deal with it

2. Test Coverage Gap

There are no tests for the proxy action (packages/cli/test/ contains tests for other actions but not proxy). The new error handling code is untested.

Recommendation: Consider adding unit tests for the proxy action, especially for error scenarios like:

• Port already in use
• Invalid database connections
• Server startup failures

3. Error Message Consistency

The error message suggests using -p option but should verify this matches the actual CLI flag name.

🚀 Performance Considerations

• Minimal Impact: The error handler only executes on server startup errors, so no performance concerns
• Process Exit: Proper cleanup with process.exit(1) prevents hanging processes

🔒 Security Considerations

• No Security Issues: The error handling doesn't expose sensitive information
• Error Information: Error messages are appropriately generic while still being helpful

📋 Suggestions

1. Consider standardizing error handling approach (see point 1 above)
2. Add test coverage for the proxy action, including error scenarios
3. Verify CLI flag reference in error message matches actual implementation

🎯 Overall Assessment

This is a good, focused improvement that addresses a common user experience issue when starting the proxy server. The code quality is solid and follows existing patterns. The main area for improvement is test coverage and potentially standardizing the error handling approach.

Recommendation: ✅ Approve with minor suggestions

[coderabbitai]

📝 Walkthrough

Walkthrough

Updates proxy startup behavior and defaults: adds an HTTP server 'error' handler that treats 'EADDRINUSE' specially (logs a red, user-friendly port-in-use message and exits with code 1; other errors throw CliError and exit 1), and changes the proxy command default port from 8008 to 2311.

Changes

Cohort / File(s)	Summary
HTTP Server Error Handling packages/cli/src/actions/proxy.ts	Add an error event listener on the HTTP server: handle EADDRINUSE by logging a red, user-friendly message about the port being in use and exit with code 1; for other errors, throw a CliError and exit with code 1.
CLI Default Port packages/cli/src/index.ts	Change default port for the proxy command option from 8008 to 2311.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nudged the port from eight to two-three-one,
And listened when the server said "I'm done."
If a socket's taken, I politely explain,
Then tip my whiskers and hop off the plane. 🥕✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title states 'handle error for proxy server' but the changeset includes both error handling improvements AND a default port change from 8008 to 2311, making the title incomplete.	Update the title to reflect both major changes, such as 'fix(cli): handle proxy server errors and update default port' or separate into two focused commits.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jiasheng-dev

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR adds error handling for the proxy server to gracefully handle cases where the server fails to start, particularly when the port is already in use.

Changes:

• Added an 'error' event handler to the server that detects EADDRINUSE errors and provides a helpful message to the user
• Added fallback error handling for other server startup errors

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@packages/cli/src/actions/proxy.ts`:
- Around line 169-178: The error handler in server.on('error', ...) currently
throws a CliError in the else branch which makes process.exit(1) unreachable and
will create an unhandled exception; replace the throw new CliError(...) with a
console.error that logs the error message (including err.message and context
like "Failed to start the server") so the handler always reaches
process.exit(1); reference the existing symbols: server.on('error', (err:
NodeJS.ErrnoException) => { ... }), the CliError type, err, and options.port to
build the error log.

[claude]

Claude finished @jiashengguo's task —— View job

---

PR Review: Handle Error for Proxy Server

Todo List:

• Read and analyze the modified proxy.ts file
• Read and analyze the modified index.ts file
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage needs
• Provide comprehensive feedback

---

Review Summary

This PR adds error handling for the Express server in the proxy action, specifically handling the EADDRINUSE error when the specified port is already in use. The implementation is solid and follows existing patterns in the codebase.

✅ Positives

1. Good Error Handling: The addition properly handles the common EADDRINUSE error case with a user-friendly message
2. Consistent with Codebase: Uses CliError class and colors for output formatting, matching existing patterns
3. Type Safety: Correctly types the error handler parameter as NodeJS.ErrnoException
4. Clean Code: The added code is well-formatted and readable
5. Proper Exit Codes: Uses process.exit(1) for error cases, consistent with other CLI actions

🔍 Code Quality & Best Practices

File: packages/cli/src/actions/proxy.ts:169-178

The implementation follows Node.js best practices:

• Proper error event listener registration
• Graceful error handling with user-friendly messages
• Appropriate process exit codes

⚠️ Areas for Consideration

1. Error Handling Pattern Inconsistency

The current approach mixes two error handling strategies:

• For EADDRINUSE: Uses console.error() + process.exit(1)
• For other errors: Uses throw new CliError() + process.exit(1)

Consideration: The throw new CliError() followed by process.exit(1) is redundant since the thrown error won't be caught (process exits immediately). Consider either:

• Use only console.error() + process.exit(1) for both cases
• Or use only throw new CliError() (remove process.exit(1)) and let the CLI's main error handler deal with it

2. Test Coverage Gap

There are no tests for the proxy action (packages/cli/test/ contains tests for other actions but not proxy). The new error handling code is untested.

Recommendation: Consider adding unit tests for the proxy action, especially for error scenarios like:

• Port already in use
• Invalid database connections
• Server startup failures

3. Error Message Consistency

The error message suggests using -p option but should verify this matches the actual CLI flag name.

🚀 Performance Considerations

• Minimal Impact: The error handler only executes on server startup errors, so no performance concerns
• Process Exit: Proper cleanup with process.exit(1) prevents hanging processes

🔒 Security Considerations

• No Security Issues: The error handling doesn't expose sensitive information
• Error Information: Error messages are appropriately generic while still being helpful

📋 Suggestions

1. Consider standardizing error handling approach (see point 1 above)
2. Add test coverage for the proxy action, including error scenarios
3. Verify CLI flag reference in error message matches actual implementation

🎯 Overall Assessment

This is a good, focused improvement that addresses a common user experience issue when starting the proxy server. The code quality is solid and follows existing patterns. The main area for improvement is test coverage and potentially standardizing the error handling approach.

Recommendation: ✅ Approve with minor suggestions

---