fix: improve error handling and stability - no logout on transient errors, add ErrorBoundary, global error logging
This commit is contained in:
+21
-3
@@ -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}")
|
||||
|
||||
@@ -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)
|
||||
@@ -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 (
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<AppRouter />
|
||||
<ErrorBoundary>
|
||||
<AppRouter />
|
||||
</ErrorBoundary>
|
||||
</QueryClientProvider>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<string, string[]> = {};
|
||||
|
||||
@@ -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<ErrorBoundaryProps, ErrorBoundaryState> {
|
||||
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 (
|
||||
<div
|
||||
style={{
|
||||
display: 'flex',
|
||||
flexDirection: 'column',
|
||||
alignItems: 'center',
|
||||
justifyContent: 'center',
|
||||
minHeight: '300px',
|
||||
padding: '2rem',
|
||||
textAlign: 'center',
|
||||
}}
|
||||
>
|
||||
<h2 style={{ marginBottom: '0.5rem', color: '#dc2626' }}>
|
||||
Something went wrong
|
||||
</h2>
|
||||
<p style={{ marginBottom: '1rem', color: '#6b7280', maxWidth: '400px' }}>
|
||||
An unexpected error occurred. You can try again or refresh the page.
|
||||
</p>
|
||||
<details style={{ marginBottom: '1rem', maxWidth: '600px', color: '#9ca3af' }}>
|
||||
<summary style={{ cursor: 'pointer', fontSize: '0.875rem' }}>
|
||||
Error details
|
||||
</summary>
|
||||
<pre
|
||||
style={{
|
||||
marginTop: '0.5rem',
|
||||
fontSize: '0.75rem',
|
||||
overflow: 'auto',
|
||||
textAlign: 'left',
|
||||
}}
|
||||
>
|
||||
{this.state.error.message}
|
||||
{this.state.error.stack ? `\n\n${this.state.error.stack}` : ''}
|
||||
</pre>
|
||||
</details>
|
||||
<button
|
||||
onClick={this.handleRetry}
|
||||
style={{
|
||||
padding: '0.5rem 1.5rem',
|
||||
backgroundColor: '#2563eb',
|
||||
color: 'white',
|
||||
border: 'none',
|
||||
borderRadius: '0.375rem',
|
||||
cursor: 'pointer',
|
||||
fontSize: '0.875rem',
|
||||
fontWeight: 500,
|
||||
}}
|
||||
>
|
||||
Retry
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return this.props.children;
|
||||
}
|
||||
}
|
||||
|
||||
export default ErrorBoundary;
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, any>;
|
||||
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<string, any>): 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 };
|
||||
Reference in New Issue
Block a user