From 808da564f3c5990a3e69f53a11ae30096eb37cc8 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Sun, 26 Jul 2026 12:18:52 +0200 Subject: [PATCH] fix: improve error handling and stability - no logout on transient errors, add ErrorBoundary, global error logging --- app/main.py | 24 +++- app/routes/errors.py | 77 +++++++++++++ frontend/src/App.tsx | 5 +- frontend/src/api/client.ts | 22 ++++ .../src/components/common/ErrorBoundary.tsx | 105 +++++++++++++++++ frontend/src/hooks/useAuth.ts | 13 ++- frontend/src/utils/errorLogger.ts | 106 ++++++++++++++++++ 7 files changed, 344 insertions(+), 8 deletions(-) create mode 100644 app/routes/errors.py create mode 100644 frontend/src/components/common/ErrorBoundary.tsx create mode 100644 frontend/src/utils/errorLogger.ts diff --git a/app/main.py b/app/main.py index faa3102..6711037 100644 --- a/app/main.py +++ b/app/main.py @@ -8,7 +8,7 @@ from contextlib import asynccontextmanager from fastapi import FastAPI, HTTPException, Request from fastapi.middleware.cors import CORSMiddleware -from fastapi.responses import FileResponse +from fastapi.responses import FileResponse, JSONResponse from fastapi.staticfiles import StaticFiles from starlette.middleware.base import BaseHTTPMiddleware import importlib @@ -29,6 +29,7 @@ from app.routes import ( ai_copilot, audit, auth, + errors, contact_folders, contacts, dashboard, @@ -308,6 +309,22 @@ def create_app() -> FastAPI: app.add_middleware(CSRFMiddleware) app.add_middleware(RequestLoggingMiddleware) + # ── Global exception handler — catch ALL unhandled exceptions ── + @app.exception_handler(Exception) + async def global_exception_handler(request: Request, exc: Exception): + logger.error(f"Unhandled exception: {exc}", exc_info=True) + record_error( + event="unhandled_exception", + method=request.method, + path=request.url.path, + status_code=500, + error=str(exc), + ) + return JSONResponse( + status_code=500, + content={"detail": "Internal server error", "code": "internal_error"}, + ) + app.include_router(health.router) app.include_router(metrics.router) app.include_router(auth.router) @@ -338,6 +355,7 @@ def create_app() -> FastAPI: app.include_router(custom_fields.router) app.include_router(saved_filters.router) app.include_router(webhooks.router) + app.include_router(errors.router) # ── Register plugin routes for all built-in plugins ── # Routes are registered here (before app start); activation status @@ -375,8 +393,8 @@ def create_app() -> FastAPI: router_module = importlib.import_module(route_def.module) router = getattr(router_module, route_def.router_attr) app.include_router(router) - except Exception: - pass + except Exception as exc: + logger.error(f"Failed to register route {route_def.module}.{route_def.router_attr}: {exc}") break except Exception as exc: logger.error(f"Failed to register plugin routes for {mod_name}: {exc}") diff --git a/app/routes/errors.py b/app/routes/errors.py new file mode 100644 index 0000000..6012bfd --- /dev/null +++ b/app/routes/errors.py @@ -0,0 +1,77 @@ +"""Error logging endpoint — accepts frontend errors and logs them. + +No auth required so errors can be logged even during logout. +Rate-limited to 10 requests per minute per IP (simple in-memory implementation). +""" + +from __future__ import annotations + +import time +import logging +from collections import defaultdict, deque +from typing import Any + +from fastapi import APIRouter, Request, Response, status +from pydantic import BaseModel, Field + +logger = logging.getLogger(__name__) + +router = APIRouter(prefix="/api/v1/errors", tags=["errors"]) + +# -- Simple in-memory rate limiter (10 req/min per IP) -- +RATE_LIMIT = 10 # max requests +RATE_WINDOW = 60 # seconds +_ip_requests: dict[str, deque[float]] = defaultdict(deque) + + +def _is_rate_limited(client_ip: str) -> bool: + """Return True if the IP has exceeded the rate limit.""" + now = time.monotonic() + dq = _ip_requests[client_ip] + + # Remove timestamps outside the window + while dq and now - dq[0] > RATE_WINDOW: + dq.popleft() + + if len(dq) >= RATE_LIMIT: + return True + + dq.append(now) + return False + + +# -- Request schema -- + +class ErrorReport(BaseModel): + timestamp: str | None = None + message: str = Field(..., max_length=2000) + stack: str | None = Field(None, max_length=10000) + context: dict[str, Any] | None = None + url: str | None = Field(None, max_length=500) + userAgent: str | None = Field(None, max_length=500) + + +@router.post("", status_code=status.HTTP_204_NO_CONTENT) +async def report_error(error: ErrorReport, request: Request) -> Response: + """Log a frontend error. No auth required. Rate-limited per IP.""" + client_ip = request.client.host if request.client else "unknown" + + if _is_rate_limited(client_ip): + return Response(status_code=status.HTTP_429_TOO_MANY_REQUESTS) + + # Log with structured info + logger.error( + "Frontend error reported: %s", + error.message, + extra={ + "error_timestamp": error.timestamp, + "error_message": error.message, + "error_stack": error.stack, + "error_context": error.context, + "error_url": error.url, + "error_user_agent": error.userAgent, + "client_ip": client_ip, + }, + ) + + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 279832e..aeac8a5 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -4,6 +4,7 @@ import { AppRouter } from '@/routes'; import { setUnauthorizedHandler } from '@/api/client'; import { useAuthStore } from '@/store/authStore'; import { useThemeStore } from '@/store/themeStore'; +import { ErrorBoundary } from '@/components/common/ErrorBoundary'; const queryClient = new QueryClient({ defaultOptions: { @@ -33,7 +34,9 @@ export default function App() { return ( - + + + ); } diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 4c5fde9..0ca1ea0 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -55,17 +55,39 @@ apiClient.interceptors.request.use( (error) => Promise.reject(error) ); +// Retry interceptor for 5xx errors (max 1 retry with short delay) +apiClient.interceptors.response.use( + (response) => response, + async (error: AxiosError) => { + const config = error.config as InternalAxiosRequestConfig & { _retried?: boolean }; + const status = error.response?.status || 0; + + // Retry 5xx errors once with a short delay + if (status >= 500 && status < 600 && config && !config._retried) { + config._retried = true; + await new Promise((resolve) => setTimeout(resolve, 500)); + return apiClient.request(config); + } + + return Promise.reject(error); + } +); + +// Main error-handling interceptor apiClient.interceptors.response.use( (response) => response, (error: AxiosError) => { const status = error.response?.status || 0; const data = error.response?.data as any; + // Only call onUnauthorized for 401 — NOT for 403 or network errors if (status === 401) { if (onUnauthorized) { onUnauthorized(); } } + // 403: Permission error — do NOT logout, let the calling code handle it + // Network errors (status === 0): do NOT logout, transient issue if (status === 422 && data?.detail) { const validationErrors: Record = {}; diff --git a/frontend/src/components/common/ErrorBoundary.tsx b/frontend/src/components/common/ErrorBoundary.tsx new file mode 100644 index 0000000..b6c5e0b --- /dev/null +++ b/frontend/src/components/common/ErrorBoundary.tsx @@ -0,0 +1,105 @@ +import React, { Component, ErrorInfo, ReactNode } from 'react'; +import { logError } from '@/utils/errorLogger'; + +interface ErrorBoundaryProps { + children: ReactNode; + /** Optional fallback render function; receives the error and a retry callback */ + fallback?: (error: Error, retry: () => void) => ReactNode; +} + +interface ErrorBoundaryState { + hasError: boolean; + error: Error | null; +} + +/** + * React Error Boundary that catches JavaScript errors in the component tree. + * Shows a friendly error message with a Retry button. + * Logs errors via the global error logger. + * Only the affected subtree is broken — the rest of the app continues. + */ +export class ErrorBoundary extends Component { + constructor(props: ErrorBoundaryProps) { + super(props); + this.state = { hasError: false, error: null }; + } + + static getDerivedStateFromError(error: Error): ErrorBoundaryState { + return { hasError: true, error }; + } + + componentDidCatch(error: Error, errorInfo: ErrorInfo): void { + // Log to console + backend + sessionStorage buffer + logError(error, { componentStack: errorInfo.componentStack }); + } + + handleRetry = (): void => { + this.setState({ hasError: false, error: null }); + }; + + render(): ReactNode { + if (this.state.hasError && this.state.error) { + // Custom fallback if provided + if (this.props.fallback) { + return this.props.fallback(this.state.error, this.handleRetry); + } + + // Default fallback UI + return ( +
+

+ Something went wrong +

+

+ An unexpected error occurred. You can try again or refresh the page. +

+
+ + Error details + + 
+              {this.state.error.message}
+              {this.state.error.stack ? `\n\n${this.state.error.stack}` : ''}
+
+
+ +
+ ); + } + + return this.props.children; + } +} + +export default ErrorBoundary; diff --git a/frontend/src/hooks/useAuth.ts b/frontend/src/hooks/useAuth.ts index 9e7a656..e0cc6cd 100644 --- a/frontend/src/hooks/useAuth.ts +++ b/frontend/src/hooks/useAuth.ts @@ -4,14 +4,19 @@ import { useCurrentUser } from '@/api/hooks'; export function useAuth() { const store = useAuthStore(); - const { data, isLoading, isError } = useCurrentUser(); + const { data, isLoading, isError, error } = useCurrentUser(); useEffect(() => { if (isError) { - store.setAuthenticated(false); - store.setUser(null); + const status = (error as any)?.status || 0; + // Only logout on 401 Unauthorized — other errors are transient + if (status === 401) { + store.setAuthenticated(false); + store.setUser(null); + } + // For 403, 500, network errors: keep session, don't logout } - }, [isError, store]); + }, [isError, error, store]); return { user: store.user, diff --git a/frontend/src/utils/errorLogger.ts b/frontend/src/utils/errorLogger.ts new file mode 100644 index 0000000..1c1e148 --- /dev/null +++ b/frontend/src/utils/errorLogger.ts @@ -0,0 +1,106 @@ +/** + * Global Error Logger + * + * Centralized error collection and reporting. + * - Always logs to console.error + * - Optionally POSTs to /api/v1/errors (silently fails if endpoint unavailable) + * - Buffers errors in sessionStorage (ring buffer, max 50) + */ + +const STORAGE_KEY = 'leocrm_error_buffer'; +const MAX_BUFFER_SIZE = 50; + +interface ErrorEntry { + timestamp: string; + message: string; + stack?: string; + context?: Record; + url?: string; + userAgent?: string; +} + +function getBuffer(): ErrorEntry[] { + try { + const raw = sessionStorage.getItem(STORAGE_KEY); + if (!raw) return []; + return JSON.parse(raw) as ErrorEntry[]; + } catch { + return []; + } +} + +function saveBuffer(buffer: ErrorEntry[]): void { + try { + // Ring buffer: keep only the last MAX_BUFFER_SIZE entries + const trimmed = buffer.slice(-MAX_BUFFER_SIZE); + sessionStorage.setItem(STORAGE_KEY, JSON.stringify(trimmed)); + } catch { + // sessionStorage might be full or unavailable — silently ignore + } +} + +function addToBuffer(entry: ErrorEntry): void { + const buffer = getBuffer(); + buffer.push(entry); + saveBuffer(buffer); +} + +/** + * Log an error with optional context. + * Always writes to console.error. + * Attempts to POST to /api/v1/errors (silently fails if unavailable). + * Buffers in sessionStorage as a ring buffer (max 50 entries). + */ +export function logError(error: Error | string, context?: Record): void { + const message = typeof error === 'string' ? error : error.message; + const stack = typeof error === 'string' ? undefined : error.stack; + + const entry: ErrorEntry = { + timestamp: new Date().toISOString(), + message, + stack, + context, + url: typeof window !== 'undefined' ? window.location.href : undefined, + userAgent: typeof navigator !== 'undefined' ? navigator.userAgent : undefined, + }; + + // Always log to console + console.error('[ErrorLogger]', message, { stack, context, url: entry.url }); + + // Buffer in sessionStorage + addToBuffer(entry); + + // Attempt to send to backend (fire-and-forget, silently fail) + try { + fetch('/api/v1/errors', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + credentials: 'include', + body: JSON.stringify(entry), + }).catch(() => { + // Silently fail — endpoint may not exist or network may be down + }); + } catch { + // Silently fail + } +} + +/** + * Retrieve the buffered errors from sessionStorage. + */ +export function getErrorBuffer(): ErrorEntry[] { + return getBuffer(); +} + +/** + * Clear the error buffer from sessionStorage. + */ +export function clearErrorBuffer(): void { + try { + sessionStorage.removeItem(STORAGE_KEY); + } catch { + // Silently ignore + } +} + +export default { logError, getErrorBuffer, clearErrorBuffer };