Exception handling

[TangoYankee]

Description

Need to structure exceptions in such a way that the concerns are separated into the appropriate parts- controller, service, or repository.

Context

Link to conversation on exception handling

Discussion comment

Adding for posterity - I stumbled up on this SO thread that I think summarizes thoughts on what sort of exceptions ought to go at which layer of the architecture: https://softwareengineering.stackexchange.com/questions/393307/where-would-you-handle-exceptions-controller-service-repository. This doesn't necessarily describe the approach we're taking now, just documenting the research.

[TangoYankee]

An Experimental branch demonstrates how we may use Exception Filters to map Application exceptions to HTTP exceptions.

Exceptions

The first step is to refactor the Application exceptions to classes that extend the built-in JS Error. This accomplishes two things. First, it decouples the business logic from the http response. Second, the creation of a new class means that it can be identified by Nest Filters.

Data Retrieval Exception example

export class DataRetrievalException extends Error {
  constructor() {
    super("Error while retrieving data");
    this.name = "DataRetrievalException";
  }
}

Filters

The second step is to create the nest filters that will capture the business logic exceptions and map them to the appropriate Http Status codes. For example, the application may error when the users passes a bad path parameters. We may also add functionality to support geometry-based requests. The application may error when the users passes an invalid geometry. Both of these errors would map to a bad request response. Nest provides an HttpStatus enum to map the codes. However, it does not map the names of the status. Consequently, I also created an 'HttpName' to map the actual name of the error.

Bad request Exception

import {
  ArgumentsHost,
  Catch,
  ExceptionFilter,
  HttpStatus,
} from "@nestjs/common";
import { HttpAdapterHost } from "@nestjs/core";
import {
  InvalidGeometryException,
  InvalidRequestParameterException,
} from "src/exception";
import { HttpName } from ".";

@Catch(InvalidGeometryException, InvalidRequestParameterException)
export class BadRequestExceptionFilter implements ExceptionFilter {
  constructor(private readonly httpAdapterHost: HttpAdapterHost) {}

  catch(exception: Error, host: ArgumentsHost) {
    const { httpAdapter } = this.httpAdapterHost;

    const httpStatus = HttpStatus.BAD_REQUEST;
    const responseBody = {
      statusCode: httpStatus,
      message: exception.message,
      error: HttpName.BAD_REQUEST,
    };

    const ctx = host.switchToHttp();

    httpAdapter.reply(ctx.getResponse(), responseBody, httpStatus);
  }
}

Finally, the appropriate filter is applied to the routes. This may be done at the controller or path level. In the example, I apply it at the controller level. There is nothing route-specific about the filters that requires each route getting its own instance of the filters. Putting it at the controller level reduces repetitive code. We can also be selective about applying filters to each controller, only adding a filter when one of its paths may throw an error that would be caught by that filter.

Tax lot controller may throw errors for invalid path parameters, database read errors, or missing tax lots. Consequently, it gets a Bad Request, Internal Server, and Not Found filters.

@Injectable()
@UseFilters(
  BadRequestExceptionFilter,
  InternalServerErrorExceptionFilter,
  NotFoundExceptionFilter,
)
@Controller("tax-lots")
export class TaxLotController {
  constructor(
    private readonly taxLotService: TaxLotService,
    @Inject(StorageConfig.KEY)
    private storageConfig: ConfigType<typeof StorageConfig>,
  ) {}

src/land-use/land-use.controller.ts may only throw errors for database read errors. Consequently, it only adds the Internal Server Filter

import { Controller, Get, UseFilters } from "@nestjs/common";
import { LandUseService } from "./land-use.service";
import { InternalServerErrorExceptionFilter } from "src/filter";

@UseFilters(InternalServerErrorExceptionFilter)
@Controller("land-uses")
export class LandUseController {
  constructor(private readonly landUseService: LandUseService) {}

  @Get()
  async findAll() {
    return this.landUseService.findAll();
  }
}

[TylerMatteo]

This sounds pretty good to me. I like the idea of applying filters at the controller level.