Service disposer

[urugator]

Is there an existing issue that is already proposing this?

• [X] I have searched the existing issues

Is your feature request related to a problem? Please describe it

I would like to be able to define disposer function that gets called when leaving a relevant scope. Eg. return db connection to pool at the end of http request.

Describe the solution you'd like

{
  provide: DB_CONNECTION,
  scope: Scope.REQUEST,
  inject: [DB_POOL],
  useFactory: pool => getConnection(),  
  dispose: conn => conn.release()
},

Disposer should be perhaps able to somehow inject services as well.

It should be also possible to define disposer like @Injectable({ dispose: self => self.dispose() }).

Transient dependencies should be disposed when their parent scope is being disposed.

Workaround I use at the moment:

// providers
{
  provide: APP_INTERCEPTOR,
  useClass: ReleaseDbConnectionInterceptor,
},
{
  provide: DB_CONNECTION,
  scope: Scope.REQUEST,
  inject: [DB_POOL, REQUEST],
  useFactory: async (dbPool: mariadb.Pool, request: Request) => { 
    // Workaround so we can release the conn in interceptor
    request[DB_CONNECTION] = await dbPool.getConnection();
    return request[DB_CONNECTION];
  },  
},

// interceptor
@Injectable()
export class ReleaseDbConnectionInterceptor implements NestInterceptor {
  intercept(context: ExecutionContext, next: CallHandler): Observable<unknown> {
    return next.handle().pipe(
      finalize(() => {
        if (context.getType() === 'http') {
          const dbConn = context.switchToHttp().getRequest()[
            DB_CONNECTION
          ] as mariadb.PoolConnection;
          if (dbConn) {
            dbConn.release();
          }
        }
      }),
    );
  }
}

Teachability, documentation, adoption, migration strategy

Clear from above

What is the motivation / use case for changing the behavior?

Ability to dispose resources, short lived in particular, seems like a fairly common use case. For related work in other DI solutions, see eg: https://docs.oracle.com/javaee/7/api/javax/enterprise/inject/Disposes.html https://docs.microsoft.com/en-us/dotnet/core/extensions/dependency-injection-guidelines#disposal-of-services

[jmcdo29]

How would this be different than making the module implement OnModuleDestory and injecting the factory provider to the module, then doing whatever cleanup you need in the onModuleDestroy method?

Example

```ts @Module({ providers: [ { provide: KYSELY_OPTIONS_TOKEN, useValue: { port: 25432, host: 'localhost', user: 'postgres', password: 'postgres', database: 'postgres', }, }, { provide: KYSELY_TOKEN, inject: [getKyselyOptionsToken()], useFactory: async (options: PostgresDialectConfig) => { return new Kysely({ dialect: new PostgresDialect(options), }); }, }, ], exports: [KYSELY_TOKEN], }) export class KyselyModule implements OnModuleDestroy { constructor(@InjectKysely() private readonly db: Kysely) {} async onModuleDestroy() { await this.db.destroy(); } } ```

[urugator]

How would this be different

Afaik a module is always application scoped. onModuleDestroy is called on app shutdown. I want the disposer to be associated with a lifetime of a service, eg Scope.REQUEST service disposer is called when request ends, please see the workaround. Scope.TRANSIENT is disposed when the injecting service (dependant) is disposed.

Secondly there is no need for extra module. If it would be possible, I could just implement whatever method (onModuleDestroy) in a module where the service is defined. However design wise it doesn't make sense to handle this at the module level. You don't use onModuleInit to create services either. It should be defined at the level of service/provider.

[jmcdo29]

onModuleDestroy is also called when a request scoped module's request is finished.

It's not an extra module, it's just making use of the existing module where you define the factory provider in the first place.I get your point about tying the implementation to the provider, just showing there is currently a way we can deal with this, rather than having to check if a provider is a factory provider and then if it implements the dispose method (which we should probably call onModuleDestory to keep with the lifecycle hooks of other providers), and if we implement that should we also implement the other lifecycle hooks as well onModuleInit, onApplicationStart, etc? Or will this be only for onModuleDestroy?

[urugator]

onModuleDestroy is also called when a request scoped module's request is finished.

I don't follow / can't replicate. Can you please explain or post a snippet where onModuleDestroy is called at the end of each http request? Can a module be somehow request scoped?

rather than having to check if a provider is a factory provider

Event though allocating resources in constructor is probably uncommon and value provider probably won't ever require disposal I don't see why they should be treated differently. If there is a disposer defined, call it with the provided value, doesn't matter if it's class/factory/value...

