-
Notifications
You must be signed in to change notification settings - Fork 98
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
Comments
can you provide minimal example repo with logs? |
@iamolegga sure, I'll isolate this |
@iamolegga |
I can confirm this happens to me either. Have you came up with a workaround to it? |
No, unfortunately, I have not. |
Does it work properly with the original built-in logger? |
@iamolegga I need to double-check this. I guess yes though. |
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. |
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 |
Same to me, but it only happens when i try to execute test scripts using jest+supertest. |
@happy-ruby
No, I have not. |
Okay thanks, what i fixed is that i switched |
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)); |
Thanks, @noahw3! Your solution should be considered as an official workaround for this issue. |
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 likeLoggerErrorInterceptor
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.
The text was updated successfully, but these errors were encountered: