feat: Implement comprehensive error handling

2025-12-20 10:31:13 -05:00 · 2025-11-04 17:34:16 +00:00
parent 2a2f172c3b
commit 87589ee08f
3 changed files with 675 additions and 14 deletions
--- a/docs/ERROR_HANDLING_GUIDE.md
+++ b/docs/ERROR_HANDLING_GUIDE.md
@@ -0,0 +1,589 @@
+# Error Handling Guide
+
+This guide outlines the standardized error handling patterns used throughout ThrillWiki to ensure consistent, debuggable, and user-friendly error management.
+
+## Core Principles
+
+1. **All errors must be logged** - Never silently swallow errors
+2. **Provide context** - Include relevant metadata for debugging
+3. **User-friendly messages** - Show clear, actionable error messages to users
+4. **Preserve error chains** - Don't lose original error information
+5. **Use structured logging** - Avoid raw `console.*` statements
+
+## When to Use What
+
+### `handleError()` - Application Errors (User-Facing)
+
+Use `handleError()` for errors that affect user operations and should be visible in the Admin Panel.
+
+**When to use:**
+- Database operation failures
+- API call failures
+- Form submission errors
+- Authentication/authorization failures
+- Any error that impacts user workflows
+
+**Example:**
+```typescript
+import { handleError } from '@/lib/errorHandler';
+import { useAuth } from '@/hooks/useAuth';
+
+try {
+  await supabase.from('parks').insert(parkData);
+  handleSuccess('Park Created', 'Your park has been added successfully');
+} catch (error) {
+  handleError(error, {
+    action: 'Create Park',
+    userId: user?.id,
+    metadata: { parkName: parkData.name }
+  });
+  throw error; // Re-throw for parent error boundaries
+}
+```
+
+**Key features:**
+- Logs to `request_metadata` table with full context
+- Shows user-friendly toast with error reference ID
+- Captures breadcrumbs (last 10 user actions)
+- Visible in Admin Panel at `/admin/error-monitoring`
+
+### `logger.*` - Development & Debugging Logs
+
+Use `logger.*` for information that helps developers debug issues without sending data to the database.
+
+**When to use:**
+- Development debugging information
+- Performance monitoring
+- Expected failures that don't need Admin Panel visibility
+- Component lifecycle events
+- Non-critical informational messages
+
+**Available methods:**
+```typescript
+import { logger } from '@/lib/logger';
+
+// Development only - not logged in production
+logger.log('Component mounted', { props });
+logger.info('User action completed', { action: 'click' });
+logger.warn('Deprecated API used', { api: 'oldMethod' });
+logger.debug('State updated', { newState });
+
+// Always logged - even in production
+logger.error('Critical failure', { context });
+
+// Specialized logging
+logger.performance('ComponentName', durationMs);
+logger.moderationAction('approve', itemId, durationMs);
+```
+
+**Example - Expected periodic failures:**
+```typescript
+// Don't show toast or log to Admin Panel for expected periodic failures
+try {
+  await supabase.rpc('release_expired_locks');
+} catch (error) {
+  logger.debug('Periodic lock release failed', { 
+    operation: 'release_expired_locks',
+    error: getErrorMessage(error)
+  });
+}
+```
+
+### `toast.*` - User Notifications
+
+Use toast notifications directly for informational messages, warnings, or confirmations.
+
+**When to use:**
+- Success confirmations (use `handleSuccess()` helper)
+- Informational messages
+- Non-error warnings
+- User confirmations
+
+**Example:**
+```typescript
+import { handleSuccess, handleInfo } from '@/lib/errorHandler';
+
+// Success messages
+handleSuccess('Changes Saved', 'Your profile has been updated');
+
+// Informational messages
+handleInfo('Processing', 'Your request is being processed');
+
+// Custom toast for special cases
+toast.info('Feature Coming Soon', {
+  description: 'This feature will be available next month',
+  duration: 4000
+});
+```
+
+### ❌ `console.*` - NEVER USE DIRECTLY
+
+**DO NOT USE** `console.*` statements in application code. They are blocked by ESLint.
+
+```typescript
+// ❌ WRONG - Will fail ESLint check
+console.log('User clicked button');
+console.error('Database error:', error);
+
+// ✅ CORRECT - Use logger or handleError
+logger.log('User clicked button');
+handleError(error, { action: 'Database Operation', userId });
+```
+
+**The only exceptions:**
+- Inside `src/lib/logger.ts` itself
+- Edge function logging (use `edgeLogger.*`)
+- Test files (*.test.ts, *.test.tsx)
+
+## Error Handling Patterns
+
+### Pattern 1: Component/Hook Errors (Most Common)
+
+For errors in components or custom hooks that affect user operations:
+
+```typescript
+import { handleError } from '@/lib/errorHandler';
+import { useAuth } from '@/hooks/useAuth';
+
+const MyComponent = () => {
+  const { user } = useAuth();
+
+  const handleSubmit = async (data: FormData) => {
+    try {
+      await saveData(data);
+      handleSuccess('Saved', 'Your changes have been saved');
+    } catch (error) {
+      handleError(error, {
+        action: 'Save Form Data',
+        userId: user?.id,
+        metadata: { formType: 'parkEdit' }
+      });
+      throw error; // Re-throw for error boundaries
+    }
+  };
+};
+```
+
+**Key points:**
+- Always include descriptive action name
+- Include userId when available
+- Add relevant metadata for debugging
+- Re-throw after handling to let error boundaries catch it
+
+### Pattern 2: TanStack Query Errors
+
+For errors within React Query hooks:
+
+```typescript
+import { useQuery } from '@tanstack/react-query';
+import { handleError } from '@/lib/errorHandler';
+
+const { data, error, isLoading } = useQuery({
+  queryKey: ['parks', parkId],
+  queryFn: async () => {
+    const { data, error } = await supabase
+      .from('parks')
+      .select('*')
+      .eq('id', parkId)
+      .single();
+    
+    if (error) {
+      handleError(error, {
+        action: 'Fetch Park Details',
+        userId: user?.id,
+        metadata: { parkId }
+      });
+      throw error;
+    }
+    
+    return data;
+  }
+});
+
+// Handle error state in UI
+if (error) {
+  return <ErrorState message="Failed to load park" />;
+}
+```
+
+### Pattern 3: Expected/Recoverable Errors
+
+For operations that may fail expectedly and should be logged but not shown to users:
+
+```typescript
+import { logger } from '@/lib/logger';
+import { getErrorMessage } from '@/lib/errorHandler';
+
+// Background operation that may fail without impacting user
+const syncCache = async () => {
+  try {
+    await performCacheSync();
+  } catch (error) {
+    // Log for debugging without user notification
+    logger.warn('Cache sync failed', { 
+      operation: 'syncCache',
+      error: getErrorMessage(error)
+    });
+    // Continue execution - cache sync is non-critical
+  }
+};
+```
+
+### Pattern 4: Error Boundaries (Top-Level)
+
+React Error Boundaries catch unhandled component errors:
+
+```typescript
+import { Component, ReactNode } from 'react';
+import { handleError } from '@/lib/errorHandler';
+
+class ErrorBoundary extends Component<
+  { children: ReactNode },
+  { hasError: boolean }
+> {
+  static getDerivedStateFromError() {
+    return { hasError: true };
+  }
+
+  componentDidCatch(error: Error, errorInfo: React.ErrorInfo) {
+    handleError(error, {
+      action: 'Component Error Boundary',
+      metadata: { 
+        componentStack: errorInfo.componentStack 
+      }
+    });
+  }
+
+  render() {
+    if (this.state.hasError) {
+      return <ErrorFallback />;
+    }
+    return this.props.children;
+  }
+}
+```
+
+### Pattern 5: Preserve Error Context in Chains
+
+When catching and re-throwing errors, preserve the original error information:
+
+```typescript
+// ❌ WRONG - Loses original error
+try {
+  await operation();
+} catch (error) {
+  throw new Error('Operation failed'); // Original error lost!
+}
+
+// ❌ WRONG - Silent catch loses context
+const data = await fetch(url)
+  .then(res => res.json())
+  .catch(() => ({ message: 'Failed' })); // Error details lost!
+
+// ✅ CORRECT - Preserve and log error
+try {
+  const response = await fetch(url);
+  if (!response.ok) {
+    const errorData = await response.json().catch((parseError) => {
+      logger.warn('Failed to parse error response', { 
+        error: getErrorMessage(parseError),
+        status: response.status
+      });
+      return { message: 'Request failed' };
+    });
+    throw new Error(errorData.message);
+  }
+  return await response.json();
+} catch (error) {
+  handleError(error, {
+    action: 'Fetch Data',
+    userId: user?.id,
+    metadata: { url }
+  });
+  throw error;
+}
+```
+
+## Automatic Breadcrumb Tracking
+
+The application automatically tracks breadcrumbs (last 10 user actions) to provide context for errors.
+
+### Automatic Tracking (No Code Needed)
+
+1. **API Calls** - All Supabase operations are tracked automatically via the wrapped client
+2. **Navigation** - Route changes are tracked automatically
+3. **Mutation Errors** - TanStack Query mutations log failures automatically
+
+### Manual Breadcrumb Tracking
+
+Add breadcrumbs for important user actions:
+
+```typescript
+import { breadcrumb } from '@/lib/errorBreadcrumbs';
+
+// Navigation breadcrumb (usually automatic)
+breadcrumb.navigation('/parks/123', '/parks');
+
+// User action breadcrumb
+breadcrumb.userAction('clicked submit', 'ParkEditForm', { 
+  parkId: '123' 
+});
+
+// API call breadcrumb (usually automatic via wrapped client)
+breadcrumb.apiCall('/api/parks', 'POST', 200);
+
+// State change breadcrumb
+breadcrumb.stateChange('filter changed', { 
+  filter: 'status=open' 
+});
+```
+
+**When to add manual breadcrumbs:**
+- Critical user actions (form submissions, deletions)
+- Important state changes (filter updates, mode switches)
+- Non-Supabase API calls
+- Complex user workflows
+
+**When NOT to add breadcrumbs:**
+- Inside loops or frequently called functions
+- For every render or effect
+- For trivial state changes
+- Inside already tracked operations
+
+## Edge Function Error Handling
+
+Edge functions use a separate logger to prevent sensitive data exposure:
+
+```typescript
+import { edgeLogger, startRequest, endRequest } from '../_shared/logger.ts';
+
+Deno.serve(async (req) => {
+  const tracking = startRequest();
+  
+  try {
+    // Your edge function logic
+    const result = await performOperation();
+    
+    const duration = endRequest(tracking);
+    edgeLogger.info('Operation completed', {
+      requestId: tracking.requestId,
+      duration
+    });
+    
+    return new Response(JSON.stringify(result), {
+      headers: { 'Content-Type': 'application/json' }
+    });
+  } catch (error) {
+    const duration = endRequest(tracking);
+    
+    edgeLogger.error('Operation failed', {
+      requestId: tracking.requestId,
+      error: error.message,
+      duration
+    });
+    
+    return new Response(
+      JSON.stringify({ 
+        error: 'Operation failed',
+        requestId: tracking.requestId 
+      }),
+      { status: 500, headers: { 'Content-Type': 'application/json' } }
+    );
+  }
+});
+```
+
+**Key features:**
+- Automatic sanitization of sensitive fields
+- Request correlation IDs
+- Structured JSON logging
+- Duration tracking
+
+## Testing Error Handling
+
+### Manual Testing
+
+1. Visit `/test-error-logging` (dev only)
+2. Click "Generate Test Error"
+3. Check Admin Panel at `/admin/error-monitoring`
+4. Verify error appears with:
+   - Full stack trace
+   - Breadcrumbs (including API calls)
+   - Environment context
+   - User information
+
+### Automated Testing
+
+```typescript
+import { handleError } from '@/lib/errorHandler';
+
+describe('Error Handling', () => {
+  it('should log errors to database', async () => {
+    const mockError = new Error('Test error');
+    
+    handleError(mockError, {
+      action: 'Test Action',
+      metadata: { test: true }
+    });
+    
+    // Verify error logged to request_metadata table
+    const { data } = await supabase
+      .from('request_metadata')
+      .select('*')
+      .eq('error_message', 'Test error')
+      .single();
+    
+    expect(data).toBeDefined();
+    expect(data.endpoint).toBe('Test Action');
+  });
+});
+```
+
+## Common Mistakes to Avoid
+
+### ❌ Mistake 1: Silent Error Catching
+```typescript
+// ❌ WRONG
+try {
+  await operation();
+} catch (error) {
+  // Nothing - error disappears!
+}
+
+// ✅ CORRECT
+try {
+  await operation();
+} catch (error) {
+  logger.debug('Expected operation failure', { 
+    operation: 'name',
+    error: getErrorMessage(error)
+  });
+}
+```
+
+### ❌ Mistake 2: Using console.* Directly
+```typescript
+// ❌ WRONG - Blocked by ESLint
+console.log('Debug info', data);
+console.error('Error occurred', error);
+
+// ✅ CORRECT
+logger.log('Debug info', data);
+handleError(error, { action: 'Operation Name', userId });
+```
+
+### ❌ Mistake 3: Not Re-throwing After Handling
+```typescript
+// ❌ WRONG - Error doesn't reach error boundary
+try {
+  await operation();
+} catch (error) {
+  handleError(error, { action: 'Operation' });
+  // Error stops here - error boundary never sees it
+}
+
+// ✅ CORRECT
+try {
+  await operation();
+} catch (error) {
+  handleError(error, { action: 'Operation' });
+  throw error; // Let error boundary handle UI fallback
+}
+```
+
+### ❌ Mistake 4: Generic Error Messages
+```typescript
+// ❌ WRONG - No context
+handleError(error, { action: 'Error' });
+
+// ✅ CORRECT - Descriptive context
+handleError(error, {
+  action: 'Update Park Opening Hours',
+  userId: user?.id,
+  metadata: { 
+    parkId: park.id,
+    parkName: park.name
+  }
+});
+```
+
+### ❌ Mistake 5: Losing Error Context
+```typescript
+// ❌ WRONG
+.catch(() => ({ error: 'Failed' }))
+
+// ✅ CORRECT
+.catch((error) => {
+  logger.warn('Operation failed', { error: getErrorMessage(error) });
+  return { error: 'Failed' };
+})
+```
+
+## Error Monitoring Dashboard
+
+Access the error monitoring dashboard at `/admin/error-monitoring`:
+
+**Features:**
+- Real-time error list with filtering
+- Search by error ID, message, or user
+- Full stack traces
+- Breadcrumb trails showing user actions before error
+- Environment context (browser, device, network)
+- Request metadata (endpoint, method, status)
+
+**Error ID Lookup:**
+Visit `/admin/error-lookup` to search for specific errors by their 8-character reference ID shown to users.
+
+## Related Files
+
+**Core Error Handling:**
+- `src/lib/errorHandler.ts` - Main error handling utilities
+- `src/lib/errorBreadcrumbs.ts` - Breadcrumb tracking system
+- `src/lib/environmentContext.ts` - Environment data capture
+- `src/lib/logger.ts` - Structured logging utility
+- `src/lib/supabaseClient.ts` - Wrapped client with auto-tracking
+
+**Admin Tools:**
+- `src/pages/admin/ErrorMonitoring.tsx` - Error dashboard
+- `src/pages/admin/ErrorLookup.tsx` - Error ID search
+- `src/components/admin/ErrorDetailsModal.tsx` - Error details view
+
+**Edge Functions:**
+- `supabase/functions/_shared/logger.ts` - Edge function logger
+
+**Database:**
+- `request_metadata` table - Stores all error logs
+- `request_breadcrumbs` table - Stores breadcrumb trails
+- `log_request_metadata` RPC - Logs errors from client
+
+## Summary
+
+**Golden Rules:**
+1. ✅ Use `handleError()` for user-facing application errors
+2. ✅ Use `logger.*` for development debugging and expected failures
+3. ✅ Use `toast.*` for success/info notifications
+4. ✅ Use `edgeLogger.*` in edge functions
+5. ❌ NEVER use `console.*` directly in application code
+6. ✅ Always preserve error context when catching
+7. ✅ Re-throw errors after handling for error boundaries
+8. ✅ Include descriptive action names and metadata
+9. ✅ Manual breadcrumbs for critical user actions only
+10. ✅ Test error handling in Admin Panel
+
+**Quick Reference:**
+```typescript
+// Application error (user-facing)
+handleError(error, { action: 'Action Name', userId, metadata });
+
+// Debug log (development only)
+logger.debug('Debug info', { context });
+
+// Expected failure (log but don't show toast)
+logger.warn('Expected failure', { error: getErrorMessage(error) });
+
+// Success notification
+handleSuccess('Title', 'Description');
+
+// Edge function error
+edgeLogger.error('Error message', { requestId, error: error.message });
+```
--- a/src/hooks/useModerationQueue.ts
+++ b/src/hooks/useModerationQueue.ts
@@ -4,6 +4,7 @@ import { useAuth } from './useAuth';
 import { useToast } from './use-toast';
 import { getErrorMessage } from '@/lib/errorHandler';
 import { getSubmissionTypeLabel } from '@/lib/moderation/entities';
