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

Close the connection when app is being shut down #10

Open
B4nan opened this issue May 21, 2020 · 6 comments
Open

Close the connection when app is being shut down #10

B4nan opened this issue May 21, 2020 · 6 comments

Comments

@B4nan
Copy link
Collaborator

B4nan commented May 21, 2020

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
Copy link

rhyek commented May 26, 2020

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
Copy link

rhyek commented May 27, 2020

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
Copy link
Collaborator Author

B4nan commented May 27, 2020

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
Copy link

rhyek commented May 27, 2020

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
Copy link
Collaborator Author

B4nan commented May 27, 2020

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
Copy link

rhyek commented May 27, 2020

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.

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

No branches or pull requests

2 participants