iamolegga / nestjs-pino

Platform agnostic logger for NestJS based on Pino with REQUEST CONTEXT IN EVERY LOG
MIT License
1.26k stars 98 forks source link

[BUG] No logs when using a custom express server #534

Closed fuglu closed 2 years ago

fuglu commented 3 years ago

[x] I've read the docs of nestjs-pino

[x] I've read the docs of pino

[x] I couldn't find the same open issue of nestjs-pino

What is the current behavior?

Only request logs will be written when using a custom express server. Other this.logger.log("...") messages won't show up.

What is the expected behavior?

All lines should be logged.

Please provide minimal example repo. Without it this issue will be closed

Simple npx -p @nestjs/cli nest new my-nest-project example is over here: https://github.com/fuglu/nestjs-pino-custom-express-server

# clone and start the service
cd /tmp
git clone https://github.com/fuglu/nestjs-pino-custom-express-server.git
cd nestjs-pino-custom-express-server
npm i
npm start

# and in a second terminal
watch curl localhost:3000

The app.controller.ts has a this.logger.log('Hello Nest'); line which won't show up.

This is the relevant main.ts:

import { NestFactory } from '@nestjs/core';
import {
  ExpressAdapter,
  NestExpressApplication,
} from '@nestjs/platform-express';
import * as express from 'express';
import { Logger } from 'nestjs-pino';
import { AppModule } from './app.module';

// This one works
async function bootstrap() {
  const app = await NestFactory.create(AppModule, { bufferLogs: true });
  app.useLogger(app.get(Logger));
  await app.listen(3000);
}

// This one has only request logs
async function bootstrapCreateServer() {
  const server = express();

  const app = await NestFactory.create<NestExpressApplication>(
    AppModule,
    new ExpressAdapter(server),
    {
      bufferLogs: true,
    },
  );
  app.useLogger(app.get(Logger));

  await app.init();

  server.listen(3000);
}

// bootstrap();
bootstrapCreateServer();

Please mention other relevant information such as Node.js version and Operating System.

OS: Debian (testing) Node.js: v16.8.0 Nest: v8.0.6 nestjs-pino: v2.2.0

Thanks for creating nestjs-pino!

iamolegga commented 3 years ago

Thanks, will check soon. Also, if you know how to fix feel free to open PR

iamolegga commented 2 years ago

just checked, pretty strange behavior: if comment this line everything works. So from my point of view, this could be a bug in Nestjs for a specific case when using Adapter and bufferLogs is set. This can be checked with dummy logger implementation

iamolegga commented 2 years ago

so as a workaround you can disable logs buffering on startup. Closing but feel free to provide a dummy logger example if you would like to check if this is a bug of nestjs-pino or framework

micalevisk commented 2 years ago

@fuglu I think we must call app.listen instead of server.listen. It works if you do that

128keaton commented 1 year ago

I am still having this issue, I unfortunately need to call server.listen since I am utilizing both an SSL and non-SSL server for my project. Without buffering logs, I can make the app emit logs semi-appropriately, however, logs before a certain point are using the default logger:

Screenshot 2023-07-17 at 11 57 12 AM

Considering opening an issue on the NestJS repository, however, I think this is something that can be addressed within nestjs-pino?

iamolegga commented 1 year ago

@128keaton please check if you still see a problem with other loggers (maybe try one from the docs) then this is surely not a bug of the current library. No need to add dirty hacks to handle bugs in the framework. Otherway feel free to open a bug report with the minimal example here.

jeg0330 commented 2 months ago

The listen() function calls flushLogs() to clear the logs accumulated in the buffer, but init() doesn't call it separately. You can call flushLogs() to do so