mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 12:15:57 +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,
|
||||
|
||||
@@ -166,7 +166,8 @@ export interface RemoteProxyStatus {
|
||||
export interface ProxyRemoteConfig {
|
||||
enabled: boolean;
|
||||
host: string;
|
||||
port: number;
|
||||
/** Port is optional - uses protocol default (443 for HTTPS, 80 for HTTP) */
|
||||
port?: number;
|
||||
protocol: 'http' | 'https';
|
||||
auth_token: string;
|
||||
}
|
||||
@@ -402,7 +403,8 @@ export const api = {
|
||||
/** Test remote proxy connection */
|
||||
test: (params: {
|
||||
host: string;
|
||||
port: number;
|
||||
/** Port is optional - uses protocol default (443 for HTTPS, 80 for HTTP) */
|
||||
port?: number;
|
||||
protocol: 'http' | 'https';
|
||||
authToken?: string;
|
||||
allowSelfSigned?: boolean;
|
||||
|
||||
+28
-14
@@ -463,8 +463,8 @@ export function SettingsPage() {
|
||||
if (!proxyConfig) return;
|
||||
|
||||
const { host, port, protocol, auth_token } = proxyConfig.remote;
|
||||
if (!host || !port) {
|
||||
setProxyError('Host and port are required');
|
||||
if (!host) {
|
||||
setProxyError('Host is required');
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -475,7 +475,7 @@ export function SettingsPage() {
|
||||
|
||||
const result = await api.cliproxyServer.test({
|
||||
host,
|
||||
port,
|
||||
port: port || undefined, // Empty/0 means use protocol default
|
||||
protocol,
|
||||
authToken: auth_token || undefined,
|
||||
});
|
||||
@@ -1310,20 +1310,24 @@ function ProxyContent({
|
||||
fetchCliproxyServerConfig,
|
||||
fetchRawConfig,
|
||||
}: ProxyContentProps) {
|
||||
// Memoized default config to avoid recreation
|
||||
// Default configs for fallback
|
||||
const defaultRemote = {
|
||||
enabled: false,
|
||||
host: '',
|
||||
port: 8317,
|
||||
port: undefined as number | undefined,
|
||||
protocol: 'http' as const,
|
||||
auth_token: '',
|
||||
};
|
||||
const defaultFallback = { enabled: true, auto_start: true };
|
||||
const defaultLocal = { port: 8317, auto_start: true };
|
||||
|
||||
// Helper to get default port based on protocol
|
||||
const getDefaultPort = (protocol: 'http' | 'https') => (protocol === 'https' ? 443 : 80);
|
||||
|
||||
// Sync local state with config (using refs to avoid lint warnings)
|
||||
const hostInput = config?.remote.host ?? '';
|
||||
const portInput = (config?.remote.port ?? 8317).toString();
|
||||
// Show empty string for port if undefined (will use protocol default)
|
||||
const portInput = config?.remote.port !== undefined ? config.remote.port.toString() : '';
|
||||
const authTokenInput = config?.remote.auth_token ?? '';
|
||||
const localPortInput = (config?.local.port ?? 8317).toString();
|
||||
|
||||
@@ -1365,9 +1369,13 @@ function ProxyContent({
|
||||
};
|
||||
|
||||
const savePort = () => {
|
||||
const port = parseInt(editedPort ?? displayPort, 10);
|
||||
if (!isNaN(port) && port !== config?.remote.port) {
|
||||
saveCliproxyServerConfig({ remote: { ...remoteConfig, port } });
|
||||
const portStr = editedPort ?? displayPort;
|
||||
// Empty string means use protocol default (undefined)
|
||||
const port = portStr === '' ? undefined : parseInt(portStr, 10);
|
||||
const effectivePort = port && !isNaN(port) && port > 0 ? port : undefined;
|
||||
|
||||
if (effectivePort !== config?.remote.port) {
|
||||
saveCliproxyServerConfig({ remote: { ...remoteConfig, port: effectivePort } });
|
||||
}
|
||||
setEditedPort(null);
|
||||
};
|
||||
@@ -1496,13 +1504,19 @@ function ProxyContent({
|
||||
{/* Port and Protocol */}
|
||||
<div className="grid grid-cols-2 gap-3">
|
||||
<div className="space-y-1">
|
||||
<label className="text-sm text-muted-foreground">Port</label>
|
||||
<label className="text-sm text-muted-foreground">
|
||||
Port{' '}
|
||||
<span className="text-xs opacity-70">
|
||||
(default: {getDefaultPort(config?.remote.protocol || 'http')})
|
||||
</span>
|
||||
</label>
|
||||
<Input
|
||||
type="number"
|
||||
type="text"
|
||||
inputMode="numeric"
|
||||
value={displayPort}
|
||||
onChange={(e) => setEditedPort(e.target.value)}
|
||||
onChange={(e) => setEditedPort(e.target.value.replace(/\D/g, ''))}
|
||||
onBlur={savePort}
|
||||
placeholder="8317"
|
||||
placeholder={`Leave empty for ${getDefaultPort(config?.remote.protocol || 'http')}`}
|
||||
className="font-mono"
|
||||
disabled={saving}
|
||||
/>
|
||||
@@ -1545,7 +1559,7 @@ function ProxyContent({
|
||||
<div className="space-y-3 pt-2">
|
||||
<Button
|
||||
onClick={handleTestConnection}
|
||||
disabled={testing || !displayHost || !displayPort}
|
||||
disabled={testing || !displayHost}
|
||||
variant="outline"
|
||||
className="w-full"
|
||||
>
|
||||
|
||||
Reference in New Issue
Block a user