🕵️♂️ fix: Handle 404 errors on agent queries for favorites#11587
Open
ethanlaj wants to merge 1 commit intodanny-avila:devfrom
Open
🕵️♂️ fix: Handle 404 errors on agent queries for favorites#11587ethanlaj wants to merge 1 commit intodanny-avila:devfrom
ethanlaj wants to merge 1 commit intodanny-avila:devfrom
Conversation
ethanlaj
force-pushed
the
fix-agent-delete-logic
branch
from
d1c5c86 to
3491734
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes 404 errors that occur when agent favorites reference deleted or inaccessible agents, which previously caused an infinite skeleton loading state in the favorites list and agent marketplace.
Changes:
- Added a new
AgentQueryResultdiscriminated type to handle both successful and failed agent queries - Modified the query function in
FavoritesList.tsxto catch 404 errors and return{found: false}instead of throwing - Updated the
combinedAgentsMaplogic to filter out agents wherefoundis false - Added comprehensive unit tests for the new error handling logic
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
client/src/common/agents-types.ts |
Defines new AgentQueryResult type to represent query outcomes for missing/deleted agents |
client/src/components/Nav/Favorites/FavoritesList.tsx |
Implements 404 error handling in agent queries and filters out unfound agents from the combined map |
client/src/components/Nav/Favorites/tests/FavoritesList.spec.tsx |
Adds unit tests for the query result type behavior and combinedAgentsMap filtering logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } from 'librechat-data-provider'; | ||
| import type { OptionWithIcon, ExtendedFile } from './types'; | ||
|
|
||
| export type AgentQueryResult = { found: boolean; agent?: Agent }; |
There was a problem hiding this comment.
The AgentQueryResult type allows agent to be optional even when found is true, which isn't type-safe. Consider using a discriminated union type instead:
export type AgentQueryResult =
| { found: true; agent: Agent }
| { found: false };This would eliminate the need for non-null assertions (!) in FavoritesList.tsx line 219 and provide better type safety.
Suggested change
| export type AgentQueryResult = { found: boolean; agent?: Agent }; | |
| export type AgentQueryResult = | |
| | { found: true; agent: Agent } | |
| | { found: false }; |
Comment on lines
+132
to
+192
| describe('AgentQueryResult type behavior', () => { | ||
| it('should handle found agent result correctly', async () => { | ||
| const mockAgent: t.Agent = { | ||
| id: 'agent-123', | ||
| name: 'Test Agent', | ||
| author: 'test-author', | ||
| } as t.Agent; | ||
|
|
||
| (dataService.getAgentById as jest.Mock).mockResolvedValueOnce(mockAgent); | ||
|
|
||
| const result = await dataService.getAgentById({ agent_id: 'agent-123' }); | ||
|
|
||
| // Simulate the query function logic | ||
| const queryResult: AgentQueryResult = { found: true, agent: result }; | ||
|
|
||
| expect(queryResult.found).toBe(true); | ||
| expect(queryResult.agent).toEqual(mockAgent); | ||
| }); | ||
|
|
||
| it('should handle not found agent result correctly', async () => { | ||
| const error = { response: { status: 404 } }; | ||
| (dataService.getAgentById as jest.Mock).mockRejectedValueOnce(error); | ||
|
|
||
| // Simulate the query function logic from FavoritesList | ||
| let queryResult: AgentQueryResult | undefined; | ||
| try { | ||
| await dataService.getAgentById({ agent_id: 'deleted-agent' }); | ||
| } catch (err) { | ||
| if (err && typeof err === 'object' && 'response' in err) { | ||
| const axiosError = err as { response?: { status?: number } }; | ||
| if (axiosError.response?.status === 404) { | ||
| queryResult = { found: false }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| expect(queryResult).toEqual({ found: false }); | ||
| }); | ||
|
|
||
| it('should rethrow non-404 errors', async () => { | ||
| const error = { response: { status: 500 }, message: 'Server error' }; | ||
| (dataService.getAgentById as jest.Mock).mockRejectedValueOnce(error); | ||
|
|
||
| // Simulate the query function logic from FavoritesList | ||
| let caughtError; | ||
| try { | ||
| await dataService.getAgentById({ agent_id: 'agent-123' }); | ||
| } catch (err) { | ||
| if (err && typeof err === 'object' && 'response' in err) { | ||
| const axiosError = err as { response?: { status?: number } }; | ||
| if (axiosError.response?.status === 404) { | ||
| // Would return { found: false }, but this is a 500 | ||
| } else { | ||
| caughtError = err; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| expect(caughtError).toEqual(error); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The tests in the "AgentQueryResult type behavior" describe block (lines 132-192) test the logic in isolation but don't actually test the component's integration with the useQueries hook. Consider adding a test that renders the FavoritesList component with favorites containing missing agent IDs and verifies that:
- The component doesn't show infinite loading skeleton
- The missing agents are correctly excluded from the rendered favorites
- Valid agents are still rendered correctly
This would better validate the bug fix described in the PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Summary
Danny has already handled agent deletion updating user favorites (in #11466), however, does not handle the following cases:
When an agent favorite doesn't exist, queries for the agent fail with a 404 error, and cause an infinite skeleton load for favorites + agent marketplace
Change Type
Testing
Deleted an agent, favorited a different agent, refreshed page, loading skeleton of favorites no longer showing.
Test Configuration:
Checklist
Please delete any irrelevant options.