mirror of
https://github.com/tiennm99/ccs.git
synced 2026-09-06 22:17:18 +00:00
fix(remote-proxy): fix TypeError and make port optional with protocol defaults
- Fix TypeError: error.code?.toLowerCase - add type guard for non-string error codes - Make remote proxy port optional - defaults to 443 (HTTPS) or 80 (HTTP) - Smart URL building - omits port when using protocol default - UI improvements - show default port hint, allow empty port field - Better validation - only host is required, port uses smart defaults Closes #142
This commit is contained in:
@@ -490,9 +490,13 @@ export function ensureProviderSettings(provider: CLIProxyProvider): void {
|
||||
*/
|
||||
export function getRemoteEnvVars(
|
||||
provider: CLIProxyProvider,
|
||||
remoteConfig: { host: string; port: number; protocol: 'http' | 'https'; authToken?: string }
|
||||
remoteConfig: { host: string; port?: number; protocol: 'http' | 'https'; authToken?: string }
|
||||
): Record<string, string> {
|
||||
const baseUrl = `${remoteConfig.protocol}://${remoteConfig.host}:${remoteConfig.port}/api/provider/${provider}`;
|
||||
// Build URL with smart port handling - omit if using protocol default
|
||||
const defaultPort = remoteConfig.protocol === 'https' ? 443 : 80;
|
||||
const effectivePort = remoteConfig.port ?? defaultPort;
|
||||
const portSuffix = effectivePort === defaultPort ? '' : `:${effectivePort}`;
|
||||
const baseUrl = `${remoteConfig.protocol}://${remoteConfig.host}${portSuffix}/api/provider/${provider}`;
|
||||
const models = getModelMapping(provider);
|
||||
|
||||
// Get global env vars (DISABLE_TELEMETRY, etc.)
|
||||
|
||||
@@ -26,8 +26,13 @@ export interface RemoteProxyStatus {
|
||||
export interface RemoteProxyClientConfig {
|
||||
/** Remote proxy host (IP or hostname) */
|
||||
host: string;
|
||||
/** Remote proxy port */
|
||||
port: number;
|
||||
/**
|
||||
* Remote proxy port.
|
||||
* Optional - defaults based on protocol:
|
||||
* - HTTPS: 443
|
||||
* - HTTP: 80
|
||||
*/
|
||||
port?: number;
|
||||
/** Protocol to use (http or https) */
|
||||
protocol: 'http' | 'https';
|
||||
/** Optional auth token for Authorization header */
|
||||
@@ -41,12 +46,45 @@ export interface RemoteProxyClientConfig {
|
||||
/** Default timeout for remote proxy requests (aggressive for CLI UX) */
|
||||
const DEFAULT_TIMEOUT_MS = 2000;
|
||||
|
||||
/**
|
||||
* Get default port for protocol
|
||||
*/
|
||||
function getDefaultPort(protocol: 'http' | 'https'): number {
|
||||
return protocol === 'https' ? 443 : 80;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build URL for remote proxy, intelligently omitting default ports
|
||||
*/
|
||||
function buildProxyUrl(
|
||||
host: string,
|
||||
port: number | undefined,
|
||||
protocol: 'http' | 'https',
|
||||
path: string
|
||||
): string {
|
||||
const defaultPort = getDefaultPort(protocol);
|
||||
const effectivePort = port ?? defaultPort;
|
||||
|
||||
// Omit port from URL if it matches the default for the protocol
|
||||
if (effectivePort === defaultPort) {
|
||||
return `${protocol}://${host}${path}`;
|
||||
}
|
||||
return `${protocol}://${host}:${effectivePort}${path}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Map error to RemoteProxyErrorCode
|
||||
*
|
||||
* Handles various error types including:
|
||||
* - NodeJS.ErrnoException (ECONNREFUSED, ETIMEDOUT)
|
||||
* - Fetch errors (AbortError, TypeError)
|
||||
* - HTTP status codes (401, 403)
|
||||
*/
|
||||
function mapErrorToCode(error: Error, statusCode?: number): RemoteProxyErrorCode {
|
||||
const message = error.message.toLowerCase();
|
||||
const code = (error as NodeJS.ErrnoException).code?.toLowerCase();
|
||||
// Handle error.code safely - it may be string, number, or undefined
|
||||
const rawCode = (error as NodeJS.ErrnoException).code;
|
||||
const code = typeof rawCode === 'string' ? rawCode.toLowerCase() : undefined;
|
||||
|
||||
// Connection refused
|
||||
if (code === 'econnrefused' || message.includes('connection refused')) {
|
||||
@@ -110,7 +148,17 @@ export async function checkRemoteProxy(
|
||||
const { host, port, protocol, authToken, allowSelfSigned = false } = config;
|
||||
const timeout = config.timeout ?? DEFAULT_TIMEOUT_MS;
|
||||
|
||||
const url = `${protocol}://${host}:${port}/health`;
|
||||
// Validate host is provided
|
||||
if (!host || host.trim() === '') {
|
||||
return {
|
||||
reachable: false,
|
||||
error: 'Host is required',
|
||||
errorCode: 'UNKNOWN',
|
||||
};
|
||||
}
|
||||
|
||||
// Use smart URL building - omit port if it's the default for the protocol
|
||||
const url = buildProxyUrl(host, port, protocol, '/health');
|
||||
const startTime = Date.now();
|
||||
|
||||
try {
|
||||
|
||||
@@ -184,7 +184,8 @@ function mergeWithDefaults(partial: Partial<UnifiedConfig>): UnifiedConfig {
|
||||
enabled:
|
||||
partial.cliproxy_server?.remote?.enabled ?? DEFAULT_CLIPROXY_SERVER_CONFIG.remote.enabled,
|
||||
host: partial.cliproxy_server?.remote?.host ?? DEFAULT_CLIPROXY_SERVER_CONFIG.remote.host,
|
||||
port: partial.cliproxy_server?.remote?.port ?? DEFAULT_CLIPROXY_SERVER_CONFIG.remote.port,
|
||||
// Port is optional - undefined means use protocol default (443/80)
|
||||
port: partial.cliproxy_server?.remote?.port,
|
||||
protocol:
|
||||
partial.cliproxy_server?.remote?.protocol ??
|
||||
DEFAULT_CLIPROXY_SERVER_CONFIG.remote.protocol,
|
||||
|
||||
@@ -195,8 +195,14 @@ export interface ProxyRemoteConfig {
|
||||
enabled: boolean;
|
||||
/** Remote proxy hostname or IP (empty = not configured) */
|
||||
host: string;
|
||||
/** Remote proxy port (default: 8317) */
|
||||
port: number;
|
||||
/**
|
||||
* Remote proxy port.
|
||||
* Optional - defaults based on protocol:
|
||||
* - HTTPS: 443
|
||||
* - HTTP: 80
|
||||
* When empty/undefined, uses protocol default.
|
||||
*/
|
||||
port?: number;
|
||||
/** Protocol for remote connection */
|
||||
protocol: 'http' | 'https';
|
||||
/** Auth token for remote proxy (optional, sent as header) */
|
||||
@@ -345,12 +351,13 @@ export const DEFAULT_COPILOT_CONFIG: CopilotConfig = {
|
||||
/**
|
||||
* Default CLIProxy server configuration.
|
||||
* Local mode by default - remote must be explicitly enabled.
|
||||
* Port is optional for remote - defaults based on protocol.
|
||||
*/
|
||||
export const DEFAULT_CLIPROXY_SERVER_CONFIG: CliproxyServerConfig = {
|
||||
remote: {
|
||||
enabled: false,
|
||||
host: '',
|
||||
port: 8317,
|
||||
// port is intentionally omitted - will use protocol default (443 for HTTPS, 80 for HTTP)
|
||||
protocol: 'http',
|
||||
auth_token: '',
|
||||
},
|
||||
|
||||
@@ -72,14 +72,20 @@ router.post('/test', async (req: Request, res: Response) => {
|
||||
try {
|
||||
const { host, port, protocol, authToken, allowSelfSigned } = req.body;
|
||||
|
||||
if (!host || !port) {
|
||||
res.status(400).json({ error: 'Host and port are required' });
|
||||
// Host is required, port is optional (uses protocol defaults)
|
||||
if (!host) {
|
||||
res.status(400).json({ error: 'Host is required' });
|
||||
return;
|
||||
}
|
||||
|
||||
// Parse port - treat empty string, 0, null as "use default"
|
||||
const parsedPort = port && port !== '' ? parseInt(String(port), 10) : undefined;
|
||||
const effectivePort =
|
||||
parsedPort && !isNaN(parsedPort) && parsedPort > 0 ? parsedPort : undefined;
|
||||
|
||||
const status = await testConnection({
|
||||
host,
|
||||
port: typeof port === 'number' ? port : parseInt(port, 10),
|
||||
port: effectivePort,
|
||||
protocol: protocol || 'http',
|
||||
authToken,
|
||||
allowSelfSigned: allowSelfSigned || false,
|
||||
|
||||
Reference in New Issue
Block a user