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 ourterms of serviceand privacy statement.We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: onApplicationReady Hook #13806

Open
1 task done
StimulCrossopened this issue Jul 20, 2024 · 14 comments
Open
1 task done

Feat: onApplicationReady Hook #13806

StimulCrossopened this issue Jul 20, 2024 · 14 comments

Comments

@StimulCross
Copy link

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

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

There are a lot of potential tasks that can be done successfully only after the start of an application (when the app is ready to handle connections).

TheonApplicationBootstraptriggers beforeawait app.listen(),so it can't be used for such tasks (correct me if I'm wrong).

Describe the solution you'd like

Although this can be easily implemented using events, another good option would be introducing a new hook that triggers afterawait app.listen().

Teachability, documentation, adoption, migration strategy

The hook's name can beonApplicationReadyoronApplicationStart.The usage of the hook will be pretty similar to the other hooks:

exportclassCustomProviderimplementsOnApplicationReady{
onApplicationReady():void{
console.log('Ready!')
}
}

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

One of the use cases could be webhook subscriptions. When I subscribe to webhooks, the publisher sends me a POST request to verify my webhook callback. It is safer to perform this task after the application is ready to listen for connections.

@StimulCross StimulCross added needs triage This issue has not been looked into type: enhancement 🐺 labels Jul 20, 2024
@CarltonK
Copy link

CarltonK commented Sep 3, 2024

+1 support
Is this feature being considered by the NestJs team?

@micalevisk
Copy link
Member

micalevisk commented Sep 3, 2024

I'm not sure about the naming and the role of that hook. I'd need to think more about it. But I can say that maybe we need a more specific name because 'ready' can be interpreted in several ways

You can supply a callback already toapp.listenmethod but I get that you want to make things more coupled with nestjs providers and modules; more high-level

@CarltonK
Copy link

CarltonK commented Sep 3, 2024

I'm not sure about the naming and the role of that hook. I'd need to think more about it. But I can say that maybe we need a more specific name because 'ready' can be interpreted in several ways

You can supply a callback already toapp.listenmethod but I get that you want to make things more coupled with nestjs providers and modules; more high-level

@micaleviskMaybe something like onApplicationInit

@micalevisk
Copy link
Member

'init' would be confusing because we haveapp.init()

@kamilmysliwiec
Copy link
Member

What about usinghttps://docs.nestjs /techniques/eventsand then

awaitapp.listen(...);
awaitapp.get(EventEmitter2).emit(...);

Done(?) This gives you full control too

@StimulCross
Copy link
Author

@kamilmysliwiecI personally already use this approach. It just feels like this functionality should be available out of the box with the rest of the hooks and have similar usage. Without this hook, lifecycle hooks feel incomplete.

@kamilmysliwiec
Copy link
Member

The thing is,onApplicationReadyis just somewhat vague as "ready" might mean different things depending on the context. For some, it's just that the app is already listening to HTTP requests, but for those who have hybrid applications (HTTP + message broker) the definition is probably going to be different. Not to mention that those who use the "Standalone application" mode wouldn't use that hook at all.

@StimulCross
Copy link
Author

Yeah, I didn't take those things into account when I came up with the hook name. Naming is not my strong point:) There's a lot to think about here. You guys know better anyway.

@micalevisk micalevisk added needs clarification and removed needs triage This issue has not been looked into labels Sep 19, 2024
@SylvainMarty
Copy link

I'm also interested by this feature. I have been using NestJS as my backend framework for years and I'd love to contribute to NestJS.

The thing is,onApplicationReadyis just somewhat vague as "ready" might mean different things depending on the context. For some, it's just that the app is already listening to HTTP requests, but for those who have hybrid applications (HTTP + message broker) the definition is probably going to be different. Not to mention that those who use the "Standalone application" mode wouldn't use that hook at all.

@kamilmysliwiecI think it would make sense to name the lifecycle "events" related to the methods we use to bootstrap a NestJS app:

constapp=awaitNestFactory.create(AppModule);
// Triggers the method `onApplicationCreated` of all `OnApplicationCreated` implementations
awaitapp.listen(...);
// Triggers the method `onApplicationListening` of all `OnApplicationListening` implementations.

What do you think of these namings?

@SylvainMarty
Copy link

On another subject, I'd like to react to this comment:

What about usinghttps://docs.nestjs /techniques/eventsand then

awaitapp.listen(...);
awaitapp.get(EventEmitter2).emit(...);

Done(?) This gives you full control too

I am not a big fan of EventEmitter2 but I have been implementing in by own synchronous event bus (with handlers priority management) in my backends for some time and it would really be awesome if NestJS had an internal event bus we could subscribe to when we need it.
Having bothonApplication[...]hooks and an event bus would give us much more freedom in how we want to architect our application code.
I would also love to provide a PR with an implementation of it in NestJS so we can discuss about it together if you're interested.

@micalevisk
Copy link
Member

@SylvainMartyas we can doapp.init()withoutlisten,the 'on application created' name would be confusing

I still have no ideia about the naming of this

@SylvainMarty
Copy link

@micaleviskThank you for your answer.

I'm not familiar with theawait app.init()method but from its documentation, it's an optional method that calls the Nest lifecycle events. From the source code, callingawait app.listen()actually callsawait app.init()in the background.

To me, it would be useful to have the following lifecycle events:

  1. await NestFactory.create(AppModule)->OnApplicationCreatehooks are called
  2. await app.init()->OnApplicationInithooks are called (previously calledOnApplicationBootstrap,we could keep the old hook to avoid introducing breaking change)
  3. Ifawait app.listent()is called ->OnApplicationListenhooks are called
  4. Using the listen method or not, the more general purpose hooksOnApplicationStartare called

About the methodawait app.startAllMicroservices():since it callsawait mc.listen()of the connected microservice, the lifecycle events would follow the said lifecycle for each microservice.

Let me know what you think of this.

@micalevisk
Copy link
Member

Looks good so far but I guess we won't rename nor change/deprecate existing hooks anytime soon

@mckramer
Copy link

I was looking into liveness/readiness indicators for Kubernetes (via Terminus). And found some events, such as the proposal here as being required to properly manage the probe states. I also develop Spring Boot apps and there are a lot of comparable patterns provided by that framework.

Specifically re: application events, Spring provides a set ofapplication eventsthat include what is being requested here, so that may be useful to compare the definitions they use.

A direct use-case for thisApplicationReadyEvent(Spring's equivalent) would allow for the readiness indicator to be determined via same logic as Spring, which is something I would like to make happen in my apps. Manually updating the readiness state afterawait app.listen(...)(as others have shared) is how I have approached it up to this point.

Ultimately the number ofNest's lifecycle eventsare somewhat finite, but I wonder if producing them via an emitter/filterable listeners as opposed to distinct hook methods could be easier/more scalable?

Some related links re: Kubernetes probes:

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

No branches or pull requests

6 participants