🔧 MEDIUM: Error Handling Inconsistencies - Poor User Experience and Debugging Difficulties

# 🔧 MEDIUM: Error Handling Inconsistencies - Poor User Experience and Debugging Difficulties

## Problem Description

The NSX application has inconsistent error handling patterns across different routes and endpoints, leading to poor user experience, security issues, and debugging difficulties.

**Current problematic patterns:**
```typescript
// server/routes/post.ts:10-28 - Inconsistent error handling
router.get('/post/:id', async (req: Request, res: Response) => {
  try {
    const post = await prisma.post.findFirst({
      where: { id: parseInt(req.params.id, 10) },
    })
    res.status(200).json(post)  // ❌ No null check
  } catch (error: unknown) {
    if (error instanceof Error) {
      Logger.error(error)
      res.status(500).json({ error: error.message })  // ❌ Exposes internal errors
    } else {
      Logger.error(error)
      res.status(500).json({
        error: `something wrong: ${JSON.stringify(error)}`,  // ❌ Poor error message
      })
    }
  }
})

// server/routes/user.ts:91-97 - Different error pattern
} else {
  Logger.warn('Invalid Password')
  res.status(200).json({ failed: 'Invalid Password' })  // ❌ Wrong status code
} else {
  Logger.warn('User does not exist')
  res.status(200).json({ failed: 'User does not exist' })  // ❌ Wrong status code
}
```

## Why This Is Dangerous

1. **Information Disclosure**: Internal error details exposed to clients
2. **Poor User Experience**: Inconsistent error messages and status codes
3. **Security Issues**: Sensitive information leaked through error messages
4. **Debugging Difficulties**: Inconsistent error logging and handling
5. **API Inconsistency**: Different error formats across endpoints

## Current Impact

- **Severity**: Medium
- **User Experience**: Poor - confusing error messages
- **Security**: Medium - potential information disclosure
- **Maintainability**: Low - difficult to debug and maintain

## Comprehensive Solution

### 1. Create Centralized Error Classes

```typescript
// server/lib/errors/AppError.ts
export class AppError extends Error {
  public readonly statusCode: number
  public readonly isOperational: boolean
  public readonly code: string
  public readonly details?: any

  constructor(
    message: string,
    statusCode: number = 500,
    code: string = 'INTERNAL_ERROR',
    isOperational: boolean = true,
    details?: any
  ) {
    super(message)
    
    this.statusCode = statusCode
    this.isOperational = isOperational
    this.code = code
    this.details = details
    
    Error.captureStackTrace(this, this.constructor)
  }
}

// Specific error types
export class ValidationError extends AppError {
  constructor(message: string, details?: any) {
    super(message, 400, 'VALIDATION_ERROR', true, details)
  }
}

export class AuthenticationError extends AppError {
  constructor(message: string = 'Authentication failed') {
    super(message, 401, 'AUTHENTICATION_ERROR')
  }
}

export class AuthorizationError extends AppError {
  constructor(message: string = 'Access denied') {
    super(message, 403, 'AUTHORIZATION_ERROR')
  }
}

export class NotFoundError extends AppError {
  constructor(resource: string = 'Resource') {
    super(`${resource} not found`, 404, 'NOT_FOUND')
  }
}

export class ConflictError extends AppError {
  constructor(message: string) {
    super(message, 409, 'CONFLICT')
  }
}

export class RateLimitError extends AppError {
  constructor(message: string = 'Too many requests') {
    super(message, 429, 'RATE_LIMIT_EXCEEDED')
  }
}

export class DatabaseError extends AppError {
  constructor(message: string = 'Database operation failed') {
    super(message, 500, 'DATABASE_ERROR')
  }
}
```

### 2. Create Error Handler Middleware

