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

Mikro ORM check returns healthy although not connected #2460

Closed
2 of 4 tasks
noe-charmet opened this issue Dec 15, 2023 · 7 comments · Fixed by #2509 or #2511
Closed
2 of 4 tasks

Mikro ORM check returns healthy although not connected #2460

noe-charmet opened this issue Dec 15, 2023 · 7 comments · Fixed by #2509 or #2511

Comments

@noe-charmet
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

When Mikro ORM's connection object return false on the isConnected method of its [Connection](https://mikro-orm.io/api/core/class/Connection) object, the health check is still considered valid.

Minimum reproduction code

// app.module.ts
import {Modulle} from '@nestjs/common';
import {ConfigModule, ConfigService} from '@nestjs/config';
import {ConfigSchema} from './config';
import {MikroOrmModule} from '@mikro-orm/nestjs';
import {defineConfig} from '@mikro-orm/postgresql';
import {TerminusModule} from '@nestjs/terminus';
import {AppController} from './app.controller';

@Module({
  imports: [
    ConfigModule.forRoot({
      validationSchema: ConfigSchema,
      isGlobal: true,
    }),
    MikroOrmModule.forRootAsync({
      imports: [ConfigModule],
      useFactory: (config: ConfigService) => ({
        ...defineConfig({
          user: config.getOrThrow('POSTGRES_USER'),
          password: config.getOrThrow('POSTGRES_PASSWORD'),
          dbName: config.getOrThrow('POSTGRES_DATABASE'),
          host: config.getOrThrow('POSTGRES_HOST'),
          port: config.getOrThrow('POSTGRES_PORT'),
        }),
        autoLoadEntities: true,
        connect: true,
        migrations: {
          path: './data/migrations',
          pattern: /^[\w-]+\d+\.[tj]s$/,
        },
      }),
      inject: [ConfigService],
    }),
    TerminusModule
  ],
  controllers: [AppController],
})
export class AppModule {}
// app.controller.ts
import {Controller, Get} from '@nestjs/common';
import {
  HealthCheckService,
  MikroOrmHealthIndicator,
  HealthCheck,
  type HealthCheckResult,
} from '@nestjs/terminus';

@Controller('/')
export class AppController {
  constructor(
    private readonly HealthCheckService: HealthCheckService,
    private readonly MikroOrmHealthIndicator: MikroOrmHealthIndicator,
  ) {}

  @Get(['live', 'livez'])
  @HealthCheck()
  live(): Promise<HealthCheckResult> {
    return this.HealthCheckService.check([]);
  }

  @Get(['ready', 'readyz'])
  @HealthCheck()
  ready(): Promise<HealthCheckResult> {
    return this.HealthCheckService.check([
      () => this.MikroOrmHealthIndicator.pingCheck('database'),
    ]);
  }
}

Steps to reproduce

  1. run nest app
  2. Do not have any DB running
  3. Call GET /live endpoint

Expected behavior

I would expect the health check to be failing if there is actually no DB connection established

Package version

10.2.0

NestJS version

10.2.10

Node.js version

18.12.1

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@Tony133
Copy link
Contributor

Tony133 commented Dec 20, 2023

Can you create minimal reproduction in a cloneable git repository?

@murbanowicz
Copy link

murbanowicz commented Jan 5, 2024

I can confirm - we discovered the same issue today.
I kill DB container, but health checks returns "up" forever despite of that.

@Arcus16
Copy link

Arcus16 commented Jan 25, 2024

Hi, we face the same bug in our services. What is interesting is that the first pingCheck after the server starts will return an error (which is right), but all ping checks after it will return up

@ivanyhchun
Copy link

We face the same problem too.

@BrunnerLivio
Copy link
Member

I created a new beta release, would you mind checking? NPMJS link, install via:

npm i @nestjs/[email protected]

@Arcus16
Copy link

Arcus16 commented Jan 31, 2024

Hi @BrunnerLivio , I tested the @nestjs/[email protected] in one of our services, and it works as expected.
I tested these cases:

  1. The database connection cannot be established. (I intentionally set a wrong port for DB to test this.)
  2. Time out error ( I set timeout to 1ms to test this)

@BrunnerLivio
Copy link
Member

BrunnerLivio commented Feb 7, 2024

This has now been officially released with 10.2.2 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants