Improve error handling and stability across the application

Refactor error handling in `useEntityVersions` and `useSearch` hooks, enhance `NotificationService` with better error extraction and logging, and implement critical fallback mechanisms in the `detect-location` function's rate limit cleanup. Update CORS configuration in `upload-image` function for stricter origin checks and better security. Replit-Commit-Author: Agent Replit-Commit-Session-Id: f4df1950-6410-48d0-b2de-f4096732504b Replit-Commit-Checkpoint-Type: intermediate_checkpoint
2025-12-21 18:31:12 -05:00 · 2025-10-08 20:23:01 +00:00
parent 26aeb46c8a
commit b580db3fb0
6 changed files with 294 additions and 87 deletions
--- a/supabase/functions/detect-location/index.ts
+++ b/supabase/functions/detect-location/index.ts
@@ -17,10 +17,16 @@ const RATE_LIMIT_WINDOW = 60000; // 1 minute in milliseconds
 const MAX_REQUESTS = 10; // 10 requests per minute per IP
 const MAX_MAP_SIZE = 10000; // Maximum number of IPs to track

+// Cleanup failure tracking to prevent silent failures
+let cleanupFailureCount = 0;
+const MAX_CLEANUP_FAILURES = 5; // Threshold before forcing drastic cleanup
+const CLEANUP_FAILURE_RESET_INTERVAL = 300000; // Reset failure count every 5 minutes
+
 function cleanupExpiredEntries() {
  try {
    const now = Date.now();
    let deletedCount = 0;
+    const mapSizeBefore = rateLimitMap.size;
    
    for (const [ip, data] of rateLimitMap.entries()) {
      if (now > data.resetAt) {
@@ -29,14 +35,74 @@ function cleanupExpiredEntries() {
      }
    }
    
+    // Log cleanup activity for monitoring
    if (deletedCount > 0) {
-      console.log(`Cleaned up ${deletedCount} expired rate limit entries`);
+      console.log(`[Cleanup] Removed ${deletedCount} expired entries. Map size: ${mapSizeBefore} -> ${rateLimitMap.size}`);
    }
+    
+    // Reset failure count on successful cleanup
+    if (cleanupFailureCount > 0) {
+      console.log(`[Cleanup] Successful cleanup after ${cleanupFailureCount} previous failures. Resetting failure count.`);
+      cleanupFailureCount = 0;
+    }
+    
  } catch (error) {
-    console.error('Error during cleanup:', error);
+    // CRITICAL: Increment failure counter and log detailed error information
+    cleanupFailureCount++;
+    
+    const errorMessage = error instanceof Error ? error.message : String(error);
+    const errorStack = error instanceof Error ? error.stack : 'No stack trace available';
+    
+    console.error(`[Cleanup Error] Cleanup failed (attempt ${cleanupFailureCount}/${MAX_CLEANUP_FAILURES})`);
+    console.error(`[Cleanup Error] Error message: ${errorMessage}`);
+    console.error(`[Cleanup Error] Stack trace: ${errorStack}`);
+    console.error(`[Cleanup Error] Current map size: ${rateLimitMap.size}`);
+    
+    // FALLBACK MECHANISM: If cleanup fails repeatedly, force clear to prevent memory leak
+    if (cleanupFailureCount >= MAX_CLEANUP_FAILURES) {
+      console.error(`[Cleanup CRITICAL] Cleanup has failed ${cleanupFailureCount} times consecutively!`);
+      console.error(`[Cleanup CRITICAL] Forcing emergency cleanup to prevent memory leak...`);
+      
+      try {
+        // Emergency: Clear oldest 50% of entries to prevent unbounded growth
+        const entriesToClear = Math.floor(rateLimitMap.size * 0.5);
+        const sortedEntries = Array.from(rateLimitMap.entries())
+          .sort((a, b) => a[1].resetAt - b[1].resetAt);
+        
+        let clearedCount = 0;
+        for (let i = 0; i < entriesToClear && i < sortedEntries.length; i++) {
+          rateLimitMap.delete(sortedEntries[i][0]);
+          clearedCount++;
+        }
+        
+        console.warn(`[Cleanup CRITICAL] Emergency cleanup completed. Cleared ${clearedCount} entries. Map size: ${rateLimitMap.size}`);
+        
+        // Reset failure count after emergency cleanup
+        cleanupFailureCount = 0;
+        
+      } catch (emergencyError) {
+        // Last resort: If even emergency cleanup fails, clear everything
+        console.error(`[Cleanup CRITICAL] Emergency cleanup failed! Clearing entire rate limit map.`);
+        console.error(`[Cleanup CRITICAL] Emergency error: ${emergencyError}`);
+        
+        const originalSize = rateLimitMap.size;
+        rateLimitMap.clear();
+        
+        console.warn(`[Cleanup CRITICAL] Cleared entire rate limit map (${originalSize} entries) to prevent memory leak.`);
+        cleanupFailureCount = 0;
+      }
+    }
  }
 }

+// Reset cleanup failure count periodically to avoid permanent emergency state
+setInterval(() => {
+  if (cleanupFailureCount > 0) {
+    console.warn(`[Cleanup] Resetting failure count (was ${cleanupFailureCount}) after timeout period.`);
+    cleanupFailureCount = 0;
+  }
+}, CLEANUP_FAILURE_RESET_INTERVAL);
+
 function checkRateLimit(ip: string): { allowed: boolean; retryAfter?: number } {
  const now = Date.now();
  const existing = rateLimitMap.get(ip);
@@ -59,15 +125,41 @@ function checkRateLimit(ip: string): { allowed: boolean; retryAfter?: number } {
    
    // If still at capacity after cleanup, remove oldest entries (LRU eviction)
    if (rateLimitMap.size >= MAX_MAP_SIZE) {
-      const toDelete = Math.floor(MAX_MAP_SIZE * 0.3); // Remove 30% of entries
-      const sortedEntries = Array.from(rateLimitMap.entries())
-        .sort((a, b) => a[1].resetAt - b[1].resetAt);
-      
-      for (let i = 0; i < toDelete && i < sortedEntries.length; i++) {
-        rateLimitMap.delete(sortedEntries[i][0]);
+      try {
+        const toDelete = Math.floor(MAX_MAP_SIZE * 0.3); // Remove 30% of entries
+        const sortedEntries = Array.from(rateLimitMap.entries())
+          .sort((a, b) => a[1].resetAt - b[1].resetAt);
+        
+        let deletedCount = 0;
+        for (let i = 0; i < toDelete && i < sortedEntries.length; i++) {
+          rateLimitMap.delete(sortedEntries[i][0]);
+          deletedCount++;
+        }
+        
+        console.warn(`[Rate Limit] Map reached ${MAX_MAP_SIZE} entries. Cleared ${deletedCount} oldest entries. New size: ${rateLimitMap.size}`);
+      } catch (evictionError) {
+        // CRITICAL: LRU eviction failed - log error and attempt emergency clear
+        console.error(`[Rate Limit CRITICAL] LRU eviction failed! Error: ${evictionError}`);
+        console.error(`[Rate Limit CRITICAL] Map size: ${rateLimitMap.size}`);
+        
+        try {
+          // Emergency: Clear first 30% of entries without sorting
+          const targetSize = Math.floor(MAX_MAP_SIZE * 0.7);
+          const keysToDelete: string[] = [];
+          
+          for (const [key] of rateLimitMap.entries()) {
+            if (rateLimitMap.size <= targetSize) break;
+            keysToDelete.push(key);
+          }
+          
+          keysToDelete.forEach(key => rateLimitMap.delete(key));
+          
+          console.warn(`[Rate Limit CRITICAL] Emergency eviction cleared ${keysToDelete.length} entries. New size: ${rateLimitMap.size}`);
+        } catch (emergencyError) {
+          console.error(`[Rate Limit CRITICAL] Emergency eviction also failed! Clearing entire map. Error: ${emergencyError}`);
+          rateLimitMap.clear();
+        }
      }
-      
-      console.warn(`Rate limit map reached ${MAX_MAP_SIZE} entries. Cleared ${toDelete} oldest entries.`);
    }
  }
  
@@ -166,7 +258,13 @@ serve(async (req) => {
    );

  } catch (error) {
-    console.error('Error detecting location:', error);
+    // Enhanced error logging for better visibility and debugging
+    const errorMessage = error instanceof Error ? error.message : String(error);
+    const errorStack = error instanceof Error ? error.stack : 'No stack trace available';
+    
+    console.error('[Location Detection Error] Request failed');
+    console.error(`[Location Detection Error] Message: ${errorMessage}`);
+    console.error(`[Location Detection Error] Stack: ${errorStack}`);
    
    // Return default (metric) with 500 status to indicate error occurred
    // This allows proper error monitoring while still providing fallback data
@@ -179,7 +277,7 @@ serve(async (req) => {
    return new Response(
      JSON.stringify({
        ...defaultResult,
-        error: error instanceof Error ? error.message : 'Failed to detect location',
+        error: errorMessage,
        fallback: true
      }),
      { 
--- a/supabase/functions/upload-image/index.ts
+++ b/supabase/functions/upload-image/index.ts
@@ -2,7 +2,12 @@ import { serve } from "https://deno.land/std@0.168.0/http/server.ts"
 import { createClient } from 'https://esm.sh/@supabase/supabase-js@2'

 // Environment-aware CORS configuration
-const getAllowedOrigin = (requestOrigin: string | null): string => {
+const getAllowedOrigin = (requestOrigin: string | null): string | null => {
+  // If no origin header, it's not a CORS request (same-origin or server-to-server)
+  if (!requestOrigin) {
+    return null;
+  }
+
  const environment = Deno.env.get('ENVIRONMENT') || 'development';
  
  // Production allowlist - configure via ALLOWED_ORIGINS environment variable
@@ -10,36 +15,44 @@ const getAllowedOrigin = (requestOrigin: string | null): string => {
  const allowedOriginsEnv = Deno.env.get('ALLOWED_ORIGINS') || '';
  const allowedOrigins = allowedOriginsEnv.split(',').filter(origin => origin.trim());
  
-  // In development, allow localhost and Replit domains
+  // In development, only allow localhost and Replit domains - nothing else
  if (environment === 'development') {
-    if (requestOrigin) {
-      if (
-        requestOrigin.includes('localhost') ||
-        requestOrigin.includes('127.0.0.1') ||
-        requestOrigin.includes('.repl.co') ||
-        requestOrigin.includes('.replit.dev')
-      ) {
-        return requestOrigin;
-      }
+    if (
+      requestOrigin.includes('localhost') ||
+      requestOrigin.includes('127.0.0.1') ||
+      requestOrigin.includes('.repl.co') ||
+      requestOrigin.includes('.replit.dev')
+    ) {
+      return requestOrigin;
    }
-    return '*';
+    // Origin not allowed in development - log and deny
+    console.warn(`[CORS] Origin not allowed in development mode: ${requestOrigin}`);
+    return null;
  }
  
  // In production, only allow specific domains from environment variable
-  if (requestOrigin && allowedOrigins.includes(requestOrigin)) {
+  if (allowedOrigins.includes(requestOrigin)) {
    return requestOrigin;
  }
  
-  // Default to first allowed origin for production, or deny if none configured
-  return allowedOrigins.length > 0 ? allowedOrigins[0] : requestOrigin || '*';
+  // Origin not allowed in production - log and deny
+  console.warn(`[CORS] Origin not allowed in production mode: ${requestOrigin}`);
+  return null;
 };

-const getCorsHeaders = (requestOrigin: string | null) => ({
-  'Access-Control-Allow-Origin': getAllowedOrigin(requestOrigin),
-  'Access-Control-Allow-Headers': 'authorization, x-client-info, apikey, content-type',
-  'Access-Control-Allow-Methods': 'GET, POST, DELETE, OPTIONS',
-  'Access-Control-Allow-Credentials': 'true',
-});
+const getCorsHeaders = (allowedOrigin: string | null): Record<string, string> => {
+  // If no allowed origin, return empty headers (no CORS access)
+  if (!allowedOrigin) {
+    return {};
+  }
+  
+  return {
+    'Access-Control-Allow-Origin': allowedOrigin,
+    'Access-Control-Allow-Headers': 'authorization, x-client-info, apikey, content-type',
+    'Access-Control-Allow-Methods': 'GET, POST, DELETE, OPTIONS',
+    'Access-Control-Allow-Credentials': 'true',
+  };
+};

 // Helper to create authenticated Supabase client
 const createAuthenticatedSupabaseClient = (authHeader: string) => {
@@ -57,7 +70,24 @@ const createAuthenticatedSupabaseClient = (authHeader: string) => {

 serve(async (req) => {
  const requestOrigin = req.headers.get('origin');
-  const corsHeaders = getCorsHeaders(requestOrigin);
+  const allowedOrigin = getAllowedOrigin(requestOrigin);
+  
+  // Check if this is a CORS request with a disallowed origin
+  if (requestOrigin && !allowedOrigin) {
+    console.error(`[CORS] Request rejected for disallowed origin: ${requestOrigin}`);
+    return new Response(
+      JSON.stringify({ 
+        error: 'Origin not allowed',
+        message: 'The origin of this request is not allowed to access this resource'
+      }),
+      { 
+        status: 403,
+        headers: { 'Content-Type': 'application/json' }
+      }
+    );
+  }
+  
+  const corsHeaders = getCorsHeaders(allowedOrigin);
  
  // Handle CORS preflight requests
  if (req.method === 'OPTIONS') {