```typescript
// server/lib/errors/errorHandler.ts
import { Request, Response, NextFunction } from 'express'
import { AppError } from './AppError'
import Logger from '../Logger'

interface ErrorResponse {
  success: false
  error: string
  code: string
  details?: any
  timestamp: string
  requestId?: string
}

export const errorHandler = (
  error: Error,
  req: Request,
  res: Response,
  next: NextFunction
) => {
  let appError: AppError

  // Convert known errors to AppError
  if (error instanceof AppError) {
    appError = error
  } else if (error.name === 'ValidationError') {
    appError = new ValidationError(error.message)
  } else if (error.name === 'CastError') {
    appError = new ValidationError('Invalid ID format')
  } else if (error.name === 'MongoError' || error.name === 'PrismaClientKnownRequestError') {
    appError = new DatabaseError('Database operation failed')
  } else {
    // Unknown error - don't expose details
    appError = new AppError(
      'An unexpected error occurred',
      500,
      'INTERNAL_ERROR',
      false
    )
  }

  // Log error appropriately
  if (appError.statusCode >= 500) {
    Logger.error('Server Error:', {
      error: appError.message,
      stack: appError.stack,
      code: appError.code,
      url: req.url,
      method: req.method,
      ip: req.ip,
      userAgent: req.get('user-agent'),
      body: req.body,
      params: req.params,
      query: req.query
    })
  } else {
    Logger.warn('Client Error:', {
      error: appError.message,
      code: appError.code,
      url: req.url,
      method: req.method,
      ip: req.ip,
      details: appError.details
    })
  }

  // Prepare error response
  const errorResponse: ErrorResponse = {
    success: false,
    error: appError.message,
    code: appError.code,
    timestamp: new Date().toISOString(),
    requestId: req.headers['x-request-id'] as string
  }

  // Add details only for validation errors in development
  if (appError.details && process.env.NODE_ENV === 'development') {
    errorResponse.details = appError.details
  }

  // Send error response
  res.status(appError.statusCode).json(errorResponse)
}

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

### 3. Create Success Response Helper

```typescript
// server/lib/responses/successResponse.ts
import { Response } from 'express'

interface SuccessResponse<T = any> {
  success: true
  data?: T
  message?: string
  meta?: {
    total?: number
    page?: number
    limit?: number
    hasNext?: boolean
  }
  timestamp: string
  requestId?: string
}

export const sendSuccess = <T>(
  res: Response,
  data?: T,
  message?: string,
  statusCode: number = 200,
  meta?: SuccessResponse<T>['meta']
) => {
  const response: SuccessResponse<T> = {
    success: true,
    data,
    message,
    meta,
    timestamp: new Date().toISOString(),
    requestId: res.get('x-request-id')
  }

  res.status(statusCode).json(response)
}

// Convenience methods
export const sendCreated = <T>(res: Response, data: T, message?: string) => {
  sendSuccess(res, data, message || 'Resource created successfully', 201)
}

export const sendUpdated = (res: Response, message?: string) => {
  sendSuccess(res, undefined, message || 'Resource updated successfully', 200)
}

export const sendDeleted = (res: Response, message?: string) => {
  sendSuccess(res, undefined, message || 'Resource deleted successfully', 200)
}
```

### 4. Update Route Implementations

```typescript
// server/routes/post.ts
import { asyncHandler } from '../lib/errors/errorHandler'
import { sendSuccess, sendCreated, sendUpdated, sendDeleted } from '../lib/responses/successResponse'
import { NotFoundError, DatabaseError } from '../lib/errors/AppError'
import { validateBody, validateParams } from '../lib/validation/middleware'
import { postSchemas } from '../lib/validation/schemas'

// Get single post
router.get('/post/:id',
  validateParams(z.object({ id: z.string().regex(/^\d+$/) })),
  asyncHandler(async (req: Request, res: Response) => {
    const postId = parseInt(req.params.id, 10)
    
    const post = await prisma.post.findFirst({
      where: { id: postId },
    })
    
    if (!post) {
      throw new NotFoundError('Post')
    }
    
    sendSuccess(res, post)
  })
)

// Create post
router.post('/create',
  isAuthorized,
  validateBody(postSchemas.create),
  asyncHandler(async (req: Request, res: Response) => {
    const { title, body } = req.body
    
    try {
      const post = await prisma.post.create({
        data: {
          title: title.trim(),
          body: body.trim(),
        },
      })
      
      sendCreated(res, post, 'Post created successfully')
    } catch (error) {
      throw new DatabaseError('Failed to create post')
    }
  })
)

// Update post
router.post('/update',
  isAuthorized,
  validateBody(postSchemas.update),
  asyncHandler(async (req: Request, res: Response) => {
    const { id, title, body } = req.body
    
    // Check if post exists
    const existingPost = await prisma.post.findFirst({
      where: { id }
    })
    
    if (!existingPost) {
      throw new NotFoundError('Post')
    }
    
    try {
      await prisma.post.update({
        where: { id },
        data: {
          title: title.trim(),
          body: body.trim(),
        },
      })
      
      sendUpdated(res, 'Post updated successfully')
    } catch (error) {
      throw new DatabaseError('Failed to update post')
    }
  })
)

// Delete post
router.delete('/post/:id',
  isAuthorized,
  validateParams(z.object({ id: z.string().regex(/^\d+$/) })),
  asyncHandler(async (req: Request, res: Response) => {
    const postId = parseInt(req.params.id, 10)
    
    // Check if post exists
    const existingPost = await prisma.post.findFirst({
      where: { id: postId }
    })
    
    if (!existingPost) {
      throw new NotFoundError('Post')
    }
    
    try {
      await prisma.post.delete({
        where: { id: postId }
      })
      
      sendDeleted(res, 'Post deleted successfully')
    } catch (error) {
      throw new DatabaseError('Failed to delete post')
    }
  })
)

// Get post list with pagination
router.get('/post_list',
  validateQuery(z.object({
    page: z.string().regex(/^\d+$/).transform(Number).default('1'),
    perPage: z.string().regex(/^\d+$/).transform(Number).default('10')
  })),
  asyncHandler(async (req: Request, res: Response) => {
    const { page, perPage } = req.query as { page: number; perPage: number }
    
    try {
      const total = await prisma.post.count()
      const offset = perPage * (page - 1)
      
      const posts = await prisma.post.findMany({
        take: perPage,
        skip: offset,
        orderBy: { id: 'desc' },
      })
      
      const meta = {
        total,
        page,
        limit: perPage,
        hasNext: offset + perPage < total
      }
      
      sendSuccess(res, posts, 'Posts retrieved successfully', 200, meta)
    } catch (error) {
      throw new DatabaseError('Failed to retrieve posts')
    }
  })
)
```

### 5. Update User Routes

```typescript
// server/routes/user.ts
import { asyncHandler } from '../lib/errors/errorHandler'
import { sendSuccess, sendCreated } from '../lib/responses/successResponse'
import { AuthenticationError, ConflictError, ValidationError } from '../lib/errors/AppError'

// Login
router.post('/login',
  loginRateLimiter,
  validateBody(userSchemas.login),
  asyncHandler(async (req: Request, res: Response) => {
    const { name, password } = req.body
    const normalizedName = name.toLowerCase().trim()
    
    // Check account lockout
    const lockoutStatus = checkAccountLockout(normalizedName)
    if (lockoutStatus.isLocked) {
      throw new AuthenticationError('Account temporarily locked due to multiple failed attempts')
    }
    
    const [user, dummyHash] = await Promise.all([
      prisma.user.findFirst({
        where: { name: normalizedName },
        select: { id: true, name: true, password: true, createdAt: true, updatedAt: true }
      }),
      bcrypt.hash('dummy', 10)
    ])
    
    const passwordToCompare = user?.password || dummyHash
    const isValidPassword = await bcrypt.compare(password, passwordToCompare)
    
    await consistentDelay()
    
    if (user && isValidPassword) {
      clearFailedAttempts(normalizedName)
      
      const token: JWTtoken = generateAccessToken(user)
      res.cookie('token', token, getCookieOptions(token))
      
      sendSuccess(res, {
        user: {
          id: user.id,
          name: user.name,
          createdAt: user.createdAt,
          updatedAt: user.updatedAt
        }
      }, 'Login successful')
    } else {
      const lockoutStatus = recordFailedAttempt(normalizedName)
      
      if (lockoutStatus.isLocked) {
        throw new AuthenticationError('Account temporarily locked due to multiple failed attempts')
      }
      
      throw new AuthenticationError('Invalid credentials')
    }
  })
)

// Signup
router.post('/signup',
  validateBody(userSchemas.signup),
  asyncHandler(async (req: Request, res: Response) => {
    const { name, password } = req.body
    const normalizedName = name.toLowerCase().trim()
    
    // Check if user already exists
    const existingUser = await prisma.user.findFirst({
      where: { name: normalizedName }
    })
    
    if (existingUser) {
      throw new ConflictError('Username already exists')
    }
    
    try {
      const salt = await bcrypt.genSalt(12)
      const hash = await bcrypt.hash(password, salt)
      
      const user = await prisma.user.create({
        data: {
          name: normalizedName,
          password: hash,
        },
      })
      
      const token: JWTtoken = generateAccessToken(user)
      res.cookie('token', token, getCookieOptions(token))
      
      sendCreated(res, {
        user: {
          id: user.id,
          name: user.name,
          createdAt: user.createdAt,
          updatedAt: user.updatedAt
        }
      }, 'Account created successfully')
    } catch (error) {
      throw new DatabaseError('Failed to create account')
    }
  })
)
```

### 6. Global Error Handling Setup

```typescript
// server/index.ts
import { errorHandler } from './lib/errors/errorHandler'
import { requestIdMiddleware } from './lib/middleware/requestId'

// Add request ID middleware
app.use(requestIdMiddleware)

// ... other middleware

// Apply error handler at the end
app.use(errorHandler)

// Handle 404 errors
app.use('*', (req: Request, res: Response) => {
  res.status(404).json({
    success: false,
    error: 'Route not found',
    code: 'NOT_FOUND',
    timestamp: new Date().toISOString(),
    requestId: req.headers['x-request-id']
  })
})
```

### 7. Request ID Middleware

```typescript
// server/lib/middleware/requestId.ts
import { Request, Response, NextFunction } from 'express'
import { randomUUID } from 'crypto'

export const requestIdMiddleware = (req: Request, res: Response, next: NextFunction) => {
  const requestId = req.headers['x-request-id'] as string || randomUUID()
  
  req.headers['x-request-id'] = requestId
  res.set('x-request-id', requestId)
  
  next()
}
```

## Testing the Implementation

### 1. Error Handling Tests

```typescript
// tests/error-handling.test.ts
import request from 'supertest'
import app from '../server/index'

describe('Error Handling', () => {
  it('should return consistent error format', async () => {
    const response = await request(app)
      .get('/api/post/999999')
      .expect(404)
    
    expect(response.body).toMatchObject({
      success: false,
      error: 'Post not found',
      code: 'NOT_FOUND',
      timestamp: expect.any(String)
    })
  })
  
  it('should not expose internal error details', async () => {
    const response = await request(app)
      .post('/api/create')
      .send({ title: '', body: '' })
      .expect(400)
    
    expect(response.body.success).toBe(false)
    expect(response.body.code).toBe('VALIDATION_ERROR')
    expect(response.body.error).toBeDefined()
    expect(response.body.stack).toBeUndefined()
  })
  
  it('should include request ID in error responses', async () => {
    const response = await request(app)
      .get('/api/nonexistent')
      .expect(404)
    
    expect(response.body.requestId).toBeDefined()
    expect(response.headers['x-request-id']).toBeDefined()
  })
})
```

### 2. Success Response Tests

```typescript
// tests/success-responses.test.ts
describe('Success Responses', () => {
  it('should return consistent success format', async () => {
    const response = await request(app)
      .get('/api/user_count')
      .expect(200)
    
    expect(response.body).toMatchObject({
      success: true,
      data: expect.any(Object),
      timestamp: expect.any(String)
    })
  })
  
  it('should include pagination meta for list endpoints', async () => {
    const response = await request(app)
      .get('/api/post_list?page=1&perPage=5')
      .expect(200)
    
    expect(response.body.meta).toMatchObject({
      total: expect.any(Number),
      page: 1,
      limit: 5,
      hasNext: expect.any(Boolean)
    })
  })
})
```

## Migration Strategy

### Phase 1: Error Classes and Middleware
1. Create error classes and handlers
2. Add global error handling middleware
3. Update critical routes

### Phase 2: Response Helpers
1. Create success response helpers
2. Update all route responses
3. Add request ID tracking

### Phase 3: Testing and Monitoring
1. Add comprehensive error tests
2. Set up error monitoring
3. Create error dashboards

## Monitoring and Alerting

### 1. Error Rate Monitoring

```typescript
// Track error rates by type
export const trackError = (error: AppError, req: Request) => {
  const errorMetrics = {
    code: error.code,
    statusCode: error.statusCode,
    url: req.url,
    method: req.method,
    timestamp: new Date().toISOString()
  }
  
  // Send to monitoring service
  Logger.info('Error tracked:', errorMetrics)
}
```

### 2. Error Alerting

```yaml
# Example alerting rules
alerts:
  - name: "High Error Rate"
    condition: "error_rate > 5% in 5 minutes"
    action: "notify_team"
  
  - name: "Database Errors"
    condition: "database_errors > 10 in 1 minute"
    action: "page_oncall"
```

## References

- [Express Error Handling](https://expressjs.com/en/guide/error-handling.html)
- [Node.js Error Handling Best Practices](https://nodejs.org/en/docs/guides/error-handling/)
- [REST API Error Handling](https://restfulapi.net/error-handling/)

---

**Priority**: 🟡 Medium  
**Estimated Effort**: 6-10 hours  
**Risk if not fixed**: Medium - Poor user experience and debugging difficulties

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🔧 MEDIUM: Error Handling Inconsistencies - Poor User Experience and Debugging Difficulties #3590

🔧 MEDIUM: Error Handling Inconsistencies - Poor User Experience and Debugging Difficulties

Problem Description

Why This Is Dangerous

Current Impact

Comprehensive Solution

1. Create Centralized Error Classes

2. Create Error Handler Middleware

3. Create Success Response Helper

4. Update Route Implementations

5. Update User Routes

6. Global Error Handling Setup

7. Request ID Middleware

Testing the Implementation

1. Error Handling Tests

2. Success Response Tests

Migration Strategy

Phase 1: Error Classes and Middleware

Phase 2: Response Helpers

Phase 3: Testing and Monitoring

Monitoring and Alerting

1. Error Rate Monitoring

2. Error Alerting

References

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

🔧 MEDIUM: Error Handling Inconsistencies - Poor User Experience and Debugging Difficulties #3590

Description

🔧 MEDIUM: Error Handling Inconsistencies - Poor User Experience and Debugging Difficulties

Problem Description

Why This Is Dangerous

Current Impact

Comprehensive Solution

1. Create Centralized Error Classes

2. Create Error Handler Middleware

3. Create Success Response Helper

4. Update Route Implementations

5. Update User Routes

6. Global Error Handling Setup

7. Request ID Middleware

Testing the Implementation

1. Error Handling Tests

2. Success Response Tests

Migration Strategy

Phase 1: Error Classes and Middleware

Phase 2: Response Helpers

Phase 3: Testing and Monitoring

Monitoring and Alerting

1. Error Rate Monitoring

2. Error Alerting

References

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions