From 365a0dc0f69a48d6dc669cade8bd2822958797b4 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 20 Mar 2026 17:10:25 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A9=BA=20refactor:=20Surface=20Descriptiv?= =?UTF-8?q?e=20OCR=20Error=20Messages=20to=20Client=20(#12344)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: pass along error message when OCR fails Right now, if OCR fails, it just says "Error processing file" which isn't very helpful. The `error.message` does has helpful information in it, but our filter wasn't including the right case to pass it along. Now it does! * fix: extract shared upload error filter, apply to images route The 'Unable to extract text from' error was only allowlisted in the files route but not the images route, which also calls processAgentFileUpload. Extract the duplicated error filter logic into a shared resolveUploadErrorMessage utility in packages/api so both routes stay in sync. --------- Co-authored-by: Dan Lew --- api/server/routes/files/files.js | 16 +------- api/server/routes/files/images.js | 12 +----- packages/api/src/utils/files.spec.ts | 55 +++++++++++++++++++++++++++- packages/api/src/utils/files.ts | 33 +++++++++++++++++ 4 files changed, 91 insertions(+), 25 deletions(-) diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index 9290d1a7ed..fdb7768c3b 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -2,7 +2,7 @@ const fs = require('fs').promises; const express = require('express'); const { EnvVar } = require('@librechat/agents'); const { logger } = require('@librechat/data-schemas'); -const { verifyAgentUploadPermission } = require('@librechat/api'); +const { verifyAgentUploadPermission, resolveUploadErrorMessage } = require('@librechat/api'); const { Time, isUUID, @@ -394,21 +394,9 @@ router.post('/', async (req, res) => { return await processAgentFileUpload({ req, res, metadata }); } catch (error) { - let message = 'Error processing file'; + const message = resolveUploadErrorMessage(error); logger.error('[/files] Error processing file:', error); - if (error.message?.includes('file_ids')) { - message += ': ' + error.message; - } - - if ( - error.message?.includes('Invalid file format') || - error.message?.includes('No OCR result') || - error.message?.includes('exceeds token limit') - ) { - message = error.message; - } - try { await fs.unlink(req.file.path); cleanup = false; diff --git a/api/server/routes/files/images.js b/api/server/routes/files/images.js index 185ec7a671..d5d8f51193 100644 --- a/api/server/routes/files/images.js +++ b/api/server/routes/files/images.js @@ -2,7 +2,7 @@ const path = require('path'); const fs = require('fs').promises; const express = require('express'); const { logger } = require('@librechat/data-schemas'); -const { verifyAgentUploadPermission } = require('@librechat/api'); +const { verifyAgentUploadPermission, resolveUploadErrorMessage } = require('@librechat/api'); const { isAssistantsEndpoint } = require('librechat-data-provider'); const { processAgentFileUpload, @@ -43,15 +43,7 @@ router.post('/', async (req, res) => { // TODO: delete remote file if it exists logger.error('[/files/images] Error processing file:', error); - let message = 'Error processing file'; - - if ( - error.message?.includes('Invalid file format') || - error.message?.includes('No OCR result') || - error.message?.includes('exceeds token limit') - ) { - message = error.message; - } + const message = resolveUploadErrorMessage(error); try { const filepath = path.join( diff --git a/packages/api/src/utils/files.spec.ts b/packages/api/src/utils/files.spec.ts index db51d51e0f..2f1b1346aa 100644 --- a/packages/api/src/utils/files.spec.ts +++ b/packages/api/src/utils/files.spec.ts @@ -1,4 +1,4 @@ -import { sanitizeFilename } from './files'; +import { sanitizeFilename, resolveUploadErrorMessage } from './files'; jest.mock('node:crypto', () => { const actualModule = jest.requireActual('node:crypto'); @@ -52,6 +52,59 @@ describe('sanitizeFilename', () => { }); }); +describe('resolveUploadErrorMessage', () => { + test('returns default message for null error', () => { + expect(resolveUploadErrorMessage(null)).toBe('Error processing file'); + }); + + test('returns default message for undefined error', () => { + expect(resolveUploadErrorMessage(undefined)).toBe('Error processing file'); + }); + + test('returns default message when error has no message property', () => { + expect(resolveUploadErrorMessage({})).toBe('Error processing file'); + }); + + test('returns default message for unrecognized error', () => { + expect(resolveUploadErrorMessage({ message: 'ENOENT: no such file or directory' })).toBe( + 'Error processing file', + ); + }); + + test('prepends default message for file_ids errors', () => { + expect(resolveUploadErrorMessage({ message: 'max file_ids reached' })).toBe( + 'Error processing file: max file_ids reached', + ); + }); + + test('surfaces "Invalid file format" errors', () => { + expect(resolveUploadErrorMessage({ message: 'Invalid file format: .xyz' })).toBe( + 'Invalid file format: .xyz', + ); + }); + + test('surfaces "exceeds token limit" errors', () => { + expect(resolveUploadErrorMessage({ message: 'Content exceeds token limit' })).toBe( + 'Content exceeds token limit', + ); + }); + + test('surfaces "Unable to extract text from" errors', () => { + const msg = 'Unable to extract text from "doc.pdf". The document may be image-based.'; + expect(resolveUploadErrorMessage({ message: msg })).toBe(msg); + }); + + test('accepts a custom default message', () => { + expect(resolveUploadErrorMessage(null, 'Custom default')).toBe('Custom default'); + }); + + test('uses custom default in file_ids prepend', () => { + expect(resolveUploadErrorMessage({ message: 'file_ids limit' }, 'Upload failed')).toBe( + 'Upload failed: file_ids limit', + ); + }); +}); + describe('sanitizeFilename with real crypto', () => { // Temporarily unmock crypto for these tests beforeAll(() => { diff --git a/packages/api/src/utils/files.ts b/packages/api/src/utils/files.ts index 2fa3b62ab3..9e78d9289c 100644 --- a/packages/api/src/utils/files.ts +++ b/packages/api/src/utils/files.ts @@ -3,6 +3,39 @@ import crypto from 'node:crypto'; import { createReadStream } from 'fs'; import { readFile, stat } from 'fs/promises'; +const USER_FACING_UPLOAD_ERRORS = [ + 'Invalid file format', + 'exceeds token limit', + 'Unable to extract text from', +] as const; + +/** + * Resolves a user-facing error message from a file upload error. + * Returns the error's own message if it matches a known user-facing pattern, + * otherwise returns the default message. + */ +export function resolveUploadErrorMessage( + error: { message?: string } | null | undefined, + defaultMessage = 'Error processing file', +): string { + const errorMessage = error?.message; + if (!errorMessage) { + return defaultMessage; + } + + if (errorMessage.includes('file_ids')) { + return `${defaultMessage}: ${errorMessage}`; + } + + for (const fragment of USER_FACING_UPLOAD_ERRORS) { + if (errorMessage.includes(fragment)) { + return errorMessage; + } + } + + return defaultMessage; +} + /** * Sanitize a filename by removing any directory components, replacing non-alphanumeric characters * @param inputName