+import { logger } from '@/lib/logger';

 interface QueuedSubmission {
  submission_id: string;
@@ -45,7 +46,11 @@ export const useModerationQueue = (config?: UseModerationQueueConfig) => {
      try {
        await supabase.rpc('release_expired_locks');
      } catch (error: unknown) {
-        // Silent failure - lock release happens periodically
+        // Log expected periodic failure for debugging without user toast
+        logger.debug('Periodic lock release failed', { 
+          operation: 'release_expired_locks',
+          error: getErrorMessage(error)
+        });
      }
    }, 120000); // 2 minutes

@@ -83,9 +88,13 @@ export const useModerationQueue = (config?: UseModerationQueueConfig) => {
      });
    }
  } catch (error: unknown) {
-      // Silent failure - stats are refreshed periodically
+    // Log stats fetch failure for debugging without user toast
+    logger.debug('Queue stats fetch failed', { 
+      operation: 'fetchStats',
+      error: getErrorMessage(error)
+    });
  }
-  }, [user]);
+}, [user]);

  // Start countdown timer for lock expiry with improved memory leak prevention
  const startLockTimer = useCallback((expiresAt: Date) => {
@@ -348,7 +357,13 @@ export const useModerationQueue = (config?: UseModerationQueueConfig) => {
      });

      if (!response.ok) {
-        const errorData = await response.json().catch(() => ({ message: 'Failed to claim submission' }));
+        const errorData = await response.json().catch((parseError) => {
+          logger.warn('Failed to parse claim error response', { 
+            error: getErrorMessage(parseError),
+            status: response.status
+          });
+          return { message: 'Failed to claim submission' };
+        });
        throw new Error(errorData.message || 'Failed to claim submission');
      }

--- a/src/lib/supabaseClient.ts
+++ b/src/lib/supabaseClient.ts
@@ -1,8 +1,65 @@
 /**
- * Central Supabase Client Export
+ * Central Supabase Client Export with Automatic Breadcrumb Tracking
 * 
 * All application code should import from this file instead of the base client.
- * This provides a central point for potential future enhancements without breaking imports.
+ * This wrapper automatically tracks all database operations as breadcrumbs for error debugging.
 */

-export { supabase } from '@/integrations/supabase/client';
+import { supabase as baseClient } from '@/integrations/supabase/client';
+import { breadcrumb } from './errorBreadcrumbs';
+
+type SupabaseClient = typeof baseClient;
+
+/**
+ * Wrap Supabase client to automatically track API calls as breadcrumbs
+ */
+function wrapSupabaseClient(client: SupabaseClient): SupabaseClient {
+  return new Proxy(client, {
+    get(target, prop: string | symbol) {
+      const value = target[prop as keyof typeof target];
+      
+      // Only wrap 'from' and 'rpc' methods for database operations
+      if ((prop === 'from' || prop === 'rpc') && typeof value === 'function') {
+        return (...args: any[]) => {
+          const result = (value as any).apply(target, args);
+          const endpoint = prop === 'from' ? `/table/${args[0]}` : `/rpc/${args[0]}`;
+          
+          // Return a proxy for chained query methods
+          return new Proxy(result, {
+            get(queryTarget: any, queryProp: string | symbol) {
+              const queryValue = queryTarget[queryProp];
+              
+              // If it's a function, wrap it to track the call
+              if (typeof queryValue === 'function') {
+                return async (...queryArgs: any[]) => {
+                  try {
+                    const response = await queryValue.apply(queryTarget, queryArgs);
+                    
+                    // Log breadcrumb after response
+                    breadcrumb.apiCall(
+                      endpoint,
+                      String(queryProp).toUpperCase(),
+                      response.error ? 400 : 200
+                    );
+                    
+                    return response;
+                  } catch (error) {
+                    // Log breadcrumb for exceptions
+                    breadcrumb.apiCall(endpoint, String(queryProp).toUpperCase(), 500);
+                    throw error;
+                  }
+                };
+              }
+              
+              return queryValue;
+            }
+          });
+        };
+      }
+      
+      return value;
+    }
+  }) as SupabaseClient;
+}
+
+export const supabase = wrapSupabaseClient(baseClient);