Skip to content

⚠️ Error Handling Inconsistency: Multiple Error Handling Patterns Causing Unreliable Error Management #17

@jrevillard

Description

@jrevillard

⚠️ Error Handling Problem: Multiple Error Handling Patterns Causing Unreliable Error Management

Problem Description

The codebase exhibits multiple, inconsistent error handling patterns throughout different layers, making error management unreliable, debugging difficult, and user experience inconsistent. There is no unified approach to error handling, leading to unpredictable behavior and poor error reporting.

Current Problematic Implementation

1. Route Handler Error Handling Variations

Location: /backend/src/routes/ (multiple files)

Pattern 1 - fcmTokens.ts (Consistent but verbose):

} catch (error) {
  logger.error('Error saving FCM token:', { error: error instanceof Error ? error.message : String(error) });
  res.status(500).json({
    success: false,
    message: 'Failed to save FCM token',
  });
}

} catch (error) {
  logger.error('Error fetching FCM tokens:', { error: error instanceof Error ? error.message : String(error) });
  res.status(500).json({
    success: false,
    message: 'Failed to fetch FCM tokens',
  });
}

Pattern 2 - invitations.ts (Using any type):

} catch (error: any) {
  logger.error('Error in invitation processing:', error);
  res.status(500).json({ 
    error: error.message || 'Internal server error' 
  });
}

} catch (error: any) {
  logger.error('Failed to send invitation:', error);
  res.status(400).json({ 
    error: error.message || 'Failed to send invitation' 
  });
}

Pattern 3 - Mixed approaches in same file:

// Some use proper error instance checking
logger.error('Error:', { error: error instanceof Error ? error.message : String(error) });

// Others directly access error properties
res.status(500).json({ error: error.message });

2. Service Layer Error Handling Variations

Pattern 1 - DashboardService.ts (Logging with re-throw):

} catch (error) {
  logger.error('Failed to calculate user stats', { 
    error: error instanceof Error ? error.message : String(error), 
    stack: error instanceof Error ? error.stack : undefined, 
    userId 
  });
  throw error; // Re-throw original error
}

} catch (error) {
  logger.error('Failed to get today trips for user', { 
    error: error instanceof Error ? error.message : String(error), 
    stack: error instanceof Error ? error.stack : undefined, 
    userId 
  });
  throw error; // Re-throw original error
}

Pattern 2 - GroupService.ts (AppError wrapping):

} catch (error) {
  this.logger.error('Create group error:', { error: error instanceof Error ? error.message : String(error) });
  if (error instanceof AppError) {
    throw error; // Re-throw AppError as-is
  }
  throw new AppError('Failed to create group', 500); // Wrap generic errors
}

} catch (error) {
  if (error instanceof AppError) {
    throw error;
  }
  this.logger.error('Join group error:', { error: error instanceof Error ? error.message : String(error) });
  throw new AppError('Failed to join group', 500);
}

Pattern 3 - Silent error handling with fallbacks:

} catch (error) {
  logger.error('Failed to count this week trips', { 
    error: error instanceof Error ? error.message : String(error), 
    userId 
  });
  return 0; // Return fallback value instead of throwing
}

3. Repository Layer Error Handling Variations

Pattern 1 - ActivityLogRepository.ts (Fallback data):

async createActivity(data: CreateActivityData): Promise<ActivityLog> {
  try {
    return await this.prisma.activityLog.create({...});
  } catch {
    // Fallback to mock if table doesn't exist yet
    logger.warn('ActivityLog table not available, returning mock data', { 
      userId: data.userId, 
      actionType: data.actionType 
    });
    const activity: ActivityLog = {
      id: `activity-${Date.now()}`,
      userId: data.userId,
      actionType: data.actionType,
      // ... mock data
    };
    return activity;
  }
}

Pattern 2 - ScheduleSlotRepository.ts (Direct error throwing):

if (!scheduleSlot) {
  throw new Error('Schedule slot not found');
}

if (!vehicle) {
  throw new Error('Vehicle not found');
}

if (existingAssignment) {
  throw new Error('Vehicle is already assigned to this schedule slot');
}

Pattern 3 - No error handling:

// Some repository methods have no try-catch blocks
async findMany(...): Promise<unknown[]> {
  return this.prisma.scheduleSlot.findMany({...});
  // No error handling - errors bubble up
}

4. Error Response Format Inconsistencies

Format 1 - Success pattern:

res.status(500).json({
  success: false,
  message: 'Failed to save FCM token',
});

Format 2 - Error object pattern:

res.status(500).json({ 
  error: error.message || 'Internal server error' 
});

Format 3 - Mixed pattern:

res.status(400).json({ 
  error: error.message,
  details: error.details 
});

Suggested Refactoring Approach

1. Create Unified Error Types

// /backend/src/errors/BaseError.ts
export abstract class BaseError extends Error {
  public readonly statusCode: number;
  public readonly code: string;
  public readonly isOperational: boolean;
  public readonly context?: Record<string, unknown>;

  constructor(
    message: string,
    statusCode: number = 500,
    code: string = 'INTERNAL_ERROR',
    isOperational: boolean = true,
    context?: Record<string, unknown>
  ) {
    super(message);
    this.name = this.constructor.name;
    this.statusCode = statusCode;
    this.code = code;
    this.isOperational = isOperational;
    this.context = context;

    Error.captureStackTrace(this, this.constructor);
  }

  toJSON() {
    return {
      name: this.name,
      message: this.message,
      code: this.code,
      statusCode: this.statusCode,
      isOperational: this.isOperational,
      context: this.context,
      ...(process.env.NODE_ENV === 'development' && { stack: this.stack }),
    };
  }
}

// /backend/src/errors/AppError.ts
export class AppError extends BaseError {
  constructor(
    message: string,
    statusCode: number = 500,
    code?: string,
    context?: Record<string, unknown>
  ) {
    super(message, statusCode, code || 'APP_ERROR', true, context);
  }
}

// /backend/src/errors/ValidationError.ts
export class ValidationError extends BaseError {
  constructor(message: string, field?: string, context?: Record<string, unknown>) {
    super(message, 400, 'VALIDATION_ERROR', true, { field, ...context });
  }
}

// /backend/src/errors/NotFoundError.ts
export class NotFoundError extends BaseError {
  constructor(resource: string, id?: string, context?: Record<string, unknown>) {
    super(
      `${resource}${id ? ` with ID ${id}` : ''} not found`,
      404,
      'NOT_FOUND',
      true,
      { resource, id, ...context }
    );
  }
}

// /backend/src/errors/UnauthorizedError.ts
export class UnauthorizedError extends BaseError {
  constructor(message: string = 'Unauthorized', context?: Record<string, unknown>) {
    super(message, 401, 'UNAUTHORIZED', true, context);
  }
}

// /backend/src/errors/ForbiddenError.ts
export class ForbiddenError extends BaseError {
  constructor(message: string = 'Forbidden', context?: Record<string, unknown>) {
    super(message, 403, 'FORBIDDEN', true, context);
  }
}

// /backend/src/errors/ConflictError.ts
export class ConflictError extends BaseError {
  constructor(message: string, context?: Record<string, unknown>) {
    super(message, 409, 'CONFLICT', true, context);
  }
}

2. Create Error Handling Utilities

// /backend/src/utils/errorHandler.ts
import { createLogger } from '../utils/logger';
import { BaseError } from '../errors/BaseError';

const logger = createLogger('errorHandler');

export const handleError = (error: unknown, context: string): BaseError => {
  // If it's already a BaseError, return as-is
  if (error instanceof BaseError) {
    logger.error('Application error', { 
      error: error.toJSON(),
      context 
    });
    return error;
  }

  // Handle Error instances
  if (error instanceof Error) {
    logger.error('Unhandled Error', { 
      name: error.name,
      message: error.message,
      stack: error.stack,
      context 
    });

    // Convert common error types
    if (error.name === 'ValidationError') {
      return new ValidationError(error.message, undefined, { originalError: error.name });
    }

    if (error.name === 'CastError') {
      return new ValidationError('Invalid data format', undefined, { originalError: error.name });
    }

    return new AppError(error.message, 500, 'UNKNOWN_ERROR', { 
      originalError: error.name,
      stack: error.stack 
    });
  }

  // Handle string errors
  if (typeof error === 'string') {
    logger.error('String error', { error, context });
    return new AppError(error, 500, 'STRING_ERROR');
  }

  // Handle unknown error types
  logger.error('Unknown error type', { error, context });
  return new AppError('An unexpected error occurred', 500, 'UNKNOWN_ERROR', { 
    originalError: typeof error,
    errorData: error 
  });
};

export const asyncHandler = (fn: Function) => {
  return (req: Request, res: Response, next: NextFunction) => {
    Promise.resolve(fn(req, res, next)).catch(next);
  };
};

export const createErrorContext = (req: Request): Record<string, unknown> => {
  return {
    method: req.method,
    url: req.url,
    userAgent: req.get('User-Agent'),
    ip: req.ip,
    userId: (req as any).user?.id,
    timestamp: new Date().toISOString(),
  };
};

3. Create Global Error Handling Middleware

// /backend/src/middleware/errorHandler.ts
import { Request, Response, NextFunction } from 'express';
import { BaseError } from '../errors/BaseError';
import { handleError, createErrorContext } from '../utils/errorHandler';
import { createLogger } from '../utils/logger';

const logger = createLogger('errorMiddleware');

export const errorHandler = (
  error: unknown,
  req: Request,
  res: Response,
  next: NextFunction
): void => {
  const context = createErrorContext(req);
  const appError = handleError(error, `${req.method} ${req.path}`);

  // Log the error
  logger.error('Request error', {
    error: appError.toJSON(),
    request: context,
  });

  // Send consistent error response
  const errorResponse = {
    success: false,
    error: {
      code: appError.code,
      message: appError.message,
      ...(appError.context && { context: appError.context }),
      ...(process.env.NODE_ENV === 'development' && {
        stack: appError.stack,
        name: appError.name,
      }),
    },
    timestamp: new Date().toISOString(),
    requestId: req.headers['x-request-id'] || 'unknown',
  };

  res.status(appError.statusCode).json(errorResponse);
};

export const notFoundHandler = (req: Request, res: Response): void => {
  const error = new NotFoundError('Route', req.path);
  res.status(404).json({
    success: false,
    error: {
      code: error.code,
      message: error.message,
    },
    timestamp: new Date().toISOString(),
  });
};

4. Refactor Service Layer Error Handling

// /backend/src/services/DashboardService.ts (Refactored)
export class DashboardService {
  // ... constructor

  async calculateUserStats(userId: string): Promise<DashboardStats> {
    try {
      const [groups, children, vehicles, thisWeekTrips] = await Promise.all([
        this.groupRepository.findByUserId(userId),
        this.childRepository.findByUserId(userId),
        this.vehicleRepository.findByUserId(userId),
        this.scheduleSlotRepository.countUserTripsInWeek(userId, this.getWeekStart(), this.getWeekEnd()),
      ]);

      return {
        groups: groups.length,
        children: children.length,
        vehicles: vehicles.length,
        thisWeekTrips,
        trends: this.calculateTrends(),
      };
    } catch (error) {
      throw handleError(error, 'calculateUserStats');
    }
  }

  async getTodayTripsForUser(userId: string): Promise<TodayTrip[]> {
    try {
      const todaySlots = await this.scheduleSlotRepository.findTodaySlotsForUser(userId);
      
      if (!todaySlots.length) {
        return [];
      }

      return this.transformSlotsToTrips(todaySlots);
    } catch (error) {
      throw handleError(error, 'getTodayTripsForUser');
    }
  }

  async getWeeklyDashboard(userId: string, startDate?: Date): Promise<DayTransportSummary[]> {
    try {
      const userWithFamily = await this.userRepository.findWithFamily(userId);
      
      if (!userWithFamily || !userWithFamily.familyMemberships?.length) {
        logger.warn('User has no family membership', { userId });
        return this.generateEmptyWeekDays(startDate || new Date());
      }

      const familyId = userWithFamily.familyMemberships[0].familyId;
      const groupIds = await this.getGroupIdsForFamily(familyId);

      if (!groupIds.length) {
        return this.generateEmptyWeekDays(startDate || new Date());
      }

      const scheduleSlots = await this.scheduleSlotRepository.findWeeklyScheduleForGroups(
        groupIds,
        this.getDateRange(startDate)
      );

      return this.aggregateSlotsByDay(scheduleSlots, startDate || new Date(), familyId);
    } catch (error) {
      throw handleError(error, 'getWeeklyDashboard');
    }
  }
}

5. Refactor Route Handlers

// /backend/src/routes/dashboard.ts (Refactored)
import { asyncHandler } from '../utils/errorHandler';
import { BaseError } from '../errors/BaseError';

export const getUserStats = asyncHandler(async (req: Request, res: Response): Promise<void> => {
  const userId = (req as any).user.id;
  
  if (!userId) {
    throw new UnauthorizedError('User not authenticated');
  }

  const stats = await dashboardService.calculateUserStats(userId);
  
  res.json({
    success: true,
    data: stats,
    timestamp: new Date().toISOString(),
  });
});

export const getTodayTrips = asyncHandler(async (req: Request, res: Response): Promise<void> => {
  const userId = (req as any).user.id;
  
  if (!userId) {
    throw new UnauthorizedError('User not authenticated');
  }

  const trips = await dashboardService.getTodayTripsForUser(userId);
  
  res.json({
    success: true,
    data: trips,
    count: trips.length,
    timestamp: new Date().toISOString(),
  });
});

export const getWeeklyDashboard = asyncHandler(async (req: Request, res: Response): Promise<void> => {
  const userId = (req as any).user.id;
  const { startDate } = req.query;
  
  if (!userId) {
    throw new UnauthorizedError('User not authenticated');
  }

  const parsedStartDate = startDate ? new Date(startDate as string) : undefined;
  
  if (startDate && isNaN(parsedStartDate!.getTime())) {
    throw new ValidationError('Invalid startDate format. Use ISO 8601 format.', 'startDate');
  }

  const weeklyData = await dashboardService.getWeeklyDashboard(userId, parsedStartDate);
  
  res.json({
    success: true,
    data: weeklyData,
    timestamp: new Date().toISOString(),
  });
});

6. Repository Layer Error Handling

// /backend/src/repositories/ScheduleSlotRepository.ts (Refactored)
export class ScheduleSlotRepository implements IScheduleSlotRepository {
  // ... constructor

  async create(data: CreateScheduleSlotData): Promise<ScheduleSlot> {
    try {
      const prismaSlot = await this.prisma.scheduleSlot.create({
        data: ScheduleSlotFactory.toPrisma(data),
        include: this.getIncludesForScheduleSlot(),
      });

      return ScheduleSlotFactory.fromPrisma(prismaSlot);
    } catch (error) {
      // Handle Prisma-specific errors
      if (error instanceof Error) {
        if (error.message.includes('Unique constraint')) {
          throw new ConflictError('Schedule slot already exists for this group and time', {
            groupId: data.groupId,
            datetime: data.datetime,
          });
        }

        if (error.message.includes('Foreign key constraint')) {
          throw new ValidationError('Invalid group ID or referenced entity', {
            groupId: data.groupId,
          });
        }
      }

      throw error; // Let the service layer handle other errors
    }
  }

  async findById(id: string): Promise<ScheduleSlot | null> {
    try {
      const prismaSlot = await this.prisma.scheduleSlot.findUnique({
        where: { id },
        include: this.getIncludesForScheduleSlot(),
      });

      return prismaSlot ? ScheduleSlotFactory.fromPrisma(prismaSlot) : null;
    } catch (error) {
      if (error instanceof Error && error.message.includes('Invalid')) {
        throw new ValidationError('Invalid schedule slot ID format', { id });
      }
      throw error;
    }
  }
}

Implementation Strategy

Phase 1: Error Infrastructure (Week 1)

  1. Create unified error type hierarchy
  2. Implement error handling utilities
  3. Create global error middleware
  4. Add comprehensive error logging

Phase 2: Repository Layer (Week 1-2)

  1. Update repository error handling
  2. Add specific error type handling
  3. Remove silent failures and fallbacks
  4. Add repository error tests

Phase 3: Service Layer (Week 2-3)

  1. Update service error handling
  2. Replace all AppError wrapping with unified approach
  3. Add proper error context
  4. Update service tests

Phase 4: Route Layer (Week 3-4)

  1. Update all route handlers with asyncHandler
  2. Replace inconsistent error response formats
  3. Add request context to errors
  4. Update integration tests

Benefits of Unified Error Handling

Development Experience

  • Consistency: Predictable error handling across all layers
  • Debugging: Rich error context and logging
  • Maintainability: Single source of truth for error handling
  • Testing: Easier to test error scenarios

User Experience

  • Consistent API: Uniform error response format
  • Better Messages: Clear, actionable error messages
  • Error Context: Rich context for debugging
  • Status Codes: Proper HTTP status codes

System Reliability

  • Error Tracking: Centralized error logging and monitoring
  • Error Recovery: Better error classification and handling
  • Performance: Reduced error handling overhead
  • Monitoring: Enhanced error metrics and alerting

Success Metrics

  • Error Consistency: 100% of routes use unified error handling
  • Error Coverage: 95% of error scenarios properly handled
  • Debugging Time: Reduce error investigation time by 70%
  • User Satisfaction: Improve error-related user complaints by 80%
  • Monitoring: Achieve comprehensive error tracking and alerting

Files to be Created/Modified

New Error Files

  • /backend/src/errors/BaseError.ts
  • /backend/src/errors/AppError.ts
  • /backend/src/errors/ValidationError.ts
  • /backend/src/errors/NotFoundError.ts
  • /backend/src/errors/UnauthorizedError.ts
  • /backend/src/errors/ForbiddenError.ts
  • /backend/src/errors/ConflictError.ts

New Utility Files

  • /backend/src/utils/errorHandler.ts
  • /backend/src/middleware/errorHandler.ts

Modified Files

  • All files in /backend/src/routes/
  • All files in /backend/src/services/
  • All files in /backend/src/repositories/
  • /backend/src/app.ts (to register error middleware)

This unified error handling implementation is essential for creating a reliable, maintainable, and user-friendly application. The current inconsistencies make debugging difficult and provide poor user experience when errors occur.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions