diff --git a/api/server/services/Files/process.js b/api/server/services/Files/process.js index 8910163047..38ccdafdd7 100644 --- a/api/server/services/Files/process.js +++ b/api/server/services/Files/process.js @@ -55,7 +55,9 @@ const processFiles = async (files, fileIds) => { } if (!fileIds) { - return await Promise.all(promises); + const results = await Promise.all(promises); + // Filter out null results from failed updateFileUsage calls + return results.filter((result) => result != null); } for (let file_id of fileIds) { @@ -67,7 +69,9 @@ const processFiles = async (files, fileIds) => { } // TODO: calculate token cost when image is first uploaded - return await Promise.all(promises); + const results = await Promise.all(promises); + // Filter out null results from failed updateFileUsage calls + return results.filter((result) => result != null); }; /** diff --git a/api/server/services/Files/processFiles.test.js b/api/server/services/Files/processFiles.test.js new file mode 100644 index 0000000000..8665d33665 --- /dev/null +++ b/api/server/services/Files/processFiles.test.js @@ -0,0 +1,208 @@ +// Mock the updateFileUsage function before importing the actual processFiles +jest.mock('~/models/File', () => ({ + updateFileUsage: jest.fn(), +})); + +// Mock winston and logger configuration to avoid dependency issues +jest.mock('~/config', () => ({ + logger: { + info: jest.fn(), + warn: jest.fn(), + debug: jest.fn(), + error: jest.fn(), + }, +})); + +// Mock all other dependencies that might cause issues +jest.mock('librechat-data-provider', () => ({ + isUUID: { parse: jest.fn() }, + megabyte: 1024 * 1024, + FileContext: { message_attachment: 'message_attachment' }, + FileSources: { local: 'local' }, + EModelEndpoint: { assistants: 'assistants' }, + EToolResources: { file_search: 'file_search' }, + mergeFileConfig: jest.fn(), + removeNullishValues: jest.fn((obj) => obj), + isAssistantsEndpoint: jest.fn(), +})); + +jest.mock('~/server/services/Files/images', () => ({ + convertImage: jest.fn(), + resizeAndConvert: jest.fn(), + resizeImageBuffer: jest.fn(), +})); + +jest.mock('~/server/controllers/assistants/v2', () => ({ + addResourceFileId: jest.fn(), + deleteResourceFileId: jest.fn(), +})); + +jest.mock('~/models/Agent', () => ({ + addAgentResourceFile: jest.fn(), + removeAgentResourceFiles: jest.fn(), +})); + +jest.mock('~/server/controllers/assistants/helpers', () => ({ + getOpenAIClient: jest.fn(), +})); + +jest.mock('~/server/services/Tools/credentials', () => ({ + loadAuthValues: jest.fn(), +})); + +jest.mock('~/server/services/Config', () => ({ + checkCapability: jest.fn(), +})); + +jest.mock('~/server/utils/queue', () => ({ + LB_QueueAsyncCall: jest.fn(), +})); + +jest.mock('./strategies', () => ({ + getStrategyFunctions: jest.fn(), +})); + +jest.mock('~/server/utils', () => ({ + determineFileType: jest.fn(), +})); + +// Import the actual processFiles function after all mocks are set up +const { processFiles } = require('./process'); +const { updateFileUsage } = require('~/models/File'); + +describe('processFiles', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('null filtering functionality', () => { + it('should filter out null results from updateFileUsage when files do not exist', async () => { + const mockFiles = [ + { file_id: 'existing-file-1' }, + { file_id: 'non-existent-file' }, + { file_id: 'existing-file-2' }, + ]; + + // Mock updateFileUsage to return null for non-existent files + updateFileUsage.mockImplementation(({ file_id }) => { + if (file_id === 'non-existent-file') { + return Promise.resolve(null); // Simulate file not found in the database + } + return Promise.resolve({ file_id, usage: 1 }); + }); + + const result = await processFiles(mockFiles); + + expect(updateFileUsage).toHaveBeenCalledTimes(3); + expect(result).toEqual([ + { file_id: 'existing-file-1', usage: 1 }, + { file_id: 'existing-file-2', usage: 1 }, + ]); + + // Critical test - ensure no null values in result + expect(result).not.toContain(null); + expect(result).not.toContain(undefined); + expect(result.length).toBe(2); // Only valid files should be returned + }); + + it('should return empty array when all updateFileUsage calls return null', async () => { + const mockFiles = [{ file_id: 'non-existent-1' }, { file_id: 'non-existent-2' }]; + + // All updateFileUsage calls return null + updateFileUsage.mockResolvedValue(null); + + const result = await processFiles(mockFiles); + + expect(updateFileUsage).toHaveBeenCalledTimes(2); + expect(result).toEqual([]); + expect(result).not.toContain(null); + expect(result.length).toBe(0); + }); + + it('should work correctly when all files exist', async () => { + const mockFiles = [{ file_id: 'file-1' }, { file_id: 'file-2' }]; + + updateFileUsage.mockImplementation(({ file_id }) => { + return Promise.resolve({ file_id, usage: 1 }); + }); + + const result = await processFiles(mockFiles); + + expect(result).toEqual([ + { file_id: 'file-1', usage: 1 }, + { file_id: 'file-2', usage: 1 }, + ]); + expect(result).not.toContain(null); + expect(result.length).toBe(2); + }); + + it('should handle fileIds parameter and filter nulls correctly', async () => { + const mockFiles = [{ file_id: 'file-1' }]; + const mockFileIds = ['file-2', 'non-existent-file']; + + updateFileUsage.mockImplementation(({ file_id }) => { + if (file_id === 'non-existent-file') { + return Promise.resolve(null); + } + return Promise.resolve({ file_id, usage: 1 }); + }); + + const result = await processFiles(mockFiles, mockFileIds); + + expect(result).toEqual([ + { file_id: 'file-1', usage: 1 }, + { file_id: 'file-2', usage: 1 }, + ]); + expect(result).not.toContain(null); + expect(result).not.toContain(undefined); + expect(result.length).toBe(2); + }); + + it('should handle duplicate file_ids correctly', async () => { + const mockFiles = [ + { file_id: 'duplicate-file' }, + { file_id: 'duplicate-file' }, // Duplicate should be ignored + { file_id: 'unique-file' }, + ]; + + updateFileUsage.mockImplementation(({ file_id }) => { + return Promise.resolve({ file_id, usage: 1 }); + }); + + const result = await processFiles(mockFiles); + + // Should only call updateFileUsage twice (duplicate ignored) + expect(updateFileUsage).toHaveBeenCalledTimes(2); + expect(result).toEqual([ + { file_id: 'duplicate-file', usage: 1 }, + { file_id: 'unique-file', usage: 1 }, + ]); + expect(result.length).toBe(2); + }); + }); + + describe('edge cases', () => { + it('should handle empty files array', async () => { + const result = await processFiles([]); + expect(result).toEqual([]); + expect(updateFileUsage).not.toHaveBeenCalled(); + }); + + it('should handle mixed null and undefined returns from updateFileUsage', async () => { + const mockFiles = [{ file_id: 'file-1' }, { file_id: 'file-2' }, { file_id: 'file-3' }]; + + updateFileUsage.mockImplementation(({ file_id }) => { + if (file_id === 'file-1') return Promise.resolve(null); + if (file_id === 'file-2') return Promise.resolve(undefined); + return Promise.resolve({ file_id, usage: 1 }); + }); + + const result = await processFiles(mockFiles); + + expect(result).toEqual([{ file_id: 'file-3', usage: 1 }]); + expect(result).not.toContain(null); + expect(result).not.toContain(undefined); + expect(result.length).toBe(1); + }); + }); +});