Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored Error Handleing. Modified Country API using new error hand… #173

Draft
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

Disura-Randunu
Copy link
Contributor

…ling

Purpose

The purpose of this PR is to fix #

Goals

Approach

Screenshots

Checklist

  • This PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.
  • I have read and understood the development best practices guidelines ( http://bit.ly/sef-best-practices )
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

Related PRs

Test environment

Learning

@Disura-Randunu
Copy link
Contributor Author

Disura-Randunu commented Oct 7, 2024

@anjula-sack what do you think on this approach. Throw errors based on a cusom error object within the service. will be handled via a middleware function. Error handling will be done in a centralized object so that we can extend it. Custom Error will also be extensible.

to reduce the try/catch, an async handle will be used in the route level.

Used this approach in another project before. We can use the services without repetition. Also, in a case where we need to check if a record is null, then we can use a try/catch (only when necassary) within the service which we use to integrate a funtion from another service.

We can also send a standardized response with a util method to create a response object

Service:

export const getCountryById = async (id: string): Promise<Country> => {
  const countryRepository = dataSource.getRepository(Country)
  const country = await countryRepository.findOneBy({
    uuid: id
  })
  if (country) {
    return country
  }
  throw new AppError('Country Not Found', 404)
}

Controller:

export const getCountryWithId = async (
  req: Request,
  res: Response
): Promise<ApiResponse<Country[]>> => {
  const data = await getCountryById(req.params.id)
  return res.status(200).json({ data, message: 'Country Details' })
}

Route:

countryRouter.get('/:id', asyncHandler(getCountryWithId))

App:

app.use('/api/emails', emailRouter)
app.use('/api/countries', countryRouter)
app.use(async (err: Error, req: Request, res: Response, next: NextFunction) => {
  await errorHandler.handleError(err, res)
})

AppError:

export class AppError extends Error {
  public readonly name: string
  public readonly httpCode: number
  public readonly isOperational: boolean

  constructor(
    message = 'Internal Server Error',
    httpCode = 500,
    isOperational = true
  ) {
    super(message)
    Object.setPrototypeOf(this, new.target.prototype)
    this.name = this.constructor.name
    this.httpCode = httpCode
    this.isOperational = isOperational
    Error.captureStackTrace(this)
  }
}

ErrorHandler:

import { type Response } from 'express'
import { AppError } from './AppError'

class ErrorHandler {
  public async handleError(error: Error, response: Response): Promise<void> {
    console.error(error)
    await this.sendErrorResponse(error, response)
  }

  private async sendErrorResponse(
    error: Error,
    response: Response
  ): Promise<void> {
    if (error instanceof AppError) {
      response.status(error.httpCode).json({
        status: false,
        message: error.message,
        data: null
      })
    } else {
      response.status(500).json({
        status: false,
        message: 'Something went wrong. Please contact administrator.',
        data: null
      })
    }
  }
}
export const errorHandler = new ErrorHandler()

AsyncHandler:

export const asyncHandler =
  (
    fn: (
      req: Request,
      res: Response,
      next: NextFunction
    ) => Promise<ApiResponse<any>>
  ) =>
  (req: Request, res: Response, next: NextFunction) => {
    Promise.resolve(fn(req, res, next)).catch(next)
  }

@anjula-sack
Copy link
Member

I really like this approach. We can keep creating few error types like, ResourceNotFound to handle the other cases. @Disura-Randunu

@Disura-Randunu
Copy link
Contributor Author

I really like this approach. We can keep creating few error types like, ResourceNotFound to handle the other cases. @Disura-Randunu

@anjula-sack we can actually create some sort of enum based error type object and use that to inject different details of the error prperties while having a single Error class. that way we can extend it however we want and keep adding new errorTypes to the enum. Will update on that so we can make sure if its good

@Disura-Randunu
Copy link
Contributor Author

@anjula-sack
Updated AppError with ErrorTypes and a factory method so that we can use it to throw standardized errors.

export function createAppError(
  errorType: AppErrorTypes,
  details?: Record<string, any>
): AppError {
  const { message, httpCode } = AppErrorDetails[errorType]
  return new AppError(errorType, message, httpCode, true, details)
}

export enum AppErrorTypes {
  NOT_FOUND_ERROR = 'NOT_FOUND_ERROR',
  MULTIPLE_RECORDS_FOUND_ERROR = 'MULTIPLE_RECORDS_FOUND_ERROR',
  VALIDATION_ERROR = 'VALIDATION_ERROR',
  AUTHENTICATION_ERROR = 'AUTHENTICATION_ERROR',
  PERMISSION_DENIED_ERROR = 'PERMISSION_DENIED_ERROR',
  INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR'
}

const AppErrorDetails: Record<
  AppErrorTypes,
  { message: string; httpCode: number }
> = {
  [AppErrorTypes.VALIDATION_ERROR]: {
    message: 'Invalid input data',
    httpCode: 400
  },
  [AppErrorTypes.AUTHENTICATION_ERROR]: {
    message: 'Authentication failed',
    httpCode: 401
  },
  [AppErrorTypes.NOT_FOUND_ERROR]: {
    message: 'Resource not found',
    httpCode: 404
  },
  [AppErrorTypes.INTERNAL_SERVER_ERROR]: {
    message: 'An internal server error occurred',
    httpCode: 500
  },
  [AppErrorTypes.PERMISSION_DENIED_ERROR]: {
    message: 'Insufficient permission for this action',
    httpCode: 403
  },
  [AppErrorTypes.MULTIPLE_RECORDS_FOUND_ERROR]: {
    message: 'Multple records has been found',
    httpCode: 400
  }
}

We can throw the error within service like below

  throw createAppError(AppErrorTypes.NOT_FOUND_ERROR)

OR

We can throw a custom error like below:

throw new AppError("CUSTOM_ERROR", "custom message", 400)

We can also make it so that ERROR NAME must be of AppErrorType enum to strict the error types. Then we just have to add that new error for the enum to make use of that insteaad of just any string. ***This is when creating a custom error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants