From c4f1da26b3397921360660b947f2ff4d8b9b9077 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Apr 2025 18:44:13 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=84=20fix:=20Avatar=20&=20Error=20Hand?= =?UTF-8?q?ling=20Enhancements=20(#6687)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Ensure safe access to agent capabilities in AgentConfig * fix: don't show agent builder if agents endpoint is not enabled * fix: Improve error logging for MCP tool calls * fix: Enhance error message for MCP tool failures * feat: Add optional spec and iconURL properties to TEndpointOption type * chore: Update condition to use constant for new conversation parameter * feat: Enhance abort error handling with additional endpoint options to properly render error message fields * fix: Throw error instead of returning message for failed MCP tool calls * refactor: separate logic to generate new S3 URLs for expired links * feat: Implement S3 URL refresh for user avatars with error handling * fix: authcontext error in chats where agent chain is used * refactor: streamline balance configuration logic in getBalanceConfig function * fix: enhance icon resolution logic in SpecIcon component * fix: allow null values for spec and iconURL in TEndpointOption type * fix: update balance check to allow null tokenCredits --- api/server/controllers/UserController.js | 18 ++++++ api/server/middleware/abortMiddleware.js | 22 +++++++- api/server/services/Config/getCustomConfig.js | 17 +++--- api/server/services/Files/S3/crud.js | 55 ++++++++++++------- api/server/services/MCP.js | 8 ++- api/typedefs.js | 6 ++ .../Menus/Endpoints/components/SpecIcon.tsx | 4 +- .../Messages/Content/Parts/AgentUpdate.tsx | 22 +++++--- .../src/components/Endpoints/ConvoIconURL.tsx | 2 +- client/src/components/Share/MessageIcon.tsx | 31 ++++++++--- .../SidePanel/Agents/Advanced/AgentChain.tsx | 42 ++++++++------ .../SidePanel/Agents/AgentConfig.tsx | 12 ++-- client/src/components/SidePanel/SidePanel.tsx | 1 + client/src/hooks/Nav/useSideNavLinks.ts | 13 ++++- client/src/hooks/SSE/useEventHandlers.ts | 2 +- config/set-balance.js | 2 +- packages/data-provider/src/types.ts | 2 + 17 files changed, 184 insertions(+), 75 deletions(-) diff --git a/api/server/controllers/UserController.js b/api/server/controllers/UserController.js index a331b8daa..1ed2c4741 100644 --- a/api/server/controllers/UserController.js +++ b/api/server/controllers/UserController.js @@ -1,6 +1,8 @@ +const { FileSources } = require('librechat-data-provider'); const { Balance, getFiles, + updateUser, deleteFiles, deleteConvos, deletePresets, @@ -12,6 +14,7 @@ const User = require('~/models/User'); const { updateUserPluginAuth, deleteUserPluginAuth } = require('~/server/services/PluginService'); const { updateUserPluginsService, deleteUserKey } = require('~/server/services/UserService'); const { verifyEmail, resendVerificationEmail } = require('~/server/services/AuthService'); +const { needsRefresh, getNewS3URL } = require('~/server/services/Files/S3/crud'); const { processDeleteRequest } = require('~/server/services/Files/process'); const { deleteAllSharedLinks } = require('~/models/Share'); const { deleteToolCalls } = require('~/models/ToolCall'); @@ -19,8 +22,23 @@ const { Transaction } = require('~/models/Transaction'); const { logger } = require('~/config'); const getUserController = async (req, res) => { + /** @type {MongoUser} */ const userData = req.user.toObject != null ? req.user.toObject() : { ...req.user }; delete userData.totpSecret; + if (req.app.locals.fileStrategy === FileSources.s3 && userData.avatar) { + const avatarNeedsRefresh = needsRefresh(userData.avatar, 3600); + if (!avatarNeedsRefresh) { + return res.status(200).send(userData); + } + const originalAvatar = userData.avatar; + try { + userData.avatar = await getNewS3URL(userData.avatar); + await updateUser(userData.id, { avatar: userData.avatar }); + } catch (error) { + userData.avatar = originalAvatar; + logger.error('Error getting new S3 URL for avatar:', error); + } + } res.status(200).send(userData); }; diff --git a/api/server/middleware/abortMiddleware.js b/api/server/middleware/abortMiddleware.js index 0053f2bde..ccc4ed043 100644 --- a/api/server/middleware/abortMiddleware.js +++ b/api/server/middleware/abortMiddleware.js @@ -148,6 +148,13 @@ const createAbortController = (req, res, getAbortData, getReqData) => { return { abortController, onStart }; }; +/** + * @param {ServerResponse} res + * @param {ServerRequest} req + * @param {Error | unknown} error + * @param {Partial & { partialText?: string }} data + * @returns { Promise } + */ const handleAbortError = async (res, req, error, data) => { if (error?.message?.includes('base64')) { logger.error('[handleAbortError] Error in base64 encoding', { @@ -178,17 +185,30 @@ const handleAbortError = async (res, req, error, data) => { errorText = `{"type":"${ErrorTypes.NO_SYSTEM_MESSAGES}"}`; } + /** + * @param {string} partialText + * @returns {Promise} + */ const respondWithError = async (partialText) => { + const endpointOption = req.body?.endpointOption; let options = { sender, messageId, conversationId, parentMessageId, text: errorText, - shouldSaveMessage: true, user: req.user.id, + shouldSaveMessage: true, + spec: endpointOption?.spec, + iconURL: endpointOption?.iconURL, + modelLabel: endpointOption?.modelLabel, + model: endpointOption?.modelOptions?.model || req.body?.model, }; + if (req.body?.agent_id) { + options.agent_id = req.body.agent_id; + } + if (partialText) { options = { ...options, diff --git a/api/server/services/Config/getCustomConfig.js b/api/server/services/Config/getCustomConfig.js index 2a154421b..fdd84878e 100644 --- a/api/server/services/Config/getCustomConfig.js +++ b/api/server/services/Config/getCustomConfig.js @@ -31,19 +31,16 @@ async function getCustomConfig() { async function getBalanceConfig() { const isLegacyEnabled = isEnabled(process.env.CHECK_BALANCE); const startBalance = process.env.START_BALANCE; - if (isLegacyEnabled || (startBalance != null && startBalance)) { - /** @type {TCustomConfig['balance']} */ - const config = { - enabled: isLegacyEnabled, - startBalance: startBalance ? parseInt(startBalance, 10) : undefined, - }; - return config; - } + /** @type {TCustomConfig['balance']} */ + const config = { + enabled: isLegacyEnabled, + startBalance: startBalance != null && startBalance ? parseInt(startBalance, 10) : undefined, + }; const customConfig = await getCustomConfig(); if (!customConfig) { - return null; + return config; } - return customConfig?.['balance'] ?? null; + return { ...config, ...(customConfig?.['balance'] ?? {}) }; } /** diff --git a/api/server/services/Files/S3/crud.js b/api/server/services/Files/S3/crud.js index 869664520..1d5768d14 100644 --- a/api/server/services/Files/S3/crud.js +++ b/api/server/services/Files/S3/crud.js @@ -303,6 +303,36 @@ function needsRefresh(signedUrl, bufferSeconds) { } } +/** + * Generates a new URL for an expired S3 URL + * @param {string} currentURL - The current file URL + * @returns {Promise} + */ +async function getNewS3URL(currentURL) { + try { + const s3Key = extractKeyFromS3Url(currentURL); + if (!s3Key) { + return; + } + const keyParts = s3Key.split('/'); + if (keyParts.length < 3) { + return; + } + + const basePath = keyParts[0]; + const userId = keyParts[1]; + const fileName = keyParts.slice(2).join('/'); + + return await getS3URL({ + userId, + fileName, + basePath, + }); + } catch (error) { + logger.error('Error getting new S3 URL:', error); + } +} + /** * Refreshes S3 URLs for an array of files if they're expired or close to expiring * @@ -333,30 +363,15 @@ async function refreshS3FileUrls(files, batchUpdateFiles, bufferSeconds = 3600) continue; } try { - const s3Key = extractKeyFromS3Url(file.filepath); - if (!s3Key) { + const newURL = await getNewS3URL(file.filepath); + if (!newURL) { continue; } - const keyParts = s3Key.split('/'); - if (keyParts.length < 3) { - continue; - } - - const basePath = keyParts[0]; - const userId = keyParts[1]; - const fileName = keyParts.slice(2).join('/'); - - const newUrl = await getS3URL({ - userId, - fileName, - basePath, - }); - filesToUpdate.push({ file_id: file.file_id, - filepath: newUrl, + filepath: newURL, }); - files[i].filepath = newUrl; + files[i].filepath = newURL; } catch (error) { logger.error(`Error refreshing S3 URL for file ${file.file_id}:`, error); } @@ -425,4 +440,6 @@ module.exports = { getS3FileStream, refreshS3FileUrls, refreshS3Url, + needsRefresh, + getNewS3URL, }; diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index 7d3897152..b64194b07 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -69,7 +69,13 @@ async function createMCPTool({ req, toolKey, provider }) { } return result; } catch (error) { - return `${toolName} MCP server tool call failed.`; + logger.error( + `[MCP][User: ${userId}][${serverName}] Error calling "${toolName}" MCP tool:`, + error, + ); + throw new Error( + `"${toolKey}" tool call failed${error?.message ? `: ${error?.message}` : '.'}`, + ); } }; diff --git a/api/typedefs.js b/api/typedefs.js index a89866c07..24dd29a93 100644 --- a/api/typedefs.js +++ b/api/typedefs.js @@ -828,6 +828,12 @@ * @memberof typedefs */ +/** + * @exports TEndpointOption + * @typedef {import('librechat-data-provider').TEndpointOption} TEndpointOption + * @memberof typedefs + */ + /** * @exports TAttachment * @typedef {import('librechat-data-provider').TAttachment} TAttachment diff --git a/client/src/components/Chat/Menus/Endpoints/components/SpecIcon.tsx b/client/src/components/Chat/Menus/Endpoints/components/SpecIcon.tsx index 11948ce2b..4ba29325b 100644 --- a/client/src/components/Chat/Menus/Endpoints/components/SpecIcon.tsx +++ b/client/src/components/Chat/Menus/Endpoints/components/SpecIcon.tsx @@ -20,7 +20,7 @@ const SpecIcon: React.FC = ({ currentSpec, endpointsConfig }) => let Icon: IconType; if (!iconURL.includes('http')) { - Icon = (icons[iconKey] ?? icons.unknown) as IconType; + Icon = (icons[iconURL] ?? icons[iconKey] ?? icons.unknown) as IconType; } else if (iconURL) { return ( = ({ currentSpec, endpointsConfig }) => /> ); } else { - Icon = (icons[endpoint ?? ''] ?? icons.unknown) as IconType; + Icon = (icons[endpoint ?? ''] ?? icons[iconKey] ?? icons.unknown) as IconType; } return ( diff --git a/client/src/components/Chat/Messages/Content/Parts/AgentUpdate.tsx b/client/src/components/Chat/Messages/Content/Parts/AgentUpdate.tsx index 4dca00107..a9610433b 100644 --- a/client/src/components/Chat/Messages/Content/Parts/AgentUpdate.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/AgentUpdate.tsx @@ -1,13 +1,16 @@ import React, { useMemo } from 'react'; import { EModelEndpoint } from 'librechat-data-provider'; +import type { TMessage } from 'librechat-data-provider'; +import MessageIcon from '~/components/Share/MessageIcon'; import { useAgentsMapContext } from '~/Providers'; -import Icon from '~/components/Endpoints/Icon'; +import { useLocalize } from '~/hooks'; interface AgentUpdateProps { currentAgentId: string; } const AgentUpdate: React.FC = ({ currentAgentId }) => { + const localize = useLocalize(); const agentsMap = useAgentsMapContext() || {}; const currentAgent = useMemo(() => agentsMap[currentAgentId], [agentsMap, currentAgentId]); if (!currentAgentId) { @@ -23,14 +26,19 @@ const AgentUpdate: React.FC = ({ currentAgentId }) => {
-
-
{currentAgent?.name}
+
+ {currentAgent?.name || localize('com_ui_agent')} +
); diff --git a/client/src/components/Endpoints/ConvoIconURL.tsx b/client/src/components/Endpoints/ConvoIconURL.tsx index 6ce0b8760..e3fd6bc63 100644 --- a/client/src/components/Endpoints/ConvoIconURL.tsx +++ b/client/src/components/Endpoints/ConvoIconURL.tsx @@ -66,7 +66,7 @@ const ConvoIconURL: React.FC = ({ agentName={agentName} iconURL={endpointIconURL} assistantName={assistantName} - avatar={assistantAvatar ?? agentAvatar} + avatar={assistantAvatar || agentAvatar} /> )} diff --git a/client/src/components/Share/MessageIcon.tsx b/client/src/components/Share/MessageIcon.tsx index 66bb7b74f..0d198bf13 100644 --- a/client/src/components/Share/MessageIcon.tsx +++ b/client/src/components/Share/MessageIcon.tsx @@ -1,5 +1,5 @@ import { useMemo } from 'react'; -import type { TMessage, TPreset, Assistant, Agent } from 'librechat-data-provider'; +import type { TMessage, Assistant, Agent } from 'librechat-data-provider'; import type { TMessageProps } from '~/common'; import MessageEndpointIcon from '../Endpoints/MessageEndpointIcon'; import ConvoIconURL from '~/components/Endpoints/ConvoIconURL'; @@ -14,11 +14,6 @@ export default function MessageIcon( ) { const { message, conversation, assistant, agent } = props; - const assistantName = assistant ? (assistant.name as string | undefined) : ''; - const assistantAvatar = assistant ? (assistant.metadata?.avatar as string | undefined) : ''; - const agentName = agent ? (agent.name as string | undefined) : ''; - const agentAvatar = agent ? (agent.metadata?.avatar as string | undefined) : ''; - const messageSettings = useMemo( () => ({ ...(conversation ?? {}), @@ -33,7 +28,27 @@ export default function MessageIcon( const iconURL = messageSettings.iconURL ?? ''; let endpoint = messageSettings.endpoint; endpoint = getIconEndpoint({ endpointsConfig: undefined, iconURL, endpoint }); - + const assistantName = (assistant ? assistant.name : '') ?? ''; + const assistantAvatar = (assistant ? assistant.metadata?.avatar : '') ?? ''; + const agentName = (agent ? agent.name : '') ?? ''; + const agentAvatar = (agent ? agent?.avatar?.filepath : '') ?? ''; + const avatarURL = useMemo(() => { + let result = ''; + if (assistant) { + result = assistantAvatar; + } else if (agent) { + result = agentAvatar; + } + return result; + }, [assistant, agent, assistantAvatar, agentAvatar]); + console.log('MessageIcon', { + endpoint, + iconURL, + assistantName, + assistantAvatar, + agentName, + agentAvatar, + }); if (message?.isCreatedByUser !== true && iconURL && iconURL.includes('http')) { return ( = ({ field, currentAgentId }) => { label: agent?.name || '', value: agent?.id, icon: ( - ), }) as OptionWithIcon, @@ -88,11 +92,14 @@ const AgentChain: React.FC = ({ field, currentAgentId }) => {
-
@@ -114,11 +121,14 @@ const AgentChain: React.FC = ({ field, currentAgentId }) => { items={selectableAgents} displayValue={getAgentDetails(agentId)?.name ?? ''} SelectIcon={ - } className="flex-1 border-border-heavy" diff --git a/client/src/components/SidePanel/Agents/AgentConfig.tsx b/client/src/components/SidePanel/Agents/AgentConfig.tsx index f914a1428..b5a5200c6 100644 --- a/client/src/components/SidePanel/Agents/AgentConfig.tsx +++ b/client/src/components/SidePanel/Agents/AgentConfig.tsx @@ -53,27 +53,27 @@ export default function AgentConfig({ const agent_id = useWatch({ control, name: 'id' }); const toolsEnabled = useMemo( - () => agentsConfig?.capabilities.includes(AgentCapabilities.tools), + () => agentsConfig?.capabilities?.includes(AgentCapabilities.tools) ?? false, [agentsConfig], ); const actionsEnabled = useMemo( - () => agentsConfig?.capabilities.includes(AgentCapabilities.actions), + () => agentsConfig?.capabilities?.includes(AgentCapabilities.actions) ?? false, [agentsConfig], ); const artifactsEnabled = useMemo( - () => agentsConfig?.capabilities.includes(AgentCapabilities.artifacts) ?? false, + () => agentsConfig?.capabilities?.includes(AgentCapabilities.artifacts) ?? false, [agentsConfig], ); const ocrEnabled = useMemo( - () => agentsConfig?.capabilities.includes(AgentCapabilities.ocr) ?? false, + () => agentsConfig?.capabilities?.includes(AgentCapabilities.ocr) ?? false, [agentsConfig], ); const fileSearchEnabled = useMemo( - () => agentsConfig?.capabilities.includes(AgentCapabilities.file_search) ?? false, + () => agentsConfig?.capabilities?.includes(AgentCapabilities.file_search) ?? false, [agentsConfig], ); const codeEnabled = useMemo( - () => agentsConfig?.capabilities.includes(AgentCapabilities.execute_code) ?? false, + () => agentsConfig?.capabilities?.includes(AgentCapabilities.execute_code) ?? false, [agentsConfig], ); diff --git a/client/src/components/SidePanel/SidePanel.tsx b/client/src/components/SidePanel/SidePanel.tsx index b01a97133..dc2015314 100644 --- a/client/src/components/SidePanel/SidePanel.tsx +++ b/client/src/components/SidePanel/SidePanel.tsx @@ -91,6 +91,7 @@ const SidePanel = ({ keyProvided, endpointType, interfaceConfig, + endpointsConfig, }); const toggleNavVisible = useCallback(() => { diff --git a/client/src/hooks/Nav/useSideNavLinks.ts b/client/src/hooks/Nav/useSideNavLinks.ts index 3b293fe77..885cdecc3 100644 --- a/client/src/hooks/Nav/useSideNavLinks.ts +++ b/client/src/hooks/Nav/useSideNavLinks.ts @@ -8,7 +8,7 @@ import { EModelEndpoint, Permissions, } from 'librechat-data-provider'; -import type { TConfig, TInterfaceConfig } from 'librechat-data-provider'; +import type { TConfig, TInterfaceConfig, TEndpointsConfig } from 'librechat-data-provider'; import type { NavLink } from '~/common'; import AgentPanelSwitch from '~/components/SidePanel/Agents/AgentPanelSwitch'; import BookmarkPanel from '~/components/SidePanel/Bookmarks/BookmarkPanel'; @@ -27,6 +27,7 @@ export default function useSideNavLinks({ endpoint, endpointType, interfaceConfig, + endpointsConfig, }: { hidePanel: () => void; assistants?: TConfig | null; @@ -35,6 +36,7 @@ export default function useSideNavLinks({ endpoint?: EModelEndpoint | null; endpointType?: EModelEndpoint | null; interfaceConfig: Partial; + endpointsConfig: TEndpointsConfig; }) { const hasAccessToPrompts = useHasAccess({ permissionType: PermissionTypes.PROMPTS, @@ -70,7 +72,13 @@ export default function useSideNavLinks({ }); } - if (hasAccessToAgents && hasAccessToCreateAgents && agents && agents.disableBuilder !== true) { + if ( + endpointsConfig?.[EModelEndpoint.agents] && + hasAccessToAgents && + hasAccessToCreateAgents && + agents && + agents.disableBuilder !== true + ) { links.push({ title: 'com_sidepanel_agent_builder', label: '', @@ -133,6 +141,7 @@ export default function useSideNavLinks({ return links; }, [ + endpointsConfig?.[EModelEndpoint.agents], interfaceConfig.parameters, keyProvided, assistants, diff --git a/client/src/hooks/SSE/useEventHandlers.ts b/client/src/hooks/SSE/useEventHandlers.ts index 63e2e4aa3..e8dd8a1fd 100644 --- a/client/src/hooks/SSE/useEventHandlers.ts +++ b/client/src/hooks/SSE/useEventHandlers.ts @@ -598,7 +598,7 @@ export default function useEventHandlers({ }); setMessages([...messages, userMessage, errorResponse]); - if (receivedConvoId && paramId === 'new' && newConversation) { + if (receivedConvoId && paramId === Constants.NEW_CONVO && newConversation) { newConversation({ template: { conversationId: receivedConvoId }, preset: tPresetSchema.parse(submission.conversation), diff --git a/config/set-balance.js b/config/set-balance.js index c28586ad2..6fb3b8ca9 100644 --- a/config/set-balance.js +++ b/config/set-balance.js @@ -98,7 +98,7 @@ const Balance = require('~/models/Balance'); } // Check the result - if (!result?.tokenCredits) { + if (result?.tokenCredits == null) { console.red('Error: Something went wrong while updating the balance!'); console.error(result); silentExit(1); diff --git a/packages/data-provider/src/types.ts b/packages/data-provider/src/types.ts index 2f16db171..f278810a5 100644 --- a/packages/data-provider/src/types.ts +++ b/packages/data-provider/src/types.ts @@ -18,6 +18,8 @@ export type TMessages = TMessage[]; /* TODO: Cleanup EndpointOption types */ export type TEndpointOption = { + spec?: string | null; + iconURL?: string | null; endpoint: EModelEndpoint; endpointType?: EModelEndpoint; modelDisplayLabel?: string;