From 0cd5c462c557fc8985deabfad42af1b4a78770f8 Mon Sep 17 00:00:00 2001 From: Reece Browne Date: Wed, 13 Aug 2025 11:37:58 +0100 Subject: [PATCH] Better tool error handling and fix OCR --- .../tools/convert/useConvertOperation.ts | 18 +++- .../src/hooks/tools/ocr/useOCROperation.ts | 39 +++----- .../src/hooks/tools/shared/useToolApiCalls.ts | 15 ++- .../hooks/tools/shared/useToolOperation.ts | 6 +- .../hooks/tools/shared/useToolResources.ts | 51 ++++++++-- frontend/src/utils/toolErrorHandler.ts | 92 ++++++++++++++++--- 6 files changed, 168 insertions(+), 53 deletions(-) diff --git a/frontend/src/hooks/tools/convert/useConvertOperation.ts b/frontend/src/hooks/tools/convert/useConvertOperation.ts index c79383f75..cf4e4ca3e 100644 --- a/frontend/src/hooks/tools/convert/useConvertOperation.ts +++ b/frontend/src/hooks/tools/convert/useConvertOperation.ts @@ -99,6 +99,8 @@ export const useConvertOperation = () => { // Convert-specific routing logic: decide batch vs individual processing if (shouldProcessFilesSeparately(selectedFiles, parameters)) { // Individual processing for complex cases (PDF→image, smart detection, etc.) + const failedFiles: { file: string; error: string }[] = []; + for (const file of selectedFiles) { try { const formData = buildFormData(parameters, [file]); @@ -108,7 +110,21 @@ export const useConvertOperation = () => { processedFiles.push(convertedFile); } catch (error) { - console.warn(`Failed to convert file ${file.name}:`, error); + const errorMessage = error instanceof Error ? error.message : String(error); + failedFiles.push({ file: file.name, error: errorMessage }); + } + } + + // If some files failed but others succeeded, throw detailed error + if (failedFiles.length > 0) { + if (processedFiles.length === 0) { + // All files failed + const errorDetails = failedFiles.map(f => `${f.file}: ${f.error}`).join(', '); + throw new Error(`All files failed to convert: ${errorDetails}`); + } else { + // Partial failure - log warning but continue with successful files + const failedNames = failedFiles.map(f => `${f.file} (${f.error})`).join(', '); + console.warn(`Some files failed to convert: ${failedNames}. Successfully converted ${processedFiles.length} files.`); } } } else { diff --git a/frontend/src/hooks/tools/ocr/useOCROperation.ts b/frontend/src/hooks/tools/ocr/useOCROperation.ts index 316b867d8..875a627bd 100644 --- a/frontend/src/hooks/tools/ocr/useOCROperation.ts +++ b/frontend/src/hooks/tools/ocr/useOCROperation.ts @@ -2,7 +2,7 @@ import { useCallback } from 'react'; import { useTranslation } from 'react-i18next'; import { OCRParameters } from '../../../components/tools/ocr/OCRSettings'; import { useToolOperation, ToolOperationConfig } from '../shared/useToolOperation'; -import { createStandardErrorHandler } from '../../../utils/toolErrorHandler'; +import { createDockerToolErrorHandler } from '../../../utils/toolErrorHandler'; import { useToolResources } from '../shared/useToolResources'; // Helper: get MIME type based on file extension @@ -16,21 +16,6 @@ function getMimeType(filename: string): string { } } -// Lightweight ZIP extractor (keep or replace with a shared util if you have one) -async function extractZipFile(zipBlob: Blob): Promise { - const JSZip = await import('jszip'); - const zip = new JSZip.default(); - const zipContent = await zip.loadAsync(await zipBlob.arrayBuffer()); - const out: File[] = []; - for (const [filename, file] of Object.entries(zipContent.files)) { - if (!file.dir) { - const content = await file.async('blob'); - out.push(new File([content], filename, { type: getMimeType(filename) })); - } - } - return out; -} - // Helper: strip extension function stripExt(name: string): string { const i = name.lastIndexOf('.'); @@ -64,14 +49,16 @@ export const useOCROperation = () => { // ZIP: sidecar or multi-asset output if (head.startsWith('PK')) { const base = stripExt(originalFiles[0].name); + try { const extracted = await extractZipFiles(blob); if (extracted.length > 0) return extracted; - } catch { /* ignore and try local extractor */ } - try { - const local = await extractZipFile(blob); // local fallback - if (local.length > 0) return local; - } catch { /* fall through */ } + } catch (error) { + // Log extraction failure but don't throw - fall back to raw ZIP + console.warn(`OCR ZIP extraction failed for ${base}, returning as ZIP file:`, error); + } + + // Fallback: return as ZIP file (this prevents "does nothing" behavior) return [new File([blob], `ocr_${base}.zip`, { type: 'application/zip' })]; } @@ -107,10 +94,12 @@ export const useOCROperation = () => { params.languages.length === 0 ? { valid: false, errors: [t('ocr.validation.languageRequired', 'Please select at least one language for OCR processing.')] } : { valid: true }, - getErrorMessage: (error) => - error.message?.includes('OCR tools') && error.message?.includes('not installed') - ? 'OCR tools (OCRmyPDF or Tesseract) are not installed on the server. Use the standard or fat Docker image instead of ultra-lite, or install OCR tools manually.' - : createStandardErrorHandler(t('ocr.error.failed', 'OCR operation failed'))(error), + getErrorMessage: createDockerToolErrorHandler( + 'OCR', + 'standard or fat', + t('ocr.error.failed', 'OCR operation failed'), + ['OCRmyPDF', 'Tesseract'] + ), }; return useToolOperation(ocrConfig); diff --git a/frontend/src/hooks/tools/shared/useToolApiCalls.ts b/frontend/src/hooks/tools/shared/useToolApiCalls.ts index 88f1f4d31..f7604686b 100644 --- a/frontend/src/hooks/tools/shared/useToolApiCalls.ts +++ b/frontend/src/hooks/tools/shared/useToolApiCalls.ts @@ -21,7 +21,7 @@ export const useToolApiCalls = () => { onStatus: (status: string) => void ): Promise => { const processedFiles: File[] = []; - const failedFiles: string[] = []; + const failedFiles: { file: string; error: string }[] = []; const total = validFiles.length; // Create cancel token for this operation @@ -54,17 +54,22 @@ export const useToolApiCalls = () => { if (axios.isCancel(error)) { throw new Error('Operation was cancelled'); } - console.error(`Failed to process ${file.name}:`, error); - failedFiles.push(file.name); + const errorMessage = error instanceof Error ? error.message : String(error); + console.error(`Failed to process ${file.name}:`, errorMessage); + failedFiles.push({ file: file.name, error: errorMessage }); } } if (failedFiles.length > 0 && processedFiles.length === 0) { - throw new Error(`Failed to process all files: ${failedFiles.join(', ')}`); + // All files failed - provide detailed error information + const errorDetails = failedFiles.map(f => `${f.file}: ${f.error}`).join('; '); + throw new Error(`Failed to process all files: ${errorDetails}`); } if (failedFiles.length > 0) { - onStatus(`Processed ${processedFiles.length}/${total} files. Failed: ${failedFiles.join(', ')}`); + // Some files failed - provide detailed status with errors + const failedNames = failedFiles.map(f => `${f.file} (${f.error})`).join(', '); + onStatus(`Processed ${processedFiles.length}/${total} files. Failed: ${failedNames}`); } else { onStatus(`Successfully processed ${processedFiles.length} file${processedFiles.length === 1 ? '' : 's'}`); } diff --git a/frontend/src/hooks/tools/shared/useToolOperation.ts b/frontend/src/hooks/tools/shared/useToolOperation.ts index 7251d1fd2..81de8fb51 100644 --- a/frontend/src/hooks/tools/shared/useToolOperation.ts +++ b/frontend/src/hooks/tools/shared/useToolOperation.ts @@ -5,7 +5,7 @@ import { useFileContext } from '../../../contexts/FileContext'; import { useToolState, type ProcessingProgress } from './useToolState'; import { useToolApiCalls, type ApiCallsConfig } from './useToolApiCalls'; import { useToolResources } from './useToolResources'; -import { extractErrorMessage } from '../../../utils/toolErrorHandler'; +import { extractErrorMessage, type ToolError } from '../../../utils/toolErrorHandler'; import { createOperation } from '../../../utils/toolOperationTracker'; import { type ResponseHandler, processResponse } from '../../../utils/toolResponseProcessor'; @@ -68,7 +68,7 @@ export interface ToolOperationConfig { validateParams?: (params: TParams) => ValidationResult; /** Extract user-friendly error messages from API errors */ - getErrorMessage?: (error: any) => string; + getErrorMessage?: (error: unknown) => string; } /** @@ -220,7 +220,7 @@ export const useToolOperation = ( markOperationApplied(fileId, operationId); } - } catch (error: any) { + } catch (error: unknown) { const errorMessage = config.getErrorMessage?.(error) || extractErrorMessage(error); actions.setError(errorMessage); actions.setStatus(''); diff --git a/frontend/src/hooks/tools/shared/useToolResources.ts b/frontend/src/hooks/tools/shared/useToolResources.ts index 69f48fe20..d7d3c70aa 100644 --- a/frontend/src/hooks/tools/shared/useToolResources.ts +++ b/frontend/src/hooks/tools/shared/useToolResources.ts @@ -54,10 +54,15 @@ export const useToolResources = () => { try { const zipFile = new File([zipBlob], 'temp.zip', { type: 'application/zip' }); const extractionResult = await zipFileService.extractPdfFiles(zipFile); - return extractionResult.success ? extractionResult.extractedFiles : []; + + if (!extractionResult.success) { + throw new Error(`ZIP extraction failed: ${extractionResult.error || 'Unknown error'}`); + } + + return extractionResult.extractedFiles; } catch (error) { - console.error('useToolResources.extractZipFiles - Error:', error); - return []; + const errorMessage = error instanceof Error ? error.message : `ZIP extraction error: ${error}`; + throw new Error(errorMessage); } }, []); @@ -74,18 +79,52 @@ export const useToolResources = () => { for (const [filename, file] of Object.entries(zipContent.files)) { if (!file.dir) { const content = await file.async('blob'); - const extractedFile = new File([content], filename, { type: 'application/pdf' }); + // Determine MIME type based on file extension + const mimeType = getMimeTypeFromFilename(filename); + const extractedFile = new File([content], filename, { type: mimeType }); extractedFiles.push(extractedFile); } } + if (extractedFiles.length === 0) { + throw new Error('ZIP file contains no extractable files'); + } + return extractedFiles; } catch (error) { - console.error('Error in extractAllZipFiles:', error); - return []; + const errorMessage = error instanceof Error ? error.message : `ZIP extraction error: ${error}`; + throw new Error(errorMessage); } }, []); + // Helper function to determine MIME type from filename + const getMimeTypeFromFilename = (filename: string): string => { + const ext = filename.toLowerCase().split('.').pop(); + switch (ext) { + case 'pdf': return 'application/pdf'; + case 'txt': return 'text/plain'; + case 'jpg': + case 'jpeg': return 'image/jpeg'; + case 'png': return 'image/png'; + case 'gif': return 'image/gif'; + case 'svg': return 'image/svg+xml'; + case 'html': + case 'htm': return 'text/html'; + case 'css': return 'text/css'; + case 'js': return 'application/javascript'; + case 'json': return 'application/json'; + case 'xml': return 'application/xml'; + case 'zip': return 'application/zip'; + case 'doc': return 'application/msword'; + case 'docx': return 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'; + case 'xls': return 'application/vnd.ms-excel'; + case 'xlsx': return 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'; + case 'ppt': return 'application/vnd.ms-powerpoint'; + case 'pptx': return 'application/vnd.openxmlformats-officedocument.presentationml.presentation'; + default: return 'application/octet-stream'; + } + }; + const createDownloadInfo = useCallback(async ( files: File[], operationType: string diff --git a/frontend/src/utils/toolErrorHandler.ts b/frontend/src/utils/toolErrorHandler.ts index ee1efe4d9..463d00d12 100644 --- a/frontend/src/utils/toolErrorHandler.ts +++ b/frontend/src/utils/toolErrorHandler.ts @@ -2,15 +2,26 @@ * Standardized error handling utilities for tool operations */ +/** + * Standard error type that covers common error patterns + */ +export interface ToolError { + message?: string; + response?: { + data?: string | unknown; + }; +} + /** * Default error extractor that follows the standard pattern */ -export const extractErrorMessage = (error: any): string => { - if (error.response?.data && typeof error.response.data === 'string') { - return error.response.data; +export const extractErrorMessage = (error: unknown): string => { + const typedError = error as ToolError; + if (typedError.response?.data && typeof typedError.response.data === 'string') { + return typedError.response.data; } - if (error.message) { - return error.message; + if (typedError.message) { + return typedError.message; } return 'Operation failed'; }; @@ -21,13 +32,68 @@ export const extractErrorMessage = (error: any): string => { * @returns Error handler function that follows the standard pattern */ export const createStandardErrorHandler = (fallbackMessage: string) => { - return (error: any): string => { - if (error.response?.data && typeof error.response.data === 'string') { - return error.response.data; - } - if (error.message) { - return error.message; - } - return fallbackMessage; + return (error: unknown): string => { + return extractErrorMessage(error) || fallbackMessage; }; +}; + +/** + * Creates error handler for tools that require specific Docker images or system dependencies. + * Detects common "tool not available" patterns and provides user-friendly Docker upgrade messages. + * + * @param toolName - Name of the tool (e.g., "OCR", "LibreOffice") + * @param requiredImages - Docker images that support this tool (e.g., "standard or fat") + * @param defaultMessage - Fallback error message + * @param detectionPatterns - Additional patterns to detect tool unavailability + * + * @example + * // OCR tool + * getErrorMessage: createDockerToolErrorHandler( + * 'OCR', + * 'standard or fat', + * t('ocr.error.failed', 'OCR operation failed'), + * ['OCRmyPDF', 'Tesseract'] + * ) + * + * // LibreOffice tool + * getErrorMessage: createDockerToolErrorHandler( + * 'LibreOffice', + * 'standard or fat', + * t('convert.error.failed', 'Conversion failed'), + * ['libreoffice', 'soffice'] + * ) + */ +export const createDockerToolErrorHandler = ( + toolName: string, + requiredImages: string, + defaultMessage: string, + detectionPatterns: string[] = [] +) => (error: unknown): string => { + const typedError = error as ToolError; + const message = typedError?.message || ''; + + // Common patterns for tool unavailability + const commonPatterns = [ + 'not installed', + 'not available', + 'command not found', + 'executable not found', + 'dependency not found' + ]; + + const allPatterns = [...commonPatterns, ...detectionPatterns]; + + // Check if error indicates tool is not available + const isToolUnavailable = allPatterns.some(pattern => + message.toLowerCase().includes(pattern.toLowerCase()) + ) && ( + message.toLowerCase().includes(toolName.toLowerCase()) || + detectionPatterns.some(pattern => message.toLowerCase().includes(pattern.toLowerCase())) + ); + + if (isToolUnavailable) { + return `${toolName} tools are not installed on the server. Use the ${requiredImages} Docker image instead of ultra-lite, or install ${toolName} tools manually.`; + } + + return createStandardErrorHandler(defaultMessage)(error); }; \ No newline at end of file