if we implement that should we also implement the other lifecycle hooks

I am probably missing something, but I don't understand how it relates to onModuleDestroy. A module can define/export providers of different scopes, correct? Lifecycle of a module isn't bound to a lifecycle of one of it's services or is it?.

EDIT: Oh, is it perhaps called when implemented directly on @Injectable? Did not tried that.... EDIT: Nope, doesn't seem so...

[jmcdo29]

Hmm, I could have sworn we called onModuleDestroy for request scoped providers, but our docs explicitly mention that's not the case, and I just tested it myself. Perhaps I'm just used to doing this for singleton providers and made an incorrect assumption. Though, the modules do get garbage collected, so there is that. I'll double check the code and see if there's a spot to add this functionality in when I have the time.

@kamilmysliwiec any thoughts on the addition of a disposer or dispose method that can be called, or will that add too much complexity in provider lifecycle management?

[kamilmysliwiec]

{
  provide: DB_CONNECTION,
  scope: Scope.REQUEST,
  inject: [DB_POOL],
  useFactory: pool => getConnection(),  
},

to:

{
  provide: DB_CONNECTION,
  scope: Scope.REQUEST,
  inject: [DB_POOL, REQUEST],
  useFactory: (pool, request) => {
    const connection = getConnection();
    req.res.on('finish', () => connection.release());
    // or req.on('end', ..) depending on what you want to accomplish
    return connection;
  }
},

Note: express-specific example

@jmcdo29 request-scoped providers are tied to the request object lifetime (and garbage-collected once request is destroyed) so simply listening to its events (as above) should be sufficient.

[urugator]

Thank you for the suggestion, while I think it's still more of a workaround, it should be sufficient for my current use case.

[kamilmysliwiec]

Thank you for the suggestion, while I think it's still more of a workaround,

Even if we decided to implement a "disposer mechanism" at the framework level, the example I just provided would be literally what we'd have been doing under the hood. There's no other way around so adding an additional abstraction layer on top of that simply wouldn't make any sense

[kamilmysliwiec]

Also, and that's something to keep in mind, establishing a connection per request (and releasing it afterward) is gonna seriously hurt your performance and isn't recommended, so exposing a mechanism at the framework level for something like this would be against best practices.

[urugator]

literally what we'd have been doing under the hood

But can't you basically say the same about the factory functions - under the hood, you "just" create a service on incoming http connection. Like ok there is a nuance that it's created lazily only when requested by other service, but isn't the whole point of the container as an abstraction to manage the objects lifecycle for you? It doesn't do much else imo. Also when REQUEST scope ends it's pretty obvious, but not so with TRANSIENT as it depends on the scope of the consumer, which is completely under the control of the container - it's (correctly) non trasparent and it's what the container tries to abstract away. Some solutions provide scopes that can span eg multiple requests flow/session/convesation or you can implement your own. The container must tell me when these scopes ends, I can't just easily predict this. I can also imagine, it could have some implications for testing, like you could "fake" these scopes, so that you don't need an actual http connection to test your scoped services, but not if you depend on actual low level details like this.

establishing a connection per request (and releasing it afterward) is gonna seriously hurt your performance and isn't recommended

It (ideally in majority of cases) doesn't create a new connection per request, but picks an existing connection from pool. If your transaction boundary is a request, which is quite common practice and usually what you want (since single request usually represets a single operation that should be atomic), then there is no way around that, it's not like you can choose. You still end up wrapping the whole request handler in TX using single connection and if you wanna split the handler logic and you can't inject the connection as a service, then you have to drill the connection as param to each function/service call, at which point you have to ask yourself why am I using DI container if I still have to pass my deps manually.

[kamilmysliwiec]

But can't you basically say the same about the factory functions - under the hood, you "just" create a service on incoming http connection.

I think you're comparing apples with oranges here.

Also when REQUEST scope ends it's pretty obvious, but not so with TRANSIENT as it depends on the scope of the consumer

I think when TRANSIENT scope ends is pretty obvious too, and you actually pointed that out above - it ends when its host component scope ends.

If your transaction boundary is a request, which is quite common practice and usually what you want (since single request usually represets a single operation that should be atomic), then there is no way around that, it's not like you can choose.

You can choose how to implement that. Again, this doesn't seem to be related to the original problem.

Long story short, there are currently no plans to add such a feature in the foreseeable future

[kamilmysliwiec]

@urugator I'll make sure to ping you/everyone in this thread if we make a decision to abstract this way and add such a feature in one of the future versions :)