From 85b0f171105ecd2e12839718a80ae91e427f9b5a Mon Sep 17 00:00:00 2001 From: kaitranntt Date: Sun, 21 Dec 2025 18:03:37 -0500 Subject: [PATCH] fix(api): add try-catch error handling to route handlers - Add try-catch to cliproxy-auth-routes GET / - Add try-catch to health-routes GET / and POST /fix/:checkId - Add try-catch to config-routes GET /format and POST /migrate - Improve error messages with specific context - All handlers now return 500 with error message on failure --- src/web-server/routes/cliproxy-auth-routes.ts | 144 +++++++++++------- src/web-server/routes/config-routes.ts | 28 ++-- src/web-server/routes/health-routes.ts | 26 ++-- 3 files changed, 121 insertions(+), 77 deletions(-) diff --git a/src/web-server/routes/cliproxy-auth-routes.ts b/src/web-server/routes/cliproxy-auth-routes.ts index 6d5d16b2..5f24ac0a 100644 --- a/src/web-server/routes/cliproxy-auth-routes.ts +++ b/src/web-server/routes/cliproxy-auth-routes.ts @@ -32,57 +32,61 @@ const validProviders: CLIProxyProvider[] = ['gemini', 'codex', 'agy', 'qwen', 'i * GET /api/cliproxy/auth - Get auth status for built-in CLIProxy profiles * Also fetches CLIProxyAPI stats to update lastUsedAt for active providers */ -router.get('/', async (_req: Request, res: Response) => { - // Initialize accounts from existing tokens on first request - initializeAccounts(); +router.get('/', async (_req: Request, res: Response): Promise => { + try { + // Initialize accounts from existing tokens on first request + initializeAccounts(); - // Fetch CLIProxyAPI usage stats to determine active providers - const stats = await fetchCliproxyStats(); + // Fetch CLIProxyAPI usage stats to determine active providers + const stats = await fetchCliproxyStats(); - // Map CLIProxyAPI provider names to our internal provider names - const statsProviderMap: Record = { - gemini: 'gemini', - antigravity: 'agy', - codex: 'codex', - qwen: 'qwen', - iflow: 'iflow', - }; + // Map CLIProxyAPI provider names to our internal provider names + const statsProviderMap: Record = { + gemini: 'gemini', + antigravity: 'agy', + codex: 'codex', + qwen: 'qwen', + iflow: 'iflow', + }; - // Update lastUsedAt for providers with recent activity - if (stats?.requestsByProvider) { - for (const [statsProvider, requestCount] of Object.entries(stats.requestsByProvider)) { - if (requestCount > 0) { - const provider = statsProviderMap[statsProvider.toLowerCase()]; - if (provider) { - // Touch the default account for this provider (or all accounts) - const accounts = getProviderAccounts(provider); - for (const account of accounts) { - // Only touch if this is the default account (most likely being used) - if (account.isDefault) { - touchAccount(provider, account.id); + // Update lastUsedAt for providers with recent activity + if (stats?.requestsByProvider) { + for (const [statsProvider, requestCount] of Object.entries(stats.requestsByProvider)) { + if (requestCount > 0) { + const provider = statsProviderMap[statsProvider.toLowerCase()]; + if (provider) { + // Touch the default account for this provider (or all accounts) + const accounts = getProviderAccounts(provider); + for (const account of accounts) { + // Only touch if this is the default account (most likely being used) + if (account.isDefault) { + touchAccount(provider, account.id); + } } } } } } + + const statuses = getAllAuthStatus(); + + const authStatus = statuses.map((status) => { + const oauthConfig = getOAuthConfig(status.provider); + return { + provider: status.provider, + displayName: oauthConfig.displayName, + authenticated: status.authenticated, + lastAuth: status.lastAuth?.toISOString() || null, + tokenFiles: status.tokenFiles.length, + accounts: status.accounts, + defaultAccount: status.defaultAccount, + }; + }); + + res.json({ authStatus }); + } catch (error) { + res.status(500).json({ error: (error as Error).message }); } - - const statuses = getAllAuthStatus(); - - const authStatus = statuses.map((status) => { - const oauthConfig = getOAuthConfig(status.provider); - return { - provider: status.provider, - displayName: oauthConfig.displayName, - authenticated: status.authenticated, - lastAuth: status.lastAuth?.toISOString() || null, - tokenFiles: status.tokenFiles.length, - accounts: status.accounts, - defaultAccount: status.defaultAccount, - }; - }); - - res.json({ authStatus }); }); // ==================== Account Management ==================== @@ -91,11 +95,16 @@ router.get('/', async (_req: Request, res: Response) => { * GET /api/cliproxy/accounts - Get all accounts across all providers */ router.get('/accounts', (_req: Request, res: Response) => { - // Initialize accounts from existing tokens - initializeAccounts(); + try { + // Initialize accounts from existing tokens + initializeAccounts(); - const accounts = getAllAccountsSummary(); - res.json({ accounts }); + const accounts = getAllAccountsSummary(); + res.json({ accounts }); + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to list accounts'; + res.status(500).json({ error: message }); + } }); /** @@ -110,8 +119,13 @@ router.get('/accounts/:provider', (req: Request, res: Response): void => { return; } - const accounts = getProviderAccounts(provider as CLIProxyProvider); - res.json({ provider, accounts }); + try { + const accounts = getProviderAccounts(provider as CLIProxyProvider); + res.json({ provider, accounts }); + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to get provider accounts'; + res.status(500).json({ error: message }); + } }); /** @@ -132,12 +146,19 @@ router.post('/accounts/:provider/default', (req: Request, res: Response): void = return; } - const success = setDefaultAccountFn(provider as CLIProxyProvider, accountId); + try { + const success = setDefaultAccountFn(provider as CLIProxyProvider, accountId); - if (success) { - res.json({ provider, defaultAccount: accountId }); - } else { - res.status(404).json({ error: 'Account not found' }); + if (success) { + res.json({ provider, defaultAccount: accountId }); + } else { + res + .status(404) + .json({ error: `Account '${accountId}' not found for provider '${provider}'` }); + } + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to set default account'; + res.status(500).json({ error: message }); } }); @@ -153,12 +174,19 @@ router.delete('/accounts/:provider/:accountId', (req: Request, res: Response): v return; } - const success = removeAccountFn(provider as CLIProxyProvider, accountId); + try { + const success = removeAccountFn(provider as CLIProxyProvider, accountId); - if (success) { - res.json({ provider, accountId, deleted: true }); - } else { - res.status(404).json({ error: 'Account not found' }); + if (success) { + res.json({ provider, accountId, deleted: true }); + } else { + res + .status(404) + .json({ error: `Account '${accountId}' not found for provider '${provider}'` }); + } + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to remove account'; + res.status(500).json({ error: message }); } }); diff --git a/src/web-server/routes/config-routes.ts b/src/web-server/routes/config-routes.ts index 1c1b7f20..776ca579 100644 --- a/src/web-server/routes/config-routes.ts +++ b/src/web-server/routes/config-routes.ts @@ -24,12 +24,16 @@ const router = Router(); /** * GET /api/config/format - Return current config format and migration status */ -router.get('/format', (_req: Request, res: Response) => { - res.json({ - format: getConfigFormat(), - migrationNeeded: needsMigration(), - backups: getBackupDirectories(), - }); +router.get('/format', (_req: Request, res: Response): void => { + try { + res.json({ + format: getConfigFormat(), + migrationNeeded: needsMigration(), + backups: getBackupDirectories(), + }); + } catch (error) { + res.status(500).json({ error: (error as Error).message }); + } }); /** @@ -89,10 +93,14 @@ router.put('/', (req: Request, res: Response): void => { /** * POST /api/config/migrate - Trigger migration from JSON to YAML */ -router.post('/migrate', async (req: Request, res: Response) => { - const dryRun = req.query.dryRun === 'true'; - const result = await migrate(dryRun); - res.json(result); +router.post('/migrate', async (req: Request, res: Response): Promise => { + try { + const dryRun = req.query.dryRun === 'true'; + const result = await migrate(dryRun); + res.json(result); + } catch (error) { + res.status(500).json({ error: (error as Error).message }); + } }); /** diff --git a/src/web-server/routes/health-routes.ts b/src/web-server/routes/health-routes.ts index 5740633a..0331b4f8 100644 --- a/src/web-server/routes/health-routes.ts +++ b/src/web-server/routes/health-routes.ts @@ -10,22 +10,30 @@ const router = Router(); /** * GET /api/health - Run health checks */ -router.get('/', async (_req: Request, res: Response) => { - const report = await runHealthChecks(); - res.json(report); +router.get('/', async (_req: Request, res: Response): Promise => { + try { + const report = await runHealthChecks(); + res.json(report); + } catch (error) { + res.status(500).json({ error: (error as Error).message }); + } }); /** * POST /api/health/fix/:checkId - Fix a health issue */ router.post('/fix/:checkId', (req: Request, res: Response): void => { - const { checkId } = req.params; - const result = fixHealthIssue(checkId); + try { + const { checkId } = req.params; + const result = fixHealthIssue(checkId); - if (result.success) { - res.json({ success: true, message: result.message }); - } else { - res.status(400).json({ success: false, message: result.message }); + if (result.success) { + res.json({ success: true, message: result.message }); + } else { + res.status(400).json({ success: false, message: result.message }); + } + } catch (error) { + res.status(500).json({ error: (error as Error).message }); } });