fix: Catch local API server various errors (#6548)
* fix: Catch local API server various errors * chore: Add tests to cover error catches
This commit is contained in:
Nghia Doan
GitHub
parent
9741bf15b5
commit
6f827872fb
@ -749,7 +749,13 @@ pub async fn start_server(
|
||||
}
|
||||
});
|
||||
|
||||
let server = Server::bind(&addr).serve(make_svc);
|
||||
let server = match Server::try_bind(&addr) {
|
||||
Ok(builder) => builder.serve(make_svc),
|
||||
Err(e) => {
|
||||
log::error!("Failed to bind to {}: {}", addr, e);
|
||||
return Err(Box::new(e));
|
||||
}
|
||||
};
|
||||
log::info!("Jan API server started on http://{}", addr);
|
||||
|
||||
let server_task = tokio::spawn(async move {
|
||||
|
||||
@ -419,7 +419,6 @@ describe('useLocalApiServer', () => {
|
||||
expect(result.current.serverHost).toBe('127.0.0.1') // Should remain default
|
||||
expect(result.current.apiPrefix).toBe('/v1') // Should remain default
|
||||
|
||||
|
||||
act(() => {
|
||||
result.current.addTrustedHost('example.com')
|
||||
})
|
||||
@ -428,4 +427,240 @@ describe('useLocalApiServer', () => {
|
||||
expect(result.current.serverPort).toBe(9000) // Should remain changed
|
||||
})
|
||||
})
|
||||
|
||||
describe('error handling scenarios', () => {
|
||||
it('should provide correct configuration for port conflict error messages', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Test common conflicting ports and verify they're stored correctly
|
||||
// These values will be used in error messages when port conflicts occur
|
||||
const conflictPorts = [
|
||||
{ port: 80, expectedMessage: 'Port 80 is already in use' },
|
||||
{ port: 443, expectedMessage: 'Port 443 is already in use' },
|
||||
{ port: 3000, expectedMessage: 'Port 3000 is already in use' },
|
||||
{ port: 8080, expectedMessage: 'Port 8080 is already in use' },
|
||||
{ port: 11434, expectedMessage: 'Port 11434 is already in use' }
|
||||
]
|
||||
|
||||
conflictPorts.forEach(({ port, expectedMessage }) => {
|
||||
act(() => {
|
||||
result.current.setServerPort(port)
|
||||
})
|
||||
|
||||
expect(result.current.serverPort).toBe(port)
|
||||
// Verify the port value that would be used in error message construction
|
||||
expect(`Port ${result.current.serverPort} is already in use`).toBe(expectedMessage)
|
||||
})
|
||||
})
|
||||
|
||||
it('should validate API key requirements for error prevention', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Test empty API key - should trigger validation error
|
||||
act(() => {
|
||||
result.current.setApiKey('')
|
||||
})
|
||||
expect(result.current.apiKey).toBe('')
|
||||
expect(result.current.apiKey.trim().length === 0).toBe(true) // Would fail validation
|
||||
|
||||
// Test whitespace only API key - should trigger validation error
|
||||
act(() => {
|
||||
result.current.setApiKey(' ')
|
||||
})
|
||||
expect(result.current.apiKey).toBe(' ')
|
||||
expect(result.current.apiKey.toString().trim().length === 0).toBe(true) // Would fail validation
|
||||
|
||||
// Test valid API key - should pass validation
|
||||
act(() => {
|
||||
result.current.setApiKey('sk-valid-api-key-123')
|
||||
})
|
||||
expect(result.current.apiKey).toBe('sk-valid-api-key-123')
|
||||
expect(result.current.apiKey.toString().trim().length > 0).toBe(true) // Would pass validation
|
||||
})
|
||||
|
||||
it('should configure trusted hosts for CORS error handling', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Add hosts that are commonly involved in CORS errors
|
||||
const corsRelatedHosts = ['localhost', '127.0.0.1', '0.0.0.0', 'example.com']
|
||||
|
||||
corsRelatedHosts.forEach((host) => {
|
||||
act(() => {
|
||||
result.current.addTrustedHost(host)
|
||||
})
|
||||
})
|
||||
|
||||
expect(result.current.trustedHosts).toEqual(corsRelatedHosts)
|
||||
expect(result.current.trustedHosts.length).toBe(4) // Verify count for error context
|
||||
|
||||
// Test removing a critical host that might cause access errors
|
||||
act(() => {
|
||||
result.current.removeTrustedHost('127.0.0.1')
|
||||
})
|
||||
|
||||
expect(result.current.trustedHosts).toEqual(['localhost', '0.0.0.0', 'example.com'])
|
||||
expect(result.current.trustedHosts.includes('127.0.0.1')).toBe(false) // Might cause localhost access errors
|
||||
})
|
||||
|
||||
it('should configure timeout values that prevent timeout errors', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Test very short timeout - likely to cause timeout errors
|
||||
act(() => {
|
||||
result.current.setProxyTimeout(1)
|
||||
})
|
||||
expect(result.current.proxyTimeout).toBe(1)
|
||||
expect(result.current.proxyTimeout < 60).toBe(true) // Likely to timeout
|
||||
|
||||
// Test reasonable timeout - should prevent timeout errors
|
||||
act(() => {
|
||||
result.current.setProxyTimeout(600)
|
||||
})
|
||||
expect(result.current.proxyTimeout).toBe(600)
|
||||
expect(result.current.proxyTimeout >= 600).toBe(true) // Should be sufficient
|
||||
|
||||
// Test very long timeout - prevents timeout but might cause UX issues
|
||||
act(() => {
|
||||
result.current.setProxyTimeout(3600)
|
||||
})
|
||||
expect(result.current.proxyTimeout).toBe(3600)
|
||||
expect(result.current.proxyTimeout > 1800).toBe(true) // Very long timeout
|
||||
})
|
||||
|
||||
it('should configure server host to prevent binding errors', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Test localhost binding - generally safe
|
||||
act(() => {
|
||||
result.current.setServerHost('127.0.0.1')
|
||||
})
|
||||
expect(result.current.serverHost).toBe('127.0.0.1')
|
||||
expect(result.current.serverHost === '127.0.0.1').toBe(true) // Localhost binding
|
||||
|
||||
// Test all interfaces binding - might cause permission errors on some systems
|
||||
act(() => {
|
||||
result.current.setServerHost('0.0.0.0')
|
||||
})
|
||||
expect(result.current.serverHost).toBe('0.0.0.0')
|
||||
expect(result.current.serverHost === '0.0.0.0').toBe(true) // All interfaces binding (potential permission issues)
|
||||
|
||||
// Verify host format for error message construction
|
||||
expect(result.current.serverHost.includes('.')).toBe(true) // Valid IP format
|
||||
})
|
||||
})
|
||||
|
||||
describe('integration error scenarios', () => {
|
||||
it('should provide configuration data that matches error message patterns', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Set up configuration that would be used in actual error messages
|
||||
act(() => {
|
||||
result.current.setServerHost('127.0.0.1')
|
||||
result.current.setServerPort(8080)
|
||||
result.current.setApiKey('test-key')
|
||||
})
|
||||
|
||||
// Verify values match what error handling expects
|
||||
const config = {
|
||||
host: result.current.serverHost,
|
||||
port: result.current.serverPort,
|
||||
apiKey: result.current.apiKey
|
||||
}
|
||||
|
||||
expect(config.host).toBe('127.0.0.1')
|
||||
expect(config.port).toBe(8080)
|
||||
expect(config.apiKey).toBe('test-key')
|
||||
|
||||
// These values would be used in error messages like:
|
||||
// "Failed to bind to 127.0.0.1:8080: Address already in use"
|
||||
// "Port 8080 is already in use. Please try a different port."
|
||||
const expectedErrorContext = `${config.host}:${config.port}`
|
||||
expect(expectedErrorContext).toBe('127.0.0.1:8080')
|
||||
})
|
||||
|
||||
it('should detect invalid configurations that would cause startup errors', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Test configuration that would prevent server startup
|
||||
act(() => {
|
||||
result.current.setApiKey('') // Invalid - empty API key
|
||||
result.current.setServerPort(0) // Invalid - port 0
|
||||
})
|
||||
|
||||
// Verify conditions that would trigger validation errors
|
||||
const hasValidApiKey = !!(result.current.apiKey && result.current.apiKey.toString().trim().length > 0)
|
||||
const hasValidPort = result.current.serverPort > 0 && result.current.serverPort <= 65535
|
||||
|
||||
expect(hasValidApiKey).toBe(false) // Would trigger "Missing API key" error
|
||||
expect(hasValidPort).toBe(false) // Would trigger "Invalid port" error
|
||||
|
||||
// Fix configuration
|
||||
act(() => {
|
||||
result.current.setApiKey('valid-key')
|
||||
result.current.setServerPort(3000)
|
||||
})
|
||||
|
||||
const hasValidApiKeyFixed = !!(result.current.apiKey && result.current.apiKey.toString().trim().length > 0)
|
||||
const hasValidPortFixed = result.current.serverPort > 0 && result.current.serverPort <= 65535
|
||||
|
||||
expect(hasValidApiKeyFixed).toBe(true) // Should pass validation
|
||||
expect(hasValidPortFixed).toBe(true) // Should pass validation
|
||||
})
|
||||
})
|
||||
|
||||
describe('configuration validation', () => {
|
||||
it('should maintain consistent state for server configuration', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Set up a complete server configuration
|
||||
act(() => {
|
||||
result.current.setServerHost('127.0.0.1')
|
||||
result.current.setServerPort(8080)
|
||||
result.current.setApiPrefix('/api/v1')
|
||||
result.current.setApiKey('test-key-123')
|
||||
result.current.setTrustedHosts(['localhost', '127.0.0.1'])
|
||||
result.current.setProxyTimeout(300)
|
||||
result.current.setCorsEnabled(true)
|
||||
result.current.setVerboseLogs(false)
|
||||
})
|
||||
|
||||
// Verify all settings are consistent
|
||||
expect(result.current.serverHost).toBe('127.0.0.1')
|
||||
expect(result.current.serverPort).toBe(8080)
|
||||
expect(result.current.apiPrefix).toBe('/api/v1')
|
||||
expect(result.current.apiKey).toBe('test-key-123')
|
||||
expect(result.current.trustedHosts).toEqual(['localhost', '127.0.0.1'])
|
||||
expect(result.current.proxyTimeout).toBe(300)
|
||||
expect(result.current.corsEnabled).toBe(true)
|
||||
expect(result.current.verboseLogs).toBe(false)
|
||||
})
|
||||
|
||||
it('should handle edge cases in configuration values', () => {
|
||||
const { result } = renderHook(() => useLocalApiServer())
|
||||
|
||||
// Test edge case: empty API prefix
|
||||
act(() => {
|
||||
result.current.setApiPrefix('')
|
||||
})
|
||||
expect(result.current.apiPrefix).toBe('')
|
||||
|
||||
// Test edge case: API prefix without leading slash
|
||||
act(() => {
|
||||
result.current.setApiPrefix('v1')
|
||||
})
|
||||
expect(result.current.apiPrefix).toBe('v1')
|
||||
|
||||
// Test edge case: minimum port number
|
||||
act(() => {
|
||||
result.current.setServerPort(1)
|
||||
})
|
||||
expect(result.current.serverPort).toBe(1)
|
||||
|
||||
// Test edge case: maximum valid port number
|
||||
act(() => {
|
||||
result.current.setServerPort(65535)
|
||||
})
|
||||
expect(result.current.serverPort).toBe(65535)
|
||||
})
|
||||
})
|
||||
})
|
||||
@ -22,6 +22,7 @@ import { ApiKeyInput } from '@/containers/ApiKeyInput'
|
||||
import { useEffect, useState } from 'react'
|
||||
import { PlatformGuard } from '@/lib/platform/PlatformGuard'
|
||||
import { PlatformFeature } from '@/lib/platform'
|
||||
import { toast } from 'sonner'
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
export const Route = createFileRoute(route.settings.local_api_server as any)({
|
||||
@ -133,6 +134,11 @@ function LocalAPIServerContent() {
|
||||
const toggleAPIServer = async () => {
|
||||
// Validate API key before starting server
|
||||
if (serverStatus === 'stopped') {
|
||||
console.log('Starting server with port:', serverPort)
|
||||
toast.info('Starting server...', {
|
||||
description: `Attempting to start server on port ${serverPort}`
|
||||
})
|
||||
|
||||
if (!apiKey || apiKey.toString().trim().length === 0) {
|
||||
setShowApiKeyError(true)
|
||||
return
|
||||
@ -179,9 +185,39 @@ function LocalAPIServerContent() {
|
||||
setServerStatus('running')
|
||||
})
|
||||
.catch((error: unknown) => {
|
||||
console.error('Error starting server:', error)
|
||||
console.error('Error starting server or model:', error)
|
||||
setServerStatus('stopped')
|
||||
setIsModelLoading(false) // Reset loading state on error
|
||||
toast.dismiss()
|
||||
|
||||
// Extract error message from various error formats
|
||||
const errorMsg = error && typeof error === 'object' && 'message' in error
|
||||
? String(error.message)
|
||||
: String(error)
|
||||
|
||||
// Port-related errors (highest priority)
|
||||
if (errorMsg.includes('Address already in use')) {
|
||||
toast.error('Port has been occupied', {
|
||||
description: `Port ${serverPort} is already in use. Please try a different port.`
|
||||
})
|
||||
}
|
||||
// Model-related errors
|
||||
else if (errorMsg.includes('Invalid or inaccessible model path')) {
|
||||
toast.error('Invalid or inaccessible model path', {
|
||||
description: errorMsg
|
||||
})
|
||||
}
|
||||
else if (errorMsg.includes('model')) {
|
||||
toast.error('Failed to start model', {
|
||||
description: errorMsg
|
||||
})
|
||||
}
|
||||
// Generic server errors
|
||||
else {
|
||||
toast.error('Failed to start server', {
|
||||
description: errorMsg
|
||||
})
|
||||
}
|
||||
})
|
||||
} else {
|
||||
setServerStatus('pending')
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user