dario1985 / nestjs-mikro-orm

NestJS MikroORM integration
36 stars 6 forks source link

Close the connection when app is being shut down #10

Open B4nan opened 4 years ago

B4nan commented 4 years ago

When one calls app.close(), it should automatically call orm.close() too. This means we need to hook into onApplicationShutdown and call the orm.close() there.

https://docs.nestjs.com/fundamentals/lifecycle-events#application-shutdown

rhyek commented 4 years ago

I was just about to post an issue about this. @B4nan, could you post an example of how you're closing it now? I want to figure out how to do it at the appmodule level so it works for e2e testing as well.

rhyek commented 4 years ago

I ended up doing this in case anyone needs an example:

@Module({
  imports: [
    MikroOrmModule.forFeature({
      entities: [Todo],
    }),
  ],
  providers: [TodoService],
  controllers: [AppController, TodosController],
})
export class AppModule implements OnModuleDestroy {
  static register(options?: {
    mikroOrmOptions?: MikroOrmModuleOptions;
  }): DynamicModule {
    return {
      module: AppModule,
      imports: [
        MikroOrmModule.forRoot({
          entities: [Todo],

          type: 'postgresql',
          host: process.env.DB_HOST,
          port: process.env.DB_PORT ? parseInt(process.env.DB_PORT) : 5432,
          user: process.env.DB_USER,
          password: process.env.DB_PASS,
          dbName: process.env.DB_DB,

          ...options?.mikroOrmOptions,
        }),
      ],
    };
  }

  constructor(private orm: MikroORM) {}

  async onModuleDestroy(): Promise<void> {
    await this.orm.close();
  }
}
B4nan commented 4 years ago

Personally I just call orm.close() in tests instead of app.close(). I was not aware of app.close() before my colleague used it in tests and was confused by the tests not finishing properly.

rhyek commented 4 years ago

Mm, yeah, but then I'd assume you'd have to call it somewhere in your app as well, because I suspect it could prevent graceful shutdowns from happening (for example when handling a SIGTERM). Have not tested if this behavior is indeed affected.

B4nan commented 4 years ago

So far there was no issue in this regard (never called close anywhere else, only in tests, and I am using the ORM from early days :]).

Trying to find some clues on other adapters, but looks like nothing there, only the nestjs/typeorm one does what I suggested here in the OP (they actually use onApplicationShutdown, so will change this suggestion in the OP as I believe they know what they are doing):

https://github.com/nestjs/typeorm/blob/6acde561118436a2db972f0630dd2eaf3794bcd1/lib/typeorm-core.module.ts

rhyek commented 4 years ago

Yeah, I had tested onApplicationShutdown before and it wasn't working for some reason in my tests, but just tried it again and it worked, so I'll just use that.