diff --git a/api/server/routes/__tests__/share.spec.js b/api/server/routes/__tests__/share.spec.js index 5f7dc7c5bd..8e30f41507 100644 --- a/api/server/routes/__tests__/share.spec.js +++ b/api/server/routes/__tests__/share.spec.js @@ -5,6 +5,7 @@ const mongoose = require('mongoose'); const mockGetSharedLinkExpiration = jest.fn(); const mockGrantCreationPermissions = jest.fn(); const mockUpdateSharedLinkPermissionsExpiration = jest.fn(); +const mockRecordShareLinkRejection = jest.fn(); const mockSharedLinksAccess = jest.fn((_req, _res, next) => next()); const mockSharedLinkConfigMiddleware = jest.fn((_req, _res, next) => next()); let mockShareTenantId; @@ -103,6 +104,8 @@ jest.mock('@librechat/api', () => ({ (...args) => mockGetSharedLangfuseSessionUrl(...args), ), + recordShareLinkRejection: (...args) => mockRecordShareLinkRejection(...args), + traceIdForMessage: (messageId) => `trace-${messageId}`, isContentFilterError: jest.fn( (error) => error?.code === 'content_filter_block' || error?.code === 'content_filter_uninspectable', @@ -113,7 +116,10 @@ jest.mock('@librechat/data-schemas', () => ({ logger: { error: jest.fn(), warn: jest.fn() }, createTempChatExpirationDate: jest.fn(() => new Date('2030-01-01T00:00:00.000Z')), runAsSystem: jest.fn((fn) => fn()), - tenantStorage: { run: jest.fn((_ctx, fn) => fn()) }, + tenantStorage: { + getStore: jest.fn(() => ({ requestId: 'request-123' })), + run: jest.fn((_ctx, fn) => fn()), + }, SYSTEM_TENANT_ID: '__SYSTEM__', SystemCapabilities: { ACCESS_ADMIN: 'access:admin' }, })); @@ -832,7 +838,31 @@ describe('share routes', () => { const response = await request(buildApp()).post('/api/share/convo-123').send({}); expect(response.status).toBe(409); - expect(response.body).toEqual({ message: 'Share already exists' }); + expect(response.body).toEqual({ message: 'Share already exists', code: 'SHARE_EXISTS' }); + }); + + it.each([ + ['TARGET_MESSAGE_NOT_FOUND', 'Target message not found', 'trace-msg-123'], + ['NO_MESSAGES', 'No messages to share', 'trace-msg-123'], + ])('returns and records the %s create rejection', async (code, message, traceId) => { + mockGetSharedLinkExpiration.mockResolvedValue(activeExpiration); + createSharedLink.mockRejectedValue(Object.assign(new Error(message), { code })); + + const response = await request(buildApp()) + .post('/api/share/convo-123') + .send({ targetMessageId: 'msg-123' }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ message, code }); + expect(mockRecordShareLinkRejection).toHaveBeenCalledWith('create', code); + expect(logger.warn).toHaveBeenCalledWith('[share] Shared link publication rejected', { + event: 'share_link_rejected', + operation: 'create', + code, + request_id: 'request-123', + trace_id: traceId, + }); + expect(logger.error).not.toHaveBeenCalledWith('Error creating shared link:', expect.anything()); }); it('returns a raw-free 400 when the exact create snapshot fails policy preflight', async () => { @@ -1332,7 +1362,35 @@ describe('share routes', () => { const response = await request(buildApp()).patch('/api/share/share-123').send({}); expect(response.status).toBe(404); - expect(response.body).toEqual({ message: 'Share not found' }); + expect(response.body).toEqual({ message: 'Share not found', code: 'SHARE_NOT_FOUND' }); + }); + + it('returns and records a missing-tail update rejection', async () => { + mongoose.models.SharedLink.findOne.mockReturnValue(lean({ conversationId: 'convo-123' })); + mockGetSharedLinkExpiration.mockResolvedValue(activeExpiration); + updateSharedLink.mockRejectedValue( + Object.assign(new Error('Target message not found'), { + code: 'TARGET_MESSAGE_NOT_FOUND', + }), + ); + + const response = await request(buildApp()) + .patch('/api/share/share-123') + .send({ targetMessageId: 'msg-123' }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ + message: 'Target message not found', + code: 'TARGET_MESSAGE_NOT_FOUND', + }); + expect(mockRecordShareLinkRejection).toHaveBeenCalledWith('update', 'TARGET_MESSAGE_NOT_FOUND'); + expect(logger.warn).toHaveBeenCalledWith('[share] Shared link publication rejected', { + event: 'share_link_rejected', + operation: 'update', + code: 'TARGET_MESSAGE_NOT_FOUND', + request_id: 'request-123', + trace_id: 'trace-msg-123', + }); }); it('allows deleting existing shares without CREATE permission gate', async () => { @@ -1466,7 +1524,10 @@ describe('share fork route', () => { .send({ targetMessageIndex: 3, shareRevision: '2026-01-01T00:00:00.000Z' }); expect(response.status).toBe(409); - expect(response.body).toEqual({ message: 'Shared link was updated' }); + expect(response.body).toEqual({ + message: 'Shared link was updated', + code: 'SHARE_REVISION_MISMATCH', + }); }); }); diff --git a/api/server/routes/share.js b/api/server/routes/share.js index d9278a65df..bbfb3407ad 100644 --- a/api/server/routes/share.js +++ b/api/server/routes/share.js @@ -21,6 +21,8 @@ const { MAX_SHARED_LINK_SEARCH_LENGTH, createSharedLinkConfigMiddleware, createSharedLangfuseSessionResolver, + recordShareLinkRejection, + traceIdForMessage, } = require('@librechat/api'); const { logger, @@ -71,10 +73,30 @@ const SHARE_SERVICE_ERROR_STATUS = { SHARE_REVISION_MISMATCH: 409, }; -const sendShareServiceError = (res, error, fallbackMessage) => { +const OBSERVABLE_SHARE_REJECTIONS = new Set(['TARGET_MESSAGE_NOT_FOUND', 'NO_MESSAGES']); + +const sendShareServiceError = (req, res, error, fallbackMessage, operation) => { const status = SHARE_SERVICE_ERROR_STATUS[error?.code] ?? 500; const message = status === 500 ? fallbackMessage : error.message; - return res.status(status).json({ message }); + const code = status === 500 ? undefined : error.code; + + if (OBSERVABLE_SHARE_REJECTIONS.has(code)) { + const targetMessageId = req.body?.targetMessageId; + const requestId = tenantStorage.getStore()?.requestId ?? req.requestId; + const traceId = + typeof targetMessageId === 'string' ? traceIdForMessage(targetMessageId) : undefined; + + recordShareLinkRejection(operation, code); + logger.warn('[share] Shared link publication rejected', { + event: 'share_link_rejected', + operation, + code, + ...(requestId && { request_id: requestId }), + ...(traceId && { trace_id: traceId }), + }); + } + + return res.status(status).json({ message, ...(code && { code }) }); }; const checkSharedLinksAccess = generateCheckAccess({ @@ -417,7 +439,7 @@ if (allowSharedLinks) { if (error?.code !== 'SHARE_REVISION_MISMATCH') { logger.error('Error forking shared conversation:', error); } - return sendShareServiceError(res, error, 'Error forking shared conversation'); + return sendShareServiceError(req, res, error, 'Error forking shared conversation', 'fork'); } }, ); @@ -649,8 +671,10 @@ router.post( if (isContentFilterError(error)) { return res.status(error.statusCode).json(error.body); } - logger.error('Error creating shared link:', error); - return sendShareServiceError(res, error, 'Error creating shared link'); + if (!OBSERVABLE_SHARE_REJECTIONS.has(error?.code)) { + logger.error('Error creating shared link:', error); + } + return sendShareServiceError(req, res, error, 'Error creating shared link', 'create'); } }, ); @@ -720,8 +744,10 @@ router.patch( if (isContentFilterError(error)) { return res.status(error.statusCode).json(error.body); } - logger.error('Error updating shared link:', error); - return sendShareServiceError(res, error, 'Error updating shared link'); + if (!OBSERVABLE_SHARE_REJECTIONS.has(error?.code)) { + logger.error('Error updating shared link:', error); + } + return sendShareServiceError(req, res, error, 'Error updating shared link', 'update'); } }, ); diff --git a/client/src/components/Conversations/ConvoOptions/ShareButton.tsx b/client/src/components/Conversations/ConvoOptions/ShareButton.tsx index adb2c5438e..8382a6cc82 100644 --- a/client/src/components/Conversations/ConvoOptions/ShareButton.tsx +++ b/client/src/components/Conversations/ConvoOptions/ShareButton.tsx @@ -1,6 +1,8 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useCallback } from 'react'; import { useRecoilValue } from 'recoil'; import { QRCodeSVG } from 'qrcode.react'; +import { useQueryClient } from '@tanstack/react-query'; +import { QueryKeys, dataService } from 'librechat-data-provider'; import { useGetSharedLinkQuery } from 'librechat-data-provider/react-query'; import { ESide, @@ -14,7 +16,7 @@ import { OGDialogContent, OGDialogDescription, } from '@librechat/client'; -import { useLatestMessageId } from '~/hooks/Messages/useLatestMessage'; +import { useGetLatestMessage, useLatestMessageId } from '~/hooks/Messages/useLatestMessage'; import SharedLinkCopyButton from './SharedLinkCopyButton'; import { useGetStartupConfig } from '~/data-provider'; import SharedLinkButton from './SharedLinkButton'; @@ -22,6 +24,12 @@ import { buildShareLinkUrl } from '~/utils'; import { useLocalize } from '~/hooks'; import store from '~/store'; +type ShareTargetErrorCode = 'TARGET_MESSAGE_NOT_FOUND' | 'NO_MESSAGES'; + +const createShareTargetError = ( + code: ShareTargetErrorCode, +): Error & { code: ShareTargetErrorCode } => Object.assign(new Error(code), { code }); + export default function ShareButton({ conversationId, open, @@ -38,16 +46,42 @@ export default function ShareButton({ const localize = useLocalize(); const { data: startupConfig } = useGetStartupConfig(); const canSnapshotFiles = startupConfig?.sharedLinksSnapshotFilesEnabled === true; + const queryClient = useQueryClient(); const [showQR, setShowQR] = useState(true); const [sharedLink, setSharedLink] = useState(''); const [snapshotFiles, setSnapshotFiles] = useState(true); const shareFilesSwitchRef = React.useRef(null); const activeConversationId = useRecoilValue(store.conversationIdByIndex(0)); const activeLatestMessageId = useLatestMessageId(0); + const getActiveLatestMessage = useGetLatestMessage(0); /** `useLatestMessageId` resolves the active pane's branch tail, so it only describes * this dialog's conversation when the two match. Sharing another conversation from * the list sends no target, which shares it in full instead of a foreign message. */ - const latestMessageId = activeConversationId === conversationId ? activeLatestMessageId : null; + const isActiveConversation = activeConversationId === conversationId; + const latestMessageId = isActiveConversation ? activeLatestMessageId : null; + const resolveTargetMessageId = useCallback(async (): Promise => { + let selectedMessageId = getActiveLatestMessage()?.messageId ?? latestMessageId; + + if (!selectedMessageId) { + await queryClient.fetchQuery( + [QueryKeys.messages, conversationId], + () => dataService.getMessagesByConvoId(conversationId), + { staleTime: 0 }, + ); + selectedMessageId = getActiveLatestMessage()?.messageId ?? null; + } + + if (!selectedMessageId) { + throw createShareTargetError('NO_MESSAGES'); + } + + const persistedMessages = await dataService.getMessageById(conversationId, selectedMessageId); + if (!persistedMessages.some((message) => message.messageId === selectedMessageId)) { + throw createShareTargetError('TARGET_MESSAGE_NOT_FOUND'); + } + + return selectedMessageId; + }, [conversationId, getActiveLatestMessage, latestMessageId, queryClient]); const { data: share, isLoading } = useGetSharedLinkQuery(conversationId); const shareId = share?.shareId ?? ''; @@ -74,6 +108,7 @@ export default function ShareButton({ share={share} conversationId={conversationId} targetMessageId={latestMessageId ?? undefined} + resolveTargetMessageId={isActiveConversation ? resolveTargetMessageId : undefined} showQR={showQR} setShowQR={setShowQR} sharedLink={sharedLink} diff --git a/client/src/components/Conversations/ConvoOptions/SharedLinkButton.tsx b/client/src/components/Conversations/ConvoOptions/SharedLinkButton.tsx index b9b433cad2..0c1687e04a 100644 --- a/client/src/components/Conversations/ConvoOptions/SharedLinkButton.tsx +++ b/client/src/components/Conversations/ConvoOptions/SharedLinkButton.tsx @@ -32,10 +32,51 @@ import { useHasAccess, useResourcePermissions, useLocalize } from '~/hooks'; import { NotificationSeverity } from '~/common'; import { buildShareLinkUrl } from '~/utils'; +type SharePublicationErrorCode = 'TARGET_MESSAGE_NOT_FOUND' | 'NO_MESSAGES'; + +const getSharePublicationErrorCode = (error: unknown): SharePublicationErrorCode | undefined => { + if (error == null || typeof error !== 'object') { + return undefined; + } + + const directCode = 'code' in error ? error.code : undefined; + if (directCode === 'TARGET_MESSAGE_NOT_FOUND' || directCode === 'NO_MESSAGES') { + return directCode; + } + + if (!('response' in error) || error.response == null || typeof error.response !== 'object') { + return undefined; + } + const data = 'data' in error.response ? error.response.data : undefined; + if (data == null || typeof data !== 'object' || !('code' in data)) { + return undefined; + } + return data.code === 'TARGET_MESSAGE_NOT_FOUND' || data.code === 'NO_MESSAGES' + ? data.code + : undefined; +}; + +const publishWithTailRetry = async ( + resolveTargetMessageId: () => Promise, + publish: (targetMessageId?: string) => Promise, +): Promise => { + try { + return await publish(await resolveTargetMessageId()); + } catch (error) { + const code = getSharePublicationErrorCode(error); + if (code !== 'TARGET_MESSAGE_NOT_FOUND' && code !== 'NO_MESSAGES') { + throw error; + } + } + + return publish(await resolveTargetMessageId()); +}; + export default function SharedLinkButton({ share, conversationId, targetMessageId, + resolveTargetMessageId, showQR, setShowQR, sharedLink, @@ -45,6 +86,7 @@ export default function SharedLinkButton({ share: TSharedLinkGetResponse | undefined; conversationId: string; targetMessageId?: string; + resolveTargetMessageId?: () => Promise; showQR: boolean; setShowQR: (showQR: boolean) => void; sharedLink: string; @@ -58,6 +100,7 @@ export default function SharedLinkButton({ const [showDeleteDialog, setShowDeleteDialog] = useState(false); const [showUpdateDialog, setShowUpdateDialog] = useState(false); const [refreshAnimationId, setRefreshAnimationId] = useState(0); + const [isPublishing, setIsPublishing] = useState(false); const [canNativeShare, setCanNativeShare] = useState(false); const [announcement, setAnnouncement] = useState(''); const shareId = share?.shareId ?? ''; @@ -67,25 +110,9 @@ export default function SharedLinkButton({ setCanNativeShare(typeof navigator !== 'undefined' && typeof navigator.share === 'function'); }, []); - const { mutateAsync: mutate, isLoading: isCreateLoading } = useCreateSharedLinkMutation({ - onError: () => { - showToast({ - message: localize('com_ui_share_error'), - severity: NotificationSeverity.ERROR, - showIcon: true, - }); - }, - }); + const { mutateAsync: mutate, isLoading: isCreateLoading } = useCreateSharedLinkMutation(); - const { mutateAsync, isLoading: isUpdateLoading } = useUpdateSharedLinkMutation({ - onError: () => { - showToast({ - message: localize('com_ui_share_error'), - severity: NotificationSeverity.ERROR, - showIcon: true, - }); - }, - }); + const { mutateAsync, isLoading: isUpdateLoading } = useUpdateSharedLinkMutation(); const deleteMutation = useDeleteSharedLinkMutation({ onSuccess: () => { @@ -110,13 +137,34 @@ export default function SharedLinkButton({ const generateShareLink = (shareId: string) => buildShareLinkUrl(shareId); + const showPublicationError = (error: unknown) => { + const code = getSharePublicationErrorCode(error); + let message = localize('com_ui_share_error'); + if (code === 'TARGET_MESSAGE_NOT_FOUND') { + message = localize('com_ui_share_target_not_saved'); + } else if (code === 'NO_MESSAGES') { + message = localize('com_ui_share_no_messages'); + } + showToast({ + message, + severity: NotificationSeverity.ERROR, + showIcon: true, + }); + }; + + const resolvePublicationTarget = () => + resolveTargetMessageId?.() ?? Promise.resolve(targetMessageId); + const updateSharedLink = async () => { if (!shareId) { return; } + setIsPublishing(true); try { - const updateShare = await mutateAsync({ shareId, targetMessageId, snapshotFiles }); + const updateShare = await publishWithTailRetry(resolvePublicationTarget, (resolvedTargetId) => + mutateAsync({ shareId, targetMessageId: resolvedTargetId, snapshotFiles }), + ); setRefreshAnimationId((animationId) => animationId + 1); setSharedLink(generateShareLink(updateShare.shareId)); setShowUpdateDialog(false); @@ -126,15 +174,24 @@ export default function SharedLinkButton({ }, 1000); } catch (error) { console.error('Failed to update shared link:', error); + showPublicationError(error); + } finally { + setIsPublishing(false); } }; const createShareLink = async () => { + setIsPublishing(true); try { - const share = await mutate({ conversationId, targetMessageId, snapshotFiles }); + const share = await publishWithTailRetry(resolvePublicationTarget, (resolvedTargetId) => + mutate({ conversationId, targetMessageId: resolvedTargetId, snapshotFiles }), + ); setSharedLink(generateShareLink(share.shareId)); } catch (error) { console.error('Failed to create shared link:', error); + showPublicationError(error); + } finally { + setIsPublishing(false); } }; @@ -207,13 +264,13 @@ export default function SharedLinkButton({ {!shareId && ( )} {shareId && ( @@ -276,7 +333,7 @@ export default function SharedLinkButton({ variant="outline" size="icon" className="size-9 sm:size-10" - disabled={isUpdateLoading} + disabled={isUpdateLoading || isPublishing} aria-label={localize('com_ui_update_shared_link')} > - {isUpdateLoading && } + {(isUpdateLoading || isPublishing) && } {localize('com_ui_update_shared_link')} diff --git a/client/src/components/Conversations/ConvoOptions/__tests__/ShareButton.test.tsx b/client/src/components/Conversations/ConvoOptions/__tests__/ShareButton.test.tsx index 14541dcdd5..5a2e476bec 100644 --- a/client/src/components/Conversations/ConvoOptions/__tests__/ShareButton.test.tsx +++ b/client/src/components/Conversations/ConvoOptions/__tests__/ShareButton.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { RecoilRoot } from 'recoil'; import { fireEvent, render, screen } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import '@testing-library/jest-dom'; import type { MutableSnapshot } from 'recoil'; import ShareButton from '../ShareButton'; @@ -17,6 +18,22 @@ let mockShare: { }; const mockCopyLink = jest.fn(() => true); const mockAnnouncePolite = jest.fn(); +const mockGetMessagesByConvoId = jest.fn(); +const mockGetMessageById = jest.fn(); +let mockResolveTargetMessageId: (() => Promise) | undefined; +let mockLatestMessageId: string | null = 'message-1'; + +jest.mock('librechat-data-provider', () => { + const actual = jest.requireActual('librechat-data-provider'); + return { + ...actual, + dataService: { + ...actual.dataService, + getMessagesByConvoId: (...args: unknown[]) => mockGetMessagesByConvoId(...args), + getMessageById: (...args: unknown[]) => mockGetMessageById(...args), + }, + }; +}); jest.mock('librechat-data-provider/react-query', () => ({ useGetSharedLinkQuery: () => ({ data: mockShare, isLoading: false }), @@ -27,7 +44,9 @@ jest.mock('~/data-provider', () => ({ })); jest.mock('~/hooks/Messages/useLatestMessage', () => ({ - useLatestMessageId: () => 'message-1', + useLatestMessageId: () => mockLatestMessageId, + useGetLatestMessage: () => () => + mockLatestMessageId == null ? null : { messageId: mockLatestMessageId }, })); jest.mock('~/hooks', () => ({ @@ -46,38 +65,51 @@ jest.mock('../SharedLinkButton', () => ({ setShowQR, snapshotFiles, targetMessageId, + resolveTargetMessageId, }: { showQR: boolean; setShowQR: (show: boolean) => void; snapshotFiles?: boolean; targetMessageId?: string; - }) => ( - - ), + resolveTargetMessageId?: () => Promise; + }) => { + mockResolveTargetMessageId = resolveTargetMessageId; + return ( + + ); + }, })); const ACTIVE_CONVERSATION_ID = 'conversation-1'; const renderShareButton = (conversationId = ACTIVE_CONVERSATION_ID) => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); const initializeState = ({ set }: MutableSnapshot) => { set(store.conversationByIndex(0), { conversationId: ACTIVE_CONVERSATION_ID, } as never); }; - return render( - - - , - ); + return { + queryClient, + ...render( + + + + + , + ), + }; }; describe('ShareButton', () => { @@ -90,6 +122,10 @@ describe('ShareButton', () => { mockCopyLink.mockClear(); mockCopyLink.mockReturnValue(true); mockAnnouncePolite.mockClear(); + mockGetMessagesByConvoId.mockReset(); + mockGetMessageById.mockReset(); + mockLatestMessageId = 'message-1'; + mockResolveTargetMessageId = undefined; }); it('centers the active QR code and keeps details behind inline info controls', () => { @@ -150,21 +186,23 @@ describe('ShareButton', () => { it('resets the file choice and link when the dialog moves to another conversation', () => { mockShare = { success: true, shareId: 'share-1', snapshotFiles: false }; - const { rerender } = renderShareButton(); + const { rerender, queryClient } = renderShareButton(); expect(screen.getByRole('switch', { name: 'com_ui_share_files' })).not.toBeChecked(); mockShare = { success: true, shareId: null }; rerender( - { - set(store.conversationByIndex(0), { - conversationId: ACTIVE_CONVERSATION_ID, - } as never); - }} - > - - , + + { + set(store.conversationByIndex(0), { + conversationId: ACTIVE_CONVERSATION_ID, + } as never); + }} + > + + + , ); expect(screen.getByRole('switch', { name: 'com_ui_share_files' })).toBeChecked(); @@ -181,6 +219,52 @@ describe('ShareButton', () => { ); }); + it('verifies the selected branch tail with a bounded persisted-message read', async () => { + mockGetMessageById.mockResolvedValue([{ messageId: 'message-1' }]); + renderShareButton(); + + await expect(mockResolveTargetMessageId?.()).resolves.toBe('message-1'); + await expect(mockResolveTargetMessageId?.()).resolves.toBe('message-1'); + expect(mockGetMessageById).toHaveBeenCalledTimes(2); + expect(mockGetMessageById).toHaveBeenCalledWith(ACTIVE_CONVERSATION_ID, 'message-1'); + expect(mockGetMessagesByConvoId).not.toHaveBeenCalled(); + }); + + it('rejects an unsaved selected tail instead of dropping the branch target', async () => { + mockGetMessageById.mockResolvedValue([]); + renderShareButton(); + + await expect(mockResolveTargetMessageId?.()).rejects.toMatchObject({ + code: 'TARGET_MESSAGE_NOT_FOUND', + }); + }); + + it('hydrates the message cache before deciding an initially unloaded chat is empty', async () => { + mockLatestMessageId = null; + mockGetMessagesByConvoId.mockImplementation(async () => { + mockLatestMessageId = 'persisted-message'; + return [{ messageId: 'persisted-message' }]; + }); + mockGetMessageById.mockResolvedValue([{ messageId: 'persisted-message' }]); + renderShareButton(); + + await expect(mockResolveTargetMessageId?.()).resolves.toBe('persisted-message'); + expect(mockGetMessagesByConvoId).toHaveBeenCalledWith(ACTIVE_CONVERSATION_ID); + expect(mockGetMessageById).toHaveBeenCalledWith(ACTIVE_CONVERSATION_ID, 'persisted-message'); + }); + + it('reports an empty chat only after checking persisted messages', async () => { + mockLatestMessageId = null; + mockGetMessagesByConvoId.mockResolvedValue([]); + renderShareButton(); + + await expect(mockResolveTargetMessageId?.()).rejects.toMatchObject({ + code: 'NO_MESSAGES', + }); + expect(mockGetMessagesByConvoId).toHaveBeenCalledWith(ACTIVE_CONVERSATION_ID); + expect(mockGetMessageById).not.toHaveBeenCalled(); + }); + it('sends no target message when sharing a conversation other than the open one', () => { renderShareButton('conversation-2'); diff --git a/client/src/components/Conversations/ConvoOptions/__tests__/SharedLinkButton.test.tsx b/client/src/components/Conversations/ConvoOptions/__tests__/SharedLinkButton.test.tsx index 90b83e5833..a86306cf2a 100644 --- a/client/src/components/Conversations/ConvoOptions/__tests__/SharedLinkButton.test.tsx +++ b/client/src/components/Conversations/ConvoOptions/__tests__/SharedLinkButton.test.tsx @@ -139,6 +139,108 @@ describe('SharedLinkButton', () => { expect(setSharedLink).toHaveBeenCalledWith(expect.stringContaining('/share/share-old')); }); + it('refetches the persisted tail and retries once when link creation misses it', async () => { + const resolveTargetMessageId = jest.fn().mockResolvedValue('message-1'); + mockCreate + .mockRejectedValueOnce({ + response: { data: { code: 'TARGET_MESSAGE_NOT_FOUND' } }, + }) + .mockResolvedValueOnce({ shareId: 'share-new' }); + const setSharedLink = jest.fn(); + renderActions({ + share: { success: false, shareId: null }, + resolveTargetMessageId, + setSharedLink, + }); + + fireEvent.click(screen.getByRole('button', { name: 'com_ui_create_link' })); + + await waitFor(() => expect(mockCreate).toHaveBeenCalledTimes(2)); + expect(resolveTargetMessageId).toHaveBeenCalledTimes(2); + expect(mockCreate).toHaveBeenNthCalledWith(1, { + conversationId: 'conversation-1', + targetMessageId: 'message-1', + snapshotFiles: true, + }); + expect(mockCreate).toHaveBeenNthCalledWith(2, { + conversationId: 'conversation-1', + targetMessageId: 'message-1', + snapshotFiles: true, + }); + expect(setSharedLink).toHaveBeenCalledWith(expect.stringContaining('/share/share-new')); + expect(mockShowToast).not.toHaveBeenCalled(); + }); + + it('does not publish another branch when the selected tail is still unsaved', async () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + const missingTail = Object.assign(new Error('missing tail'), { + code: 'TARGET_MESSAGE_NOT_FOUND', + }); + const resolveTargetMessageId = jest.fn().mockRejectedValue(missingTail); + renderActions({ + share: { success: false, shareId: null }, + resolveTargetMessageId, + }); + + fireEvent.click(screen.getByRole('button', { name: 'com_ui_create_link' })); + + await waitFor(() => expect(resolveTargetMessageId).toHaveBeenCalledTimes(2)); + expect(mockCreate).not.toHaveBeenCalled(); + expect(mockShowToast).toHaveBeenCalledWith({ + message: 'com_ui_share_target_not_saved', + severity: 'error', + showIcon: true, + }); + consoleError.mockRestore(); + }); + + it('retries a temporary empty read and publishes once persistence catches up', async () => { + const noMessages = Object.assign(new Error('no messages'), { code: 'NO_MESSAGES' }); + const resolveTargetMessageId = jest + .fn() + .mockRejectedValueOnce(noMessages) + .mockResolvedValueOnce('message-1'); + mockCreate.mockResolvedValue({ shareId: 'share-new' }); + const setSharedLink = jest.fn(); + renderActions({ + share: { success: false, shareId: null }, + resolveTargetMessageId, + setSharedLink, + }); + + fireEvent.click(screen.getByRole('button', { name: 'com_ui_create_link' })); + + await waitFor(() => expect(resolveTargetMessageId).toHaveBeenCalledTimes(2)); + expect(mockCreate).toHaveBeenCalledWith({ + conversationId: 'conversation-1', + targetMessageId: 'message-1', + snapshotFiles: true, + }); + expect(setSharedLink).toHaveBeenCalledWith(expect.stringContaining('/share/share-new')); + expect(mockShowToast).not.toHaveBeenCalled(); + }); + + it('shows a precise error when messages are still absent after the retry', async () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + const noMessages = Object.assign(new Error('no messages'), { code: 'NO_MESSAGES' }); + const resolveTargetMessageId = jest.fn().mockRejectedValue(noMessages); + renderActions({ + share: { success: false, shareId: null }, + resolveTargetMessageId, + }); + + fireEvent.click(screen.getByRole('button', { name: 'com_ui_create_link' })); + + await waitFor(() => expect(resolveTargetMessageId).toHaveBeenCalledTimes(2)); + expect(mockCreate).not.toHaveBeenCalled(); + expect(mockShowToast).toHaveBeenCalledWith({ + message: 'com_ui_share_no_messages', + severity: 'error', + showIcon: true, + }); + consoleError.mockRestore(); + }); + it('does not fake success when updating the link fails', async () => { const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); mockUpdate.mockRejectedValue(new Error('update failed')); diff --git a/client/src/components/Share/ShareView.spec.tsx b/client/src/components/Share/ShareView.spec.tsx index 7b650efc1f..cf5e2f3577 100644 --- a/client/src/components/Share/ShareView.spec.tsx +++ b/client/src/components/Share/ShareView.spec.tsx @@ -1,5 +1,5 @@ -import { render, screen } from '@testing-library/react'; -import { ShareHeader } from './ShareView'; +import { fireEvent, render, screen } from '@testing-library/react'; +import { ShareHeader, SharedLinkUnavailable } from './ShareView'; const defaultProps = { title: 'Shared conversation', @@ -35,3 +35,35 @@ describe('ShareHeader', () => { ).not.toBeInTheDocument(); }); }); + +describe('SharedLinkUnavailable', () => { + it('lets the viewer retry a broken shared-link load', () => { + const onRetry = jest.fn(); + + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + + expect(onRetry).toHaveBeenCalledTimes(1); + }); + + it('disables retry while the shared link is refetching', () => { + render( + , + ); + + expect(screen.getByRole('button', { name: 'Retry' })).toBeDisabled(); + }); +}); diff --git a/client/src/components/Share/ShareView.tsx b/client/src/components/Share/ShareView.tsx index 6f80b62797..0129df5995 100644 --- a/client/src/components/Share/ShareView.tsx +++ b/client/src/components/Share/ShareView.tsx @@ -4,7 +4,7 @@ import { buildTree } from 'librechat-data-provider'; import { useParams, useNavigate } from 'react-router-dom'; import { useRecoilState, useRecoilValue, useRecoilCallback } from 'recoil'; import { useGetSharedMessages } from 'librechat-data-provider/react-query'; -import { CalendarDays, ExternalLink, Settings, MessageSquarePlus } from 'lucide-react'; +import { CalendarDays, ExternalLink, RefreshCw, Settings, MessageSquarePlus } from 'lucide-react'; import { Spinner, Button, @@ -43,7 +43,7 @@ function SharedView() { const { theme, setTheme } = useContext(ThemeContext); const { shareId } = useParams(); const { data: config } = useGetSharedStartupConfig(shareId, { enabled: isAuthReady }); - const { data, isLoading, refetch } = useGetSharedMessages(shareId ?? '', { + const { data, isLoading, isFetching, refetch } = useGetSharedMessages(shareId ?? '', { enabled: isAuthReady, }); const dataTree = data && buildTree({ messages: data.messages }); @@ -203,9 +203,12 @@ function SharedView() { ); } else { content = ( -
- {localize('com_ui_shared_link_not_found')} -
+ void refetch()} + /> ); } @@ -250,6 +253,32 @@ function SharedView() { ); } +export function SharedLinkUnavailable({ + message, + retryLabel, + isRetrying, + onRetry, +}: { + message: string; + retryLabel: string; + isRetrying: boolean; + onRetry: () => void; +}) { + return ( +
+

{message}

+ +
+ ); +} + function ShareTitle({ title }: { title?: string }) { if (title == null || title === '') { return null; diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 98abf4fae9..5fd1b363f1 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -2041,7 +2041,9 @@ "com_ui_share_files_description": "Images and files in this conversation won't be visible to viewers unless this is enabled.", "com_ui_share_files_update_note": "Choose your setting, then select Update link. The same URL will include the latest messages and file choice.", "com_ui_share_link_to_chat": "Share link to chat", + "com_ui_share_no_messages": "This chat has no saved messages to share yet.", "com_ui_share_qr_code_description": "QR code for sharing this conversation link", + "com_ui_share_target_not_saved": "The latest message has not finished saving. Try sharing again in a moment.", "com_ui_share_update_message": "Your name and custom instructions stay private. Edits to shared messages appear right away; select Update link to include new messages without changing the URL.", "com_ui_share_var": "Share {{0}}", "com_ui_shared_link": "shared link", diff --git a/packages/api/src/app/metrics.spec.ts b/packages/api/src/app/metrics.spec.ts index 6d6c1b87e6..59267d36ed 100644 --- a/packages/api/src/app/metrics.spec.ts +++ b/packages/api/src/app/metrics.spec.ts @@ -16,6 +16,7 @@ import { recordOpenIDUserLookup, recordRedisOperation, recordRumProxyRequest, + recordShareLinkRejection, setGenerationJobsInFlight, } from './metrics'; @@ -414,6 +415,28 @@ describe('createMetrics', () => { ); }); + it('tracks bounded shared-link rejection outcomes', async () => { + const app = express(); + process.env.METRICS_SECRET = 'test-secret'; + const { metricsRouter } = createMetrics(); + app.use('/metrics', metricsRouter); + + recordShareLinkRejection('create', 'TARGET_MESSAGE_NOT_FOUND'); + recordShareLinkRejection('update', 'NO_MESSAGES'); + + const response = await request(app) + .get('/metrics') + .set('Authorization', 'Bearer test-secret') + .expect(200); + + expect(response.text).toMatch( + /share_link_rejections_total\{operation="create",code="TARGET_MESSAGE_NOT_FOUND"\} 1/, + ); + expect(response.text).toMatch( + /share_link_rejections_total\{operation="update",code="NO_MESSAGES"\} 1/, + ); + }); + it('tracks mongoose query counts and latency by model and operation', async () => { class FakeQuery { model = { modelName: 'User' }; diff --git a/packages/api/src/app/metrics.ts b/packages/api/src/app/metrics.ts index 0d31a7c2ed..d83a64ceb4 100644 --- a/packages/api/src/app/metrics.ts +++ b/packages/api/src/app/metrics.ts @@ -168,6 +168,8 @@ export type RumProxyResult = | 'collector_5xx' | 'collector_error' | 'collector_timeout'; +export type ShareLinkOperation = 'create' | 'update'; +export type ShareLinkRejectionCode = 'TARGET_MESSAGE_NOT_FOUND' | 'NO_MESSAGES'; export type RedisClient = 'ioredis' | 'keyv'; export type RedisOperationStatus = 'success' | 'error'; @@ -235,6 +237,14 @@ let rumProxyMetrics: RumProxyMetrics = { recordRequest: () => undefined, }; +type ShareLinkMetrics = { + recordRejection: (operation: ShareLinkOperation, code: ShareLinkRejectionCode) => void; +}; + +let shareLinkMetrics: ShareLinkMetrics = { + recordRejection: () => undefined, +}; + type RedisOperationMetrics = { recordOperation: ( client: RedisClient, @@ -270,6 +280,9 @@ const resetMetricRecorders = (): void => { rumProxyMetrics = { recordRequest: () => undefined, }; + shareLinkMetrics = { + recordRejection: () => undefined, + }; redisOperationMetrics = { recordOperation: () => undefined, }; @@ -328,6 +341,13 @@ export function recordRumProxyRequest(endpoint: RumProxyEndpoint, result: RumPro rumProxyMetrics.recordRequest(endpoint, result); } +export function recordShareLinkRejection( + operation: ShareLinkOperation, + code: ShareLinkRejectionCode, +): void { + shareLinkMetrics.recordRejection(operation, code); +} + export function recordRedisOperation( client: RedisClient, useCase: string, @@ -643,6 +663,13 @@ export function createMetrics(options: MetricsOptions = {}): PrometheusMetrics { registers: [registry], }); + const shareLinkRejections = new Counter({ + name: 'share_link_rejections_total', + help: 'Shared link publication rejections by operation and bounded domain code', + labelNames: ['operation', 'code'] as const, + registers: [registry], + }); + const redisOperations = new Counter({ name: 'redis_operations_total', help: 'Logical Redis operations by client, use case, operation, and status', @@ -750,6 +777,10 @@ export function createMetrics(options: MetricsOptions = {}): PrometheusMetrics { recordRequest: (endpoint, result) => rumProxyRequests.inc({ endpoint, result }), }; + shareLinkMetrics = { + recordRejection: (operation, code) => shareLinkRejections.inc({ operation, code }), + }; + redisOperationMetrics = { recordOperation: (client, useCase, operation, status, durationSeconds) => { const labels = { client, use_case: useCase, operation, status }; diff --git a/packages/data-provider/src/data-service.ts b/packages/data-provider/src/data-service.ts index 2b7262f3ec..1621370b61 100644 --- a/packages/data-provider/src/data-service.ts +++ b/packages/data-provider/src/data-service.ts @@ -1002,6 +1002,10 @@ export function getMessagesByConvoId(conversationId: string): Promise { + return request.get(endpoints.messages({ conversationId, messageId })); +} + export function getParentSubagents(parentConversationId: string): Promise { return request.get(endpoints.parentSubagents(parentConversationId)); } diff --git a/packages/data-schemas/src/methods/share.test.ts b/packages/data-schemas/src/methods/share.test.ts index f0238b6864..32bb3def9c 100644 --- a/packages/data-schemas/src/methods/share.test.ts +++ b/packages/data-schemas/src/methods/share.test.ts @@ -10,6 +10,7 @@ import { type ShareMethods, type SharedLinkContentSnapshot, } from './share'; +import logger from '~/config/winston'; describe('Share Methods', () => { let mongoServer: MongoMemoryServer; @@ -1521,6 +1522,30 @@ describe('Share Methods', () => { expect(await SharedLink.findOne({ shareId })).not.toBeNull(); }); + test('does not error-log expected refresh target rejections', async () => { + const userId = new mongoose.Types.ObjectId().toString(); + const conversationId = `conv_${nanoid()}`; + const shareId = `share_${nanoid()}`; + const errorSpy = jest.spyOn(logger, 'error').mockImplementation(() => logger); + await SharedLink.create({ shareId, conversationId, user: userId, messages: [] }); + await Message.create({ + messageId: `msg_${nanoid()}`, + conversationId, + user: userId, + text: 'Current message', + isCreatedByUser: true, + }); + + try { + await expect( + shareMethods.updateSharedLink(userId, shareId, 'missing-message'), + ).rejects.toMatchObject({ code: 'TARGET_MESSAGE_NOT_FOUND' }); + expect(errorSpy).not.toHaveBeenCalled(); + } finally { + errorSpy.mockRestore(); + } + }); + test('should only update with messages from the same user', async () => { const userId = new mongoose.Types.ObjectId().toString(); const otherUserId = new mongoose.Types.ObjectId().toString(); diff --git a/packages/data-schemas/src/methods/share.ts b/packages/data-schemas/src/methods/share.ts index 2dedcc1667..7e13a16b95 100644 --- a/packages/data-schemas/src/methods/share.ts +++ b/packages/data-schemas/src/methods/share.ts @@ -22,6 +22,8 @@ class ShareServiceError extends Error { } } +const EXPECTED_SHARE_REJECTION_CODES = new Set(['TARGET_MESSAGE_NOT_FOUND', 'NO_MESSAGES']); + type ShareOrder = Pick; const isEarlierShare = (candidate: ShareOrder, subject: ShareOrder): boolean => { @@ -1368,11 +1370,13 @@ export function createShareMethods(mongoose: typeof import('mongoose')): { if (preflightFailed) { throw error; } - logger.error('[updateSharedLink] Error updating shared link', { - error: error instanceof Error ? error.message : 'Unknown error', - user, - shareId, - }); + if (!(error instanceof ShareServiceError && EXPECTED_SHARE_REJECTION_CODES.has(error.code))) { + logger.error('[updateSharedLink] Error updating shared link', { + error: error instanceof Error ? error.message : 'Unknown error', + user, + shareId, + }); + } throw new ShareServiceError( error instanceof ShareServiceError ? error.message : 'Error updating shared link', error instanceof ShareServiceError ? error.code : 'SHARE_UPDATE_ERROR',