From 66af5c7386a12f2efcbbe8837c48628dc71f75a8 Mon Sep 17 00:00:00 2001 From: lugnicca Date: Fri, 5 Sep 2025 19:56:28 +0200 Subject: [PATCH] fix: use webprovider services to fetch models --- .../hooks/__tests__/useProviderModels.test.ts | 42 ++++++++++--------- web-app/src/hooks/useProviderModels.ts | 9 ++-- web-app/src/services/providers/web.ts | 42 ++++++++++++++++--- 3 files changed, 63 insertions(+), 30 deletions(-) diff --git a/web-app/src/hooks/__tests__/useProviderModels.test.ts b/web-app/src/hooks/__tests__/useProviderModels.test.ts index 89bc9e26a..e6ca301f7 100644 --- a/web-app/src/hooks/__tests__/useProviderModels.test.ts +++ b/web-app/src/hooks/__tests__/useProviderModels.test.ts @@ -1,22 +1,19 @@ import { describe, it, expect, beforeEach, vi } from 'vitest' import { renderHook, waitFor } from '@testing-library/react' import { useProviderModels } from '../useProviderModels' +import { WebProvidersService } from '../../services/providers/web' -// Mock the providers service -vi.mock('@/services/providers', () => ({ - fetchModelsFromProvider: vi.fn(), -})) +let fetchModelsSpy: ReturnType -import { fetchModelsFromProvider } from '@/services/providers' -const mockFetchModelsFromProvider = vi.mocked(fetchModelsFromProvider) - -import type { ModelProvider } from '@/types/modelProviders' - -// Mock ModelProvider type -type MockModelProvider = Pick< - ModelProvider, - 'active' | 'provider' | 'base_url' | 'api_key' | 'settings' | 'models' -> +// Local minimal provider type for tests +type MockModelProvider = { + active: boolean + provider: string + base_url?: string + api_key?: string + settings: any[] + models: any[] +} describe('useProviderModels', () => { const mockProvider: MockModelProvider = { @@ -31,7 +28,12 @@ describe('useProviderModels', () => { const mockModels = ['gpt-4', 'gpt-3.5-turbo', 'gpt-4-turbo'] beforeEach(() => { + vi.restoreAllMocks() vi.clearAllMocks() + fetchModelsSpy = vi.spyOn( + WebProvidersService.prototype, + 'fetchModelsFromProvider' + ) }) it('should initialize with empty state', () => { @@ -45,17 +47,17 @@ describe('useProviderModels', () => { it('should not fetch models when provider is undefined', () => { renderHook(() => useProviderModels(undefined)) - expect(mockFetchModelsFromProvider).not.toHaveBeenCalled() + expect(fetchModelsSpy).not.toHaveBeenCalled() }) it('should not fetch models when provider has no base_url', () => { const providerWithoutUrl = { ...mockProvider, base_url: undefined } renderHook(() => useProviderModels(providerWithoutUrl)) - expect(mockFetchModelsFromProvider).not.toHaveBeenCalled() + expect(fetchModelsSpy).not.toHaveBeenCalled() }) it('should fetch and sort models', async () => { - mockFetchModelsFromProvider.mockResolvedValueOnce(mockModels) + fetchModelsSpy.mockResolvedValueOnce(mockModels) const { result } = renderHook(() => useProviderModels(mockProvider)) @@ -66,11 +68,11 @@ describe('useProviderModels', () => { // Should be sorted alphabetically expect(result.current.models).toEqual(['gpt-3.5-turbo', 'gpt-4', 'gpt-4-turbo']) expect(result.current.error).toBe(null) - expect(mockFetchModelsFromProvider).toHaveBeenCalledWith(mockProvider) + expect(fetchModelsSpy).toHaveBeenCalledWith(mockProvider) }) it('should clear models when switching to invalid provider', async () => { - mockFetchModelsFromProvider.mockResolvedValueOnce(mockModels) + fetchModelsSpy.mockResolvedValueOnce(mockModels) const { result, rerender } = renderHook( ({ provider }) => useProviderModels(provider), @@ -96,6 +98,6 @@ describe('useProviderModels', () => { result.current.refetch() - expect(mockFetchModelsFromProvider).not.toHaveBeenCalled() + expect(fetchModelsSpy).not.toHaveBeenCalled() }) }) \ No newline at end of file diff --git a/web-app/src/hooks/useProviderModels.ts b/web-app/src/hooks/useProviderModels.ts index ee5104133..1be17b706 100644 --- a/web-app/src/hooks/useProviderModels.ts +++ b/web-app/src/hooks/useProviderModels.ts @@ -1,5 +1,5 @@ -import { useState, useEffect, useCallback, useRef } from 'react' -import { fetchModelsFromProvider } from '@/services/providers' +import { useState, useEffect, useCallback, useRef, useMemo } from 'react' +import { WebProvidersService } from '../services/providers/web' type UseProviderModelsState = { models: string[] @@ -12,6 +12,7 @@ const modelsCache = new Map() const CACHE_DURATION = 5 * 60 * 1000 // 5 minutes export const useProviderModels = (provider?: ModelProvider): UseProviderModelsState => { + const providersService = useMemo(() => new WebProvidersService(), []) const [models, setModels] = useState([]) const [loading, setLoading] = useState(false) const [error, setError] = useState(null) @@ -50,7 +51,7 @@ export const useProviderModels = (provider?: ModelProvider): UseProviderModelsSt setError(null) try { - const fetchedModels = await fetchModelsFromProvider(provider) + const fetchedModels = await providersService.fetchModelsFromProvider(provider) if (currentRequestId !== requestIdRef.current) return const sortedModels = fetchedModels.sort((a, b) => a.localeCompare(b)) @@ -69,7 +70,7 @@ export const useProviderModels = (provider?: ModelProvider): UseProviderModelsSt } finally { if (currentRequestId === requestIdRef.current) setLoading(false) } - }, [provider]) + }, [provider, providersService]) const refetch = useCallback(() => { if (provider) { diff --git a/web-app/src/services/providers/web.ts b/web-app/src/services/providers/web.ts index 30fe71366..5ac1430da 100644 --- a/web-app/src/services/providers/web.ts +++ b/web-app/src/services/providers/web.ts @@ -138,9 +138,24 @@ export class WebProvidersService implements ProvidersService { }) if (!response.ok) { - throw new Error( - `Cannot connect to ${provider.provider} at ${provider.base_url}. Please check that the service is running and accessible.` - ) + // Provide more specific error messages based on status code + if (response.status === 401) { + throw new Error( + `Authentication failed: API key is required or invalid for ${provider.provider}` + ) + } else if (response.status === 403) { + throw new Error( + `Access forbidden: Check your API key permissions for ${provider.provider}` + ) + } else if (response.status === 404) { + throw new Error( + `Models endpoint not found for ${provider.provider}. Check the base URL configuration.` + ) + } else { + throw new Error( + `Failed to fetch models from ${provider.provider}: ${response.status} ${response.statusText}` + ) + } } const data = await response.json() @@ -170,13 +185,28 @@ export class WebProvidersService implements ProvidersService { } catch (error) { console.error('Error fetching models from provider:', error) + const structuredErrorPrefixes = [ + 'Authentication failed', + 'Access forbidden', + 'Models endpoint not found', + 'Failed to fetch models from' + ] + + if (error instanceof Error && + structuredErrorPrefixes.some(prefix => (error as Error).message.startsWith(prefix))) { + throw new Error(error.message) + } + // Provide helpful error message for any connection errors if (error instanceof Error && error.message.includes('Cannot connect')) { - throw error + throw new Error( + `Cannot connect to ${provider.provider} at ${provider.base_url}. Please check that the service is running and accessible.` + ) } - + + // Generic fallback throw new Error( - `Cannot connect to ${provider.provider} at ${provider.base_url}. Please check that the service is running and accessible.` + `Unexpected error while fetching models from ${provider.provider}: ${error instanceof Error ? error.message : 'Unknown error'}` ) } }