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

[FEATURE REQUEST] Correct stack trace of a guard error in request errored pino-http's log #1111

Open
artursudnik opened this issue Aug 31, 2022 · 14 comments
Assignees
Labels
enhancement New feature or request

Comments

@artursudnik
Copy link

The problem I am facing is similar to those described in this feature request. Despite after using LoggerErrorInterceptor I am getting the correct stack trace of errors thrown from controllers or services when an error is thrown from a guard, it behaves like LoggerErrorInterceptor was not used.

Describe the solution you'd like
I am not sure if it is a bug or if additional functionality to be added. Probably this problem is caused by Interceptor not being called at all when application flow is broken on a guard level.

@artursudnik artursudnik added the enhancement New feature or request label Aug 31, 2022
@iamolegga
Copy link
Owner

can you provide minimal example repo with logs?

@artursudnik
Copy link
Author

@iamolegga sure, I'll isolate this

@artursudnik
Copy link
Author

@iamolegga
I apologize for the delay.
Here is this problem reproduced: https://github.com/artursudnik/nestjs-pino-issue.

@mattvgm
Copy link

mattvgm commented Oct 20, 2022

I can confirm this happens to me either. Have you came up with a workaround to it?

@artursudnik
Copy link
Author

No, unfortunately, I have not.

@iamolegga
Copy link
Owner

Does it work properly with the original built-in logger?

@artursudnik
Copy link
Author

@iamolegga I need to double-check this. I guess yes though.

@eyalch
Copy link

eyalch commented Dec 18, 2022

Considering NestJS's request lifecycle, should this logic be in an interceptor? Wouldn't an exception filter be a better fit for this?

The same issue is happening for me. All errors except ones from guards are logged just fine.

@iamolegga
Copy link
Owner

Wouldn't an exception filter be a better fit for this?

agree, we need to change this class to a filter and add tests to verify that it correctly catches and logs errors from different layers

@hapetherw
Copy link

hapetherw commented Jan 5, 2023

Same to me, but it only happens when i try to execute test scripts using jest+supertest.
I am not sure how to fix it.
@artursudnik Did you find out the solution?

@artursudnik
Copy link
Author

artursudnik commented Jan 6, 2023

@happy-ruby

@artursudnik Did you find out the solution?

No, I have not.

@hapetherw
Copy link

Okay thanks, what i fixed is that i switched logger.warn to logger.errror in exception cases.
Then i could catch out the issue logs.
That may be an alternative solution.

@noahw3
Copy link

noahw3 commented May 8, 2023

As @eyalch pointed out, the problem is that interceptors run after guards. The solution is to add an exception filter to do this instead.

The following seems to work well; it's heavily based on the sample exception filter. I have not tested it in a fastify environment.

import {
  type ArgumentsHost,
  Catch,
  type ExceptionFilter,
  HttpException,
  InternalServerErrorException,
} from "@nestjs/common";
import type { HttpAdapterHost } from "@nestjs/core";

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

  catch(error: 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 response = host.switchToHttp().getResponse();

    const isFastifyResponse = response.raw !== undefined;

    if (isFastifyResponse) {
      response.raw.err = error;
    } else {
      response.err = error;
    }

    const httpException =
      error instanceof HttpException
        ? error
        : new InternalServerErrorException();

    httpAdapter.reply(
      response,
      httpException.getResponse(),
      httpException.getStatus()
    );
  }
}

Install during setup with:

const httpAdapterHost = app.get(HttpAdapterHost);
app.useGlobalFilters(new LoggerExceptionFilter(httpAdapterHost));

@artursudnik
Copy link
Author

Thanks, @noahw3! Your solution should be considered as an official workaround for this issue.

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

No branches or pull requests

6 participants