Search code examples
error-handlingnestjsdry

NestJS - How to structure code to elegantly handle errors?


I'm struggling to handle errors in my NestJS app because I'm writing try/catch everywhere. I'm also not sure whether it's cleaner/best practice to throw errors in a service or in the resolver/controller, or catch it and rethrow it in some interceptor or exception filter. Also is writing code with fn().then().catch() cleaner since I seem to be rewriting the same errors a lot when using try/catch.

Here is an example of my code with poor error handling with lots of boilerplate code.

How would you handle the errors more cleanly and DRY?

  async validateUser(
    email: string,
    plainTextPassword: string,
  ): Promise<User | null> {
    try {
      const user = await this.usersRepository.findOne({ email });
      if (!user) {
        throw new HttpException(
          "Wrong credentials provided",
          HttpStatus.BAD_REQUEST,
        );
      }

      const isMatch = await this.verifyPassword(
        plainTextPassword,
        user?.password,
      );
      if (isMatch) {
        await this.usersRepository.filter(user);
        return user;
      }
    } catch (err) {
      this.logger.error(err);
      throw new HttpException(
        "Something went wrong",
        HttpStatus.INTERNAL_SERVER_ERROR,
      );
    }
  }

  /** This function is used by the validateUser function
   *  Note: This could be written much much shorter in 1-2 lines...
   */
  async verifyPassword(plainTextPassword: string, hashedPassword: string) {
    try {
      const isMatch = await bcrypt.compare(plainTextPassword, hashedPassword);
      if (!isMatch) {
        throw new HttpException(
          "Wrong credentials provided",
          HttpStatus.BAD_REQUEST,
        );
      }
      return isMatch;
    } catch (err) {
      this.logger.error(err);
      throw new HttpException(
        "Something went wrong",
        HttpStatus.INTERNAL_SERVER_ERROR,
      );
    }
  }

Solution

  • Here are some suggestions:

    • verifyPassword: there isn't much purpose in returning a boolean if the only possible value is true (when false, the function throws). You can move the error-throwing part of the function to validateUser and simply return whether the password matches.
    • In both functions, you are first throwing a specific error, then catching it and replacing it with another. In case someone enters an inexistent email they can never find out this way, because they will receive an internal server error that gives no information as to why was the error caused. With this you can remove both try/catch clauses.
    • In my humble opinion, the 400 thrown when the user's email is not found should be a 404 (not found), and the one when the password doesn't match should be a 403 (forbidden).
    • To further simplify, you can use nest's specific exceptions instead of HttpException, such as NotFoundException, ForbiddenException and InternalServerErrorException. See this.
    • There is no need to throw InternalServerErrorExceptions, since NestJS's default filter will handle any uncaught error and throw the 500 himself.
    • In case you have some specific errors you want to handle differently (mostly library-specific errors that you might want to notify yourself about or log them particularly), you can implement custom filters for these errors alone, and handle them however you want, and restrict them to only some modules or use them in your whole app. This way, you don't need to have any error-handling logic in your services/controllers, except very specific cases (when a function throwing is something you'd expect in a normal flow)