From 0c5e66bccbee9730b8c4bc164efa99a754615e2d Mon Sep 17 00:00:00 2001 From: lyzno1 <92089059+lyzno1@users.noreply.github.com> Date: Mon, 11 Aug 2025 11:57:06 +0800 Subject: [PATCH] fix: unified error handling for GotoAnything search actions (#23715) --- .../search-error-handling.test.ts | 197 ++++++++++++++++++ .../components/goto-anything/actions/app.tsx | 27 ++- .../components/goto-anything/actions/index.ts | 8 +- .../goto-anything/actions/knowledge.tsx | 28 ++- .../goto-anything/actions/plugin.tsx | 38 ++-- .../goto-anything/actions/workflow-nodes.tsx | 28 +-- .../workflow/hooks/use-workflow-search.tsx | 26 ++- 7 files changed, 285 insertions(+), 67 deletions(-) create mode 100644 web/__tests__/goto-anything/search-error-handling.test.ts diff --git a/web/__tests__/goto-anything/search-error-handling.test.ts b/web/__tests__/goto-anything/search-error-handling.test.ts new file mode 100644 index 000000000..d2fd921e1 --- /dev/null +++ b/web/__tests__/goto-anything/search-error-handling.test.ts @@ -0,0 +1,197 @@ +/** + * Test GotoAnything search error handling mechanisms + * + * Main validations: + * 1. @plugin search error handling when API fails + * 2. Regular search (without @prefix) error handling when API fails + * 3. Verify consistent error handling across different search types + * 4. Ensure errors don't propagate to UI layer causing "search failed" + */ + +import { Actions, searchAnything } from '@/app/components/goto-anything/actions' +import { postMarketplace } from '@/service/base' +import { fetchAppList } from '@/service/apps' +import { fetchDatasets } from '@/service/datasets' + +// Mock API functions +jest.mock('@/service/base', () => ({ + postMarketplace: jest.fn(), +})) + +jest.mock('@/service/apps', () => ({ + fetchAppList: jest.fn(), +})) + +jest.mock('@/service/datasets', () => ({ + fetchDatasets: jest.fn(), +})) + +const mockPostMarketplace = postMarketplace as jest.MockedFunction +const mockFetchAppList = fetchAppList as jest.MockedFunction +const mockFetchDatasets = fetchDatasets as jest.MockedFunction + +describe('GotoAnything Search Error Handling', () => { + beforeEach(() => { + jest.clearAllMocks() + // Suppress console.warn for clean test output + jest.spyOn(console, 'warn').mockImplementation(() => { + // Suppress console.warn for clean test output + }) + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + describe('@plugin search error handling', () => { + it('should return empty array when API fails instead of throwing error', async () => { + // Mock marketplace API failure (403 permission denied) + mockPostMarketplace.mockRejectedValue(new Error('HTTP 403: Forbidden')) + + const pluginAction = Actions.plugin + + // Directly call plugin action's search method + const result = await pluginAction.search('@plugin', 'test', 'en') + + // Should return empty array instead of throwing error + expect(result).toEqual([]) + expect(mockPostMarketplace).toHaveBeenCalledWith('/plugins/search/advanced', { + body: { + page: 1, + page_size: 10, + query: 'test', + type: 'plugin', + }, + }) + }) + + it('should return empty array when user has no plugin data', async () => { + // Mock marketplace returning empty data + mockPostMarketplace.mockResolvedValue({ + data: { plugins: [] }, + }) + + const pluginAction = Actions.plugin + const result = await pluginAction.search('@plugin', '', 'en') + + expect(result).toEqual([]) + }) + + it('should return empty array when API returns unexpected data structure', async () => { + // Mock API returning unexpected data structure + mockPostMarketplace.mockResolvedValue({ + data: null, + }) + + const pluginAction = Actions.plugin + const result = await pluginAction.search('@plugin', 'test', 'en') + + expect(result).toEqual([]) + }) + }) + + describe('Other search types error handling', () => { + it('@app search should return empty array when API fails', async () => { + // Mock app API failure + mockFetchAppList.mockRejectedValue(new Error('API Error')) + + const appAction = Actions.app + const result = await appAction.search('@app', 'test', 'en') + + expect(result).toEqual([]) + }) + + it('@knowledge search should return empty array when API fails', async () => { + // Mock knowledge API failure + mockFetchDatasets.mockRejectedValue(new Error('API Error')) + + const knowledgeAction = Actions.knowledge + const result = await knowledgeAction.search('@knowledge', 'test', 'en') + + expect(result).toEqual([]) + }) + }) + + describe('Unified search entry error handling', () => { + it('regular search (without @prefix) should return successful results even when partial APIs fail', async () => { + // Set app and knowledge success, plugin failure + mockFetchAppList.mockResolvedValue({ data: [], has_more: false, limit: 10, page: 1, total: 0 }) + mockFetchDatasets.mockResolvedValue({ data: [], has_more: false, limit: 10, page: 1, total: 0 }) + mockPostMarketplace.mockRejectedValue(new Error('Plugin API failed')) + + const result = await searchAnything('en', 'test') + + // Should return successful results even if plugin search fails + expect(result).toEqual([]) + expect(console.warn).toHaveBeenCalledWith('Plugin search failed:', expect.any(Error)) + }) + + it('@plugin dedicated search should return empty array when API fails', async () => { + // Mock plugin API failure + mockPostMarketplace.mockRejectedValue(new Error('Plugin service unavailable')) + + const pluginAction = Actions.plugin + const result = await searchAnything('en', '@plugin test', pluginAction) + + // Should return empty array instead of throwing error + expect(result).toEqual([]) + }) + + it('@app dedicated search should return empty array when API fails', async () => { + // Mock app API failure + mockFetchAppList.mockRejectedValue(new Error('App service unavailable')) + + const appAction = Actions.app + const result = await searchAnything('en', '@app test', appAction) + + expect(result).toEqual([]) + }) + }) + + describe('Error handling consistency validation', () => { + it('all search types should return empty array when encountering errors', async () => { + // Mock all APIs to fail + mockPostMarketplace.mockRejectedValue(new Error('Plugin API failed')) + mockFetchAppList.mockRejectedValue(new Error('App API failed')) + mockFetchDatasets.mockRejectedValue(new Error('Dataset API failed')) + + const actions = [ + { name: '@plugin', action: Actions.plugin }, + { name: '@app', action: Actions.app }, + { name: '@knowledge', action: Actions.knowledge }, + ] + + for (const { name, action } of actions) { + const result = await action.search(name, 'test', 'en') + expect(result).toEqual([]) + } + }) + }) + + describe('Edge case testing', () => { + it('empty search term should be handled properly', async () => { + mockPostMarketplace.mockResolvedValue({ data: { plugins: [] } }) + + const result = await searchAnything('en', '@plugin ', Actions.plugin) + expect(result).toEqual([]) + }) + + it('network timeout should be handled correctly', async () => { + const timeoutError = new Error('Network timeout') + timeoutError.name = 'TimeoutError' + + mockPostMarketplace.mockRejectedValue(timeoutError) + + const result = await searchAnything('en', '@plugin test', Actions.plugin) + expect(result).toEqual([]) + }) + + it('JSON parsing errors should be handled correctly', async () => { + const parseError = new SyntaxError('Unexpected token in JSON') + mockPostMarketplace.mockRejectedValue(parseError) + + const result = await searchAnything('en', '@plugin test', Actions.plugin) + expect(result).toEqual([]) + }) + }) +}) diff --git a/web/app/components/goto-anything/actions/app.tsx b/web/app/components/goto-anything/actions/app.tsx index 9d3834f1b..6fcefd353 100644 --- a/web/app/components/goto-anything/actions/app.tsx +++ b/web/app/components/goto-anything/actions/app.tsx @@ -38,16 +38,21 @@ export const appAction: ActionItem = { title: 'Search Applications', description: 'Search and navigate to your applications', // action, - search: async (_, searchTerm = '', locale) => { - const response = (await fetchAppList({ - url: 'apps', - params: { - page: 1, - name: searchTerm, - }, - })) - const apps = response.data || [] - - return parser(apps) + search: async (_, searchTerm = '', _locale) => { + try { + const response = await fetchAppList({ + url: 'apps', + params: { + page: 1, + name: searchTerm, + }, + }) + const apps = response?.data || [] + return parser(apps) + } + catch (error) { + console.warn('App search failed:', error) + return [] + } }, } diff --git a/web/app/components/goto-anything/actions/index.ts b/web/app/components/goto-anything/actions/index.ts index d8d99a2b5..87369784a 100644 --- a/web/app/components/goto-anything/actions/index.ts +++ b/web/app/components/goto-anything/actions/index.ts @@ -18,7 +18,13 @@ export const searchAnything = async ( ): Promise => { if (actionItem) { const searchTerm = query.replace(actionItem.key, '').replace(actionItem.shortcut, '').trim() - return await actionItem.search(query, searchTerm, locale) + try { + return await actionItem.search(query, searchTerm, locale) + } + catch (error) { + console.warn(`Search failed for ${actionItem.key}:`, error) + return [] + } } if (query.startsWith('@')) diff --git a/web/app/components/goto-anything/actions/knowledge.tsx b/web/app/components/goto-anything/actions/knowledge.tsx index 63619332c..832f4f82f 100644 --- a/web/app/components/goto-anything/actions/knowledge.tsx +++ b/web/app/components/goto-anything/actions/knowledge.tsx @@ -35,16 +35,22 @@ export const knowledgeAction: ActionItem = { title: 'Search Knowledge Bases', description: 'Search and navigate to your knowledge bases', // action, - search: async (_, searchTerm = '', locale) => { - const response = await fetchDatasets({ - url: '/datasets', - params: { - page: 1, - limit: 10, - keyword: searchTerm, - }, - }) - - return parser(response.data) + search: async (_, searchTerm = '', _locale) => { + try { + const response = await fetchDatasets({ + url: '/datasets', + params: { + page: 1, + limit: 10, + keyword: searchTerm, + }, + }) + const datasets = response?.data || [] + return parser(datasets) + } + catch (error) { + console.warn('Knowledge search failed:', error) + return [] + } }, } diff --git a/web/app/components/goto-anything/actions/plugin.tsx b/web/app/components/goto-anything/actions/plugin.tsx index f091f2593..69edbc3cc 100644 --- a/web/app/components/goto-anything/actions/plugin.tsx +++ b/web/app/components/goto-anything/actions/plugin.tsx @@ -24,18 +24,30 @@ export const pluginAction: ActionItem = { title: 'Search Plugins', description: 'Search and navigate to your plugins', search: async (_, searchTerm = '', locale) => { - const response = await postMarketplace<{ data: PluginsFromMarketplaceResponse }>('/plugins/search/advanced', { - body: { - page: 1, - page_size: 10, - query: searchTerm, - type: 'plugin', - }, - }) - const list = (response.data.plugins || []).map(plugin => ({ - ...plugin, - icon: getPluginIconInMarketplace(plugin), - })) - return parser(list, locale!) + try { + const response = await postMarketplace<{ data: PluginsFromMarketplaceResponse }>('/plugins/search/advanced', { + body: { + page: 1, + page_size: 10, + query: searchTerm, + type: 'plugin', + }, + }) + + if (!response?.data?.plugins) { + console.warn('Plugin search: Unexpected response structure', response) + return [] + } + + const list = response.data.plugins.map(plugin => ({ + ...plugin, + icon: getPluginIconInMarketplace(plugin), + })) + return parser(list, locale!) + } + catch (error) { + console.warn('Plugin search failed:', error) + return [] + } }, } diff --git a/web/app/components/goto-anything/actions/workflow-nodes.tsx b/web/app/components/goto-anything/actions/workflow-nodes.tsx index a93f3405c..29b6517be 100644 --- a/web/app/components/goto-anything/actions/workflow-nodes.tsx +++ b/web/app/components/goto-anything/actions/workflow-nodes.tsx @@ -1,6 +1,4 @@ import type { ActionItem } from './types' -import { BoltIcon } from '@heroicons/react/24/outline' -import i18n from 'i18next' // Create the workflow nodes action export const workflowNodesAction: ActionItem = { @@ -12,32 +10,14 @@ export const workflowNodesAction: ActionItem = { search: async (_, searchTerm = '', locale) => { try { // Use the searchFn if available (set by useWorkflowSearch hook) - if (workflowNodesAction.searchFn) { - // searchFn already returns SearchResult[] type, no need to use parser + if (workflowNodesAction.searchFn) return workflowNodesAction.searchFn(searchTerm) - } - - // If not in workflow context or search function not registered - if (!searchTerm.trim()) { - return [{ - id: 'help', - title: i18n.t('app.gotoAnything.actions.searchWorkflowNodes', { lng: locale }), - description: i18n.t('app.gotoAnything.actions.searchWorkflowNodesHelp', { lng: locale }), - type: 'workflow-node', - path: '#', - data: {} as any, - icon: ( -
- -
- ), - }] - } + // If not in workflow context, return empty array return [] } - catch (error) { - console.error('Error searching workflow nodes:', error) + catch (error) { + console.warn('Workflow nodes search failed:', error) return [] } }, diff --git a/web/app/components/workflow/hooks/use-workflow-search.tsx b/web/app/components/workflow/hooks/use-workflow-search.tsx index fa5994cba..b512d3d14 100644 --- a/web/app/components/workflow/hooks/use-workflow-search.tsx +++ b/web/app/components/workflow/hooks/use-workflow-search.tsx @@ -46,9 +46,9 @@ export const useWorkflowSearch = () => { // Create search function for workflow nodes const searchWorkflowNodes = useCallback((query: string) => { - if (!searchableNodes.length || !query.trim()) return [] + if (!searchableNodes.length) return [] - const searchTerm = query.toLowerCase() + const searchTerm = query.toLowerCase().trim() const results = searchableNodes .map((node) => { @@ -58,11 +58,18 @@ export const useWorkflowSearch = () => { let score = 0 - if (titleMatch.startsWith(searchTerm)) score += 100 - else if (titleMatch.includes(searchTerm)) score += 50 - else if (typeMatch === searchTerm) score += 80 - else if (typeMatch.includes(searchTerm)) score += 30 - else if (descMatch.includes(searchTerm)) score += 20 + // If no search term, show all nodes with base score + if (!searchTerm) { + score = 1 + } + else { + // Score based on search relevance + if (titleMatch.startsWith(searchTerm)) score += 100 + else if (titleMatch.includes(searchTerm)) score += 50 + else if (typeMatch === searchTerm) score += 80 + else if (typeMatch.includes(searchTerm)) score += 30 + else if (descMatch.includes(searchTerm)) score += 20 + } return score > 0 ? { @@ -89,6 +96,11 @@ export const useWorkflowSearch = () => { }) .filter((node): node is NonNullable => node !== null) .sort((a, b) => { + // If no search term, sort alphabetically + if (!searchTerm) + return a.title.localeCompare(b.title) + + // Sort by relevance when searching const aTitle = a.title.toLowerCase() const bTitle = b.title.toLowerCase()