docs(exception-filters): pass full HttpAdapterHost

[pepijnolivier]

PR Checklist

Please check if your PR fulfills the following requirements:

• [x] The commit message follows our guidelines: https://github.com/nestjs/docs.nestjs.com/blob/master/CONTRIBUTING.md

PR Type

What kind of change does this PR introduce?

Fix documentation issue: pass in the entire HttpAdapterHost as the example code requires

• [ ] Bugfix
• [ ] Feature
• [ ] Code style update (formatting, local variables)
• [ ] Refactoring (no functional changes, no api changes)
• [ ] Build related changes
• [x] Docs
• [ ] Other... Please describe:

What is the current behavior?

Typescript error when using the example code

Argument of type 'AbstractHttpAdapter<any, any, any>' is not assignable to parameter of type 'HttpAdapterHost<AbstractHttpAdapter<any, any, any>>'.
  Property 'httpAdapter' is missing in type 'AbstractHttpAdapter<any, any, any>' but required in type 'HttpAdapterHost<AbstractHttpAdapter<any, any, any>>'.

What is the new behavior?

Fix typescript error

Does this PR introduce a breaking change?

• [ ] Yes
• [x] No

Other information

None

[jmcdo29]

If you'll notice, this is actually a different filter than the one mentioned above, and thus has a different signature and parameters. Just above is a full implementation while this one is an extension of the BaseExceptionFilter

[Tony133]

As specified by Jay Actually the documentation is correct, let's try to specify it again but with code snippets:

This snippet code

import { Catch, ArgumentsHost } from '@nestjs/common';
import { BaseExceptionFilter } from '@nestjs/core';

@Catch()
export class AllExceptionsFilter extends BaseExceptionFilter {
  catch(exception: unknown, host: ArgumentsHost) {
    super.catch(exception, host);
  }
}

and linked to this:

async function bootstrap() {
  const app = await NestFactory.create(AppModule);

  const { httpAdapter } = app.get(HttpAdapterHost);
  app.useGlobalFilters(new AllExceptionsFilter(httpAdapter));

  await app.listen(3000);
}
bootstrap();

And not to this code snippet:

import {
  ExceptionFilter,
  Catch,
  ArgumentsHost,
  HttpException,
  HttpStatus,
} from '@nestjs/common';
import { HttpAdapterHost } from '@nestjs/core';

@Catch()
export class AllExceptionsFilter implements ExceptionFilter {
  constructor(private readonly httpAdapterHost: HttpAdapterHost) {}

  catch(exception: unknown, host: ArgumentsHost): void {
    // In certain situations `httpAdapter` might not be available in the
    // constructor method, thus we should resolve it here.
    const { httpAdapter } = this.httpAdapterHost;

    const ctx = host.switchToHttp();

    const httpStatus =
      exception instanceof HttpException
        ? exception.getStatus()
        : HttpStatus.INTERNAL_SERVER_ERROR;

    const responseBody = {
      statusCode: httpStatus,
      timestamp: new Date().toISOString(),
      path: httpAdapter.getRequestUrl(ctx.getRequest()),
    };

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

However, I think it's already the 6th PR with request for this change (list of closed PRs):

• https://github.com/nestjs/docs.nestjs.com/pull/2506
• https://github.com/nestjs/docs.nestjs.com/pull/2260
• https://github.com/nestjs/docs.nestjs.com/pull/2201
• https://github.com/nestjs/docs.nestjs.com/pull/2460
• https://github.com/nestjs/docs.nestjs.com/pull/2437

So it still confuses people a bit about this point in the documentation, maybe because it's skipped while reading? or why the classes have the same name? 🤔 Maybe an alert could fix this or change the name of one of the two classes in the snippets? 🤔

[micalevisk]

maybe because it's skipped while reading?

I guess isn't that clear that we should read it sequentially

... change the name of one of the two classes in the snippets?

this sounds good to me

---

@pepijnolivier could you please tell us why did you thought that was wrong?

[kamilmysliwiec]

... change the name of one of the two classes in the snippets?

This sounds good to me too

[pepijnolivier]

@micalevisk @kamilmysliwiec

Thanks for the fast responses! You guys are correct of course.

maybe because it's skipped while reading?

This is definitely it. 🙂 All I wanted to do was to quickly create a "catch-all exception handler" in my project, so I copy-pasted the code and scrolled down and copy-pasted the snippet that adds the handler to the bootstrap function as well.

Now that I see what happened, here is my suggestion on how the docs can be improved:

• Either use a different className for the 2 examples (full implementation vs extension of the BaseExceptionFilter)
• Or, the first example with the full implementation could just receive httpAdapter rather than the { httpAdapter } in its constructor. This would make both Exception implementations compatible with each other (and swappable).

☝️I think the second option would be best, this way other readers like me that quickly skimmed over the documentation to "quickly copy the global exception handler" should be able to do that without any issue.

[kamilmysliwiec]

Either use a different className for the 2 examples (full implementation vs extension of the BaseExceptionFilter)

Let's proceed with this option. We can't really do the other one since, in some specific situations, httpAdapter might not be available in the constructor