From b92a62ebc8c306bd3fd2ad315f12892f2bcfe283 Mon Sep 17 00:00:00 2001 From: "gpt-engineer-app[bot]" <159125892+gpt-engineer-app[bot]@users.noreply.github.com> Date: Thu, 6 Nov 2025 17:43:16 +0000 Subject: [PATCH] feat: Add idempotency to useModerationActions Implement idempotency integration in the useModerationActions hook as per the detailed plan. --- src/hooks/moderation/useModerationActions.ts | 234 +++++++++++++++++-- src/lib/edgeFunctionTracking.ts | 27 ++- src/lib/idempotencyHelpers.ts | 90 +++++++ 3 files changed, 321 insertions(+), 30 deletions(-) create mode 100644 src/lib/idempotencyHelpers.ts diff --git a/src/hooks/moderation/useModerationActions.ts b/src/hooks/moderation/useModerationActions.ts index a8989c1c..a2a47033 100644 --- a/src/hooks/moderation/useModerationActions.ts +++ b/src/hooks/moderation/useModerationActions.ts @@ -6,6 +6,12 @@ import { logger } from '@/lib/logger'; import { getErrorMessage, handleError, isSupabaseConnectionError } from '@/lib/errorHandler'; // Validation removed from client - edge function is single source of truth import { invokeWithTracking } from '@/lib/edgeFunctionTracking'; +import { + generateIdempotencyKey, + is409Conflict, + getRetryAfter, + sleep +} from '@/lib/idempotencyHelpers'; import type { User } from '@supabase/supabase-js'; import type { ModerationItem } from '@/types/moderation'; @@ -42,6 +48,105 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio const { toast } = useToast(); const queryClient = useQueryClient(); + /** + * Invoke edge function with idempotency key and 409 retry logic + * + * Wraps invokeWithTracking with: + * - Automatic idempotency key generation + * - Special handling for 409 Conflict responses + * - Exponential backoff retry for conflicts + * + * @param functionName - Edge function to invoke + * @param payload - Request payload + * @param idempotencyKey - Pre-generated idempotency key + * @param userId - User ID for tracking + * @param maxConflictRetries - Max retries for 409 responses (default: 3) + * @returns Result with data, error, requestId, etc. + */ + async function invokeWithIdempotency( + functionName: string, + payload: any, + idempotencyKey: string, + userId?: string, + maxConflictRetries: number = 3, + timeout: number = 30000 + ): Promise<{ + data: T | null; + error: any; + requestId: string; + duration: number; + attempts?: number; + cached?: boolean; + conflictRetries?: number; + }> { + let conflictRetries = 0; + let lastError: any = null; + + while (conflictRetries <= maxConflictRetries) { + const result = await invokeWithTracking( + functionName, + payload, + userId, + undefined, + undefined, + timeout, + { maxAttempts: 3, baseDelay: 1500 }, // Standard retry for transient errors + { 'X-Idempotency-Key': idempotencyKey } // NEW: Custom header + ); + + // Success or non-409 error - return immediately + if (!result.error || !is409Conflict(result.error)) { + // Check if response indicates cached result + const isCached = result.data && typeof result.data === 'object' && 'cached' in result.data + ? (result.data as any).cached + : false; + + return { + ...result, + cached: isCached, + conflictRetries, + }; + } + + // 409 Conflict detected + lastError = result.error; + conflictRetries++; + + if (conflictRetries > maxConflictRetries) { + // Max retries exceeded + logger.error('Max 409 conflict retries exceeded', { + functionName, + idempotencyKey: idempotencyKey.substring(0, 32) + '...', + conflictRetries, + submissionId: payload.submissionId, + }); + break; + } + + // Extract retry-after from error and wait + const retryAfterSeconds = getRetryAfter(result.error); + const retryDelayMs = retryAfterSeconds * 1000; + + logger.log(`409 Conflict detected, retrying after ${retryAfterSeconds}s (attempt ${conflictRetries}/${maxConflictRetries})`, { + functionName, + idempotencyKey: idempotencyKey.substring(0, 32) + '...', + retryAfterSeconds, + }); + + await sleep(retryDelayMs); + } + + // All retries exhausted + return { + data: null, + error: lastError || { message: 'Unknown conflict retry error' }, + requestId: 'conflict-retry-failed', + duration: 0, + attempts: 0, + conflictRetries, + }; + } + /** * Perform moderation action (approve/reject) with optimistic updates */ @@ -138,29 +243,70 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio // Client-side only performs basic UX validation (non-empty, format) in forms. // If server-side validation fails, the edge function returns detailed 400/500 errors. - const { data, error, requestId, attempts } = await invokeWithTracking( + // Generate idempotency key BEFORE calling edge function + const idempotencyKey = generateIdempotencyKey( + 'approval', + item.id, + submissionItems.map((i) => i.id), + config.user?.id || 'unknown' + ); + + logger.log('Generated idempotency key for approval', { + submissionId: item.id, + itemCount: submissionItems.length, + idempotencyKey: idempotencyKey.substring(0, 32) + '...', // Log partial key + }); + + const { + data, + error, + requestId, + attempts, + cached, + conflictRetries + } = await invokeWithIdempotency( 'process-selective-approval', { itemIds: submissionItems.map((i) => i.id), submissionId: item.id, }, + idempotencyKey, config.user?.id, - undefined, - undefined, - 30000, // 30s timeout - { maxAttempts: 3, baseDelay: 1500 } // Critical operation - retry config + 3, // Max 3 conflict retries + 30000 // 30s timeout ); - // Log if retries were needed + // Log retry attempts if (attempts && attempts > 1) { - logger.log(`Approval succeeded after ${attempts} attempts for ${item.id}`); + logger.log(`Approval succeeded after ${attempts} network retries`, { + submissionId: item.id, + requestId, + }); + } + + if (conflictRetries && conflictRetries > 0) { + logger.log(`Resolved 409 conflict after ${conflictRetries} retries`, { + submissionId: item.id, + requestId, + cached: !!cached, + }); } - if (error) throw error; + if (error) { + // Enhance error with context for better UI feedback + if (is409Conflict(error)) { + throw new Error( + 'This approval is being processed by another request. Please wait and try again if it does not complete.' + ); + } + throw error; + } toast({ - title: 'Submission Approved', - description: `Successfully processed ${submissionItems.length} item(s)${requestId ? ` (Request: ${requestId.substring(0, 8)})` : ''}`, + title: cached ? 'Cached Result' : 'Submission Approved', + description: cached + ? `Returned cached result for ${submissionItems.length} item(s)` + : `Successfully processed ${submissionItems.length} item(s)${requestId ? ` (Request: ${requestId.substring(0, 8)})` : ''}`, }); return; } else if (action === 'rejected') { @@ -267,6 +413,7 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio // Enhanced error handling with reference ID and network detection const isNetworkError = isSupabaseConnectionError(error); + const isConflict = is409Conflict(error); // NEW: Detect 409 conflicts const errorMessage = getErrorMessage(error) || `Failed to ${variables.action} content`; // Check if this is a validation error from edge function @@ -276,8 +423,12 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio toast({ title: isNetworkError ? 'Connection Error' : - isValidationError ? 'Validation Failed' : 'Action Failed', - description: errorMessage, + isValidationError ? 'Validation Failed' : + isConflict ? 'Duplicate Request' : // NEW: Conflict title + 'Action Failed', + description: isConflict + ? 'This action is already being processed. Please wait for it to complete.' // NEW: Conflict message + : errorMessage, variant: 'destructive', }); @@ -288,6 +439,7 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio errorId: error.errorId, isNetworkError, isValidationError, + isConflict, // NEW: Log conflict status }); }, onSuccess: (data) => { @@ -490,24 +642,62 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio failedItemsCount = failedItems.length; - const { data, error, requestId, attempts } = await invokeWithTracking( + // Generate idempotency key for retry operation + const idempotencyKey = generateIdempotencyKey( + 'retry', + item.id, + failedItems.map((i) => i.id), + config.user?.id || 'unknown' + ); + + logger.log('Generated idempotency key for retry', { + submissionId: item.id, + itemCount: failedItems.length, + idempotencyKey: idempotencyKey.substring(0, 32) + '...', + }); + + const { + data, + error, + requestId, + attempts, + cached, + conflictRetries + } = await invokeWithIdempotency( 'process-selective-approval', { itemIds: failedItems.map((i) => i.id), submissionId: item.id, }, + idempotencyKey, config.user?.id, - undefined, - undefined, - 30000, - { maxAttempts: 3, baseDelay: 1500 } // Retry for failed items + 3, // Max 3 conflict retries + 30000 // 30s timeout ); if (attempts && attempts > 1) { - logger.log(`Retry succeeded after ${attempts} attempts for ${item.id}`); + logger.log(`Retry succeeded after ${attempts} network retries`, { + submissionId: item.id, + requestId, + }); } - if (error) throw error; + if (conflictRetries && conflictRetries > 0) { + logger.log(`Retry resolved 409 conflict after ${conflictRetries} retries`, { + submissionId: item.id, + requestId, + cached: !!cached, + }); + } + + if (error) { + if (is409Conflict(error)) { + throw new Error( + 'This retry is being processed by another request. Please wait and try again if it does not complete.' + ); + } + throw error; + } // Log audit trail for retry if (user) { @@ -529,8 +719,10 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio } toast({ - title: 'Items Retried', - description: `Successfully retried ${failedItems.length} failed item(s)${requestId ? ` (Request: ${requestId.substring(0, 8)})` : ''}`, + title: cached ? 'Cached Retry Result' : 'Items Retried', + description: cached + ? `Returned cached result for ${failedItems.length} item(s)` + : `Successfully retried ${failedItems.length} failed item(s)${requestId ? ` (Request: ${requestId.substring(0, 8)})` : ''}`, }); logger.log(`✅ Retried ${failedItems.length} failed items for ${item.id}`); diff --git a/src/lib/edgeFunctionTracking.ts b/src/lib/edgeFunctionTracking.ts index 330f5ef2..6616e394 100644 --- a/src/lib/edgeFunctionTracking.ts +++ b/src/lib/edgeFunctionTracking.ts @@ -19,7 +19,10 @@ import { breadcrumb } from './errorBreadcrumbs'; * @param userId - User ID for tracking (optional) * @param parentRequestId - Parent request ID for chaining (optional) * @param traceId - Trace ID for distributed tracing (optional) - * @returns Response data with requestId + * @param timeout - Request timeout in milliseconds (default: 30000) + * @param retryOptions - Optional retry configuration + * @param customHeaders - Custom headers to include in the request (e.g., X-Idempotency-Key) + * @returns Response data with requestId, status, and tracking info */ export async function invokeWithTracking( functionName: string, @@ -27,9 +30,10 @@ export async function invokeWithTracking( userId?: string, parentRequestId?: string, traceId?: string, - timeout: number = 30000, // Default 30s timeout - retryOptions?: Partial // NEW: Optional retry configuration -): Promise<{ data: T | null; error: any; requestId: string; duration: number; attempts?: number }> { + timeout: number = 30000, + retryOptions?: Partial, + customHeaders?: Record +): Promise<{ data: T | null; error: any; requestId: string; duration: number; attempts?: number; status?: number }> { // Configure retry options with defaults const effectiveRetryOptions: RetryOptions = { maxAttempts: retryOptions?.maxAttempts ?? 3, @@ -75,14 +79,16 @@ export async function invokeWithTracking( const { data, error } = await supabase.functions.invoke(functionName, { body: { ...payload, clientRequestId: context.requestId }, signal: controller.signal, + headers: customHeaders, }); clearTimeout(timeoutId); if (error) { - // Enhance error with status for retry logic + // Enhance error with status and context for retry logic const enhancedError = new Error(error.message || 'Edge function error'); (enhancedError as any).status = error.status; + (enhancedError as any).context = error.context; throw enhancedError; } @@ -97,7 +103,7 @@ export async function invokeWithTracking( } ); - return { data: result, error: null, requestId, duration, attempts: attemptCount }; + return { data: result, error: null, requestId, duration, attempts: attemptCount, status: 200 }; } catch (error: unknown) { // Handle AbortError specifically if (error instanceof Error && error.name === 'AbortError') { @@ -110,16 +116,18 @@ export async function invokeWithTracking( requestId: 'timeout', duration: timeout, attempts: attemptCount, + status: 408, }; } const errorMessage = getErrorMessage(error); return { data: null, - error: { message: errorMessage }, + error: { message: errorMessage, status: (error as any)?.status }, requestId: 'unknown', duration: 0, attempts: attemptCount, + status: (error as any)?.status, }; } } @@ -148,6 +156,7 @@ export async function invokeBatchWithTracking( requestId: string; duration: number; attempts?: number; + status?: number; }> > { const traceId = crypto.randomUUID(); @@ -160,8 +169,8 @@ export async function invokeBatchWithTracking( userId, undefined, traceId, - 30000, // default timeout - op.retryOptions // Pass through retry options + 30000, + op.retryOptions ); return { functionName: op.functionName, ...result }; }) diff --git a/src/lib/idempotencyHelpers.ts b/src/lib/idempotencyHelpers.ts new file mode 100644 index 00000000..dbca7b31 --- /dev/null +++ b/src/lib/idempotencyHelpers.ts @@ -0,0 +1,90 @@ +/** + * Idempotency Key Utilities + * + * Provides helper functions for generating and managing idempotency keys + * for moderation operations to prevent duplicate requests. + */ + +/** + * Generate a deterministic idempotency key for a moderation action + * + * Format: action_submissionId_itemIds_userId_timestamp + * Example: approval_abc123_def456_ghi789_user123_1699564800000 + * + * @param action - The moderation action type ('approval', 'rejection', 'retry') + * @param submissionId - The submission ID + * @param itemIds - Array of item IDs being processed + * @param userId - The moderator's user ID + * @returns Deterministic idempotency key + */ +export function generateIdempotencyKey( + action: 'approval' | 'rejection' | 'retry', + submissionId: string, + itemIds: string[], + userId: string +): string { + // Sort itemIds to ensure consistency regardless of order + const sortedItemIds = [...itemIds].sort().join('_'); + + // Include timestamp to allow same moderator to retry after 24h window + const timestamp = Date.now(); + + return `${action}_${submissionId}_${sortedItemIds}_${userId}_${timestamp}`; +} + +/** + * Check if an error is a 409 Conflict (duplicate request) + * + * @param error - Error object to check + * @returns True if error is 409 Conflict + */ +export function is409Conflict(error: unknown): boolean { + if (!error || typeof error !== 'object') return false; + + const errorObj = error as { status?: number; message?: string }; + + // Check status code + if (errorObj.status === 409) return true; + + // Check error message for conflict indicators + const message = errorObj.message?.toLowerCase() || ''; + return message.includes('duplicate request') || + message.includes('already in progress') || + message.includes('race condition'); +} + +/** + * Extract retry-after value from error response + * + * @param error - Error object with potential Retry-After header + * @returns Seconds to wait before retry, defaults to 3 + */ +export function getRetryAfter(error: unknown): number { + if (!error || typeof error !== 'object') return 3; + + const errorObj = error as { + retryAfter?: number; + context?: { headers?: { 'Retry-After'?: string } } + }; + + // Check structured retryAfter field + if (errorObj.retryAfter) return errorObj.retryAfter; + + // Check Retry-After header + const retryAfterHeader = errorObj.context?.headers?.['Retry-After']; + if (retryAfterHeader) { + const seconds = parseInt(retryAfterHeader, 10); + return isNaN(seconds) ? 3 : seconds; + } + + return 3; // Default 3 seconds +} + +/** + * Sleep for a specified duration + * + * @param ms - Milliseconds to sleep + */ +export function sleep(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); +}