-
Notifications
You must be signed in to change notification settings - Fork 291
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
Release method #1179
Comments
Thanks for submitting this issue. I want to clarify a few things here.
OnStop hook is actually called even if the app fails to start because one of the start hooks fail. In fact, if an app fails to start, only the OnStop hooks whose OnStart counterparts successfully ran would be run, in reverse order of the OnStart hooks were run. This is indeed for cleaning up any resources that may have been taken during OnStart execution phase. If this was not clear from the existing documentation, we should go and address that, but I believe Fx already provides what you are looking for. |
Hi @sywhang, The concern is more about application lifecycle.
However, in the real world, dependency injection also includes resource allocation. And some applications don't even need to call fx.Start, but they still need to release resources that were allocated during dependency injection. |
Hey @saturn4er - here's my two cents:
From my understanding, these two statements are conflicting. If you have some resources you need to allocate and then release, that is a reason why a service does need to start/stop the app. In general, anything that is required to be cleaned up is supposed to be delegated to hooks, not run within constructors. Do you have any reasoning/examples behind avoiding starting/stopping the app? |
Hey @JacobOaks
Not really For example, you have some function which builds set of options for your application, where you define providers for sql.DB, some repository and some service. And there's two binaries where you want to use it. For example in server and in some data migration command which populates db. In second one you don't need to start application, you can just provide Invoke which will accept service, call method of this service and return error. func applicationOptions() fx.Option {
return fx.Options(
NewDB,
NewRepo,
NewService,
...
)
} server.go func main(){
fx.New(
applicationOptions(),
fx.Provide(NewHTTPServer),
fx.Invoke(RegisterStartServerHooks),
).Run()
} data_migration.go func main(){
app := fx.New(
applicationOptions(),
fx.Invoke(MigrateDataUsingService),
)
if err := app.Error(); err!=nil{
panic(err)
}
} |
@sywhang It seems like documentation label do not match this issue |
Hi @saturn4er, I think there's some core misunderstanding here. In general, an Fx app has two distinct phases. The first phase, initialization, is purely for initializing dependencies. Namely, it should not be used for starting anything that must be stopped later. That should happen in the second phase, execution. This phase runs any hooks registered during initialization, making sure to run associated stop hooks later. I would recommend reading these docs to become more familiar: https://uber-go.github.io/fx/lifecycle.html. If you have a specific example that current lifecycle & module structure does not support, feel free to leave it here, but with the example you gave alone, it's not clear why you cannot structure your modules such that core functionality can be re-used by both programs while removing program-specific functionality from the common module. |
Hi @JacobOaks, I understand the lifecycle of fx applications. The second example is not a long running applications it just runs some small job and dies. I'll provide more detailed example: func applicationOptions() fx.Option {
return fx.Options(
fx.Provide(fx.Annotate(NewDB, fx.OnStop(CloseDB))), // Here we create database connection that should be closed.
)
} server.go - this one is main service(application that starts and wait until linux will send some signal) func main(){
fx.New(
applicationOptions(),
fx.Provide(func(db *sql.DB) *http.Server {
return &http.Server{
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// query database and write response
}),
}
}),
fx.Invoke(func(lc fx.Lifecycle, server *http.Server) {
lc.Append(fx.Hook{
OnStart: func(ctx context.Context) error {
return server.ListenAndServe()
},
OnStop: func(ctx context.Context) error {
return server.Shutdown(ctx)
},
})
}),
).Run()
} job.go - this one starts and dies after all invokes will be executed. func main(){
if err := fx.New(
applicationOptions(),
fx.Provide(func(db *sql.DB) *http.Server {
return &http.Server{
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// query database and write response
}),
}
}),
fx.Invoke(func(db *sql.DB) error {
// do some work with db, for example applying migrations and die
return nil
}),
).Err(); err!=nil {
fmt.Println("err")
}
} |
Why can't you
FWIW, based on the comment, this invoke should probably also be in a start hook, and so should whatever initiates your connection to the DB (whatever needs cleaned up in CloseDB). |
fx.Invoke(func(db *sql.DB, lc fx.Lifecycle, shutdowner fx.Shutdowner) {
lc.Append(fx.Hook{
OnStart: func(ctx context.Context) error {
// do some work with db, for example applying migrations and die
shutdowner.Shutdown()
return nil
},
})
}) It looks like some workaround other than good solution. I don't need to start any application here, just invoke some function |
FWIW, this snippet matches what I've been doing for short-lived Fx applications. |
Is your feature request related to a problem? Please describe.
Currently, the fx.App lifecycle management allows us to create and start application components seamlessly. However, there's a missing piece in the lifecycle: a clean, explicit way to release resources such as database connections or open files, which are initiated during provider invocation. This becomes particularly necessary in scenarios where the application might not reach the .Start phase, or when only a single job is needed, thus not requiring the full application start-stop cycle. Without an explicit release mechanism, we are forced to implement ad-hoc, potentially error-prone solutions to ensure resources are properly released.
Describe the solution you'd like
I propose the addition of a .Release method to the fx.App structure, alongside a new lifecycle hook: fx.OnRelease. This would allow developers to define clean-up functions that are executed when .Release is called, ensuring that all resources are properly closed or released, regardless of whether the application fully starts or not. This feature could be implemented as follows:
Here, CloseMySQLDB would be a function designed to cleanly shut down the MySQL database connection established by NewMySQLDB, and would be automatically invoked when fx.App.Release is called.
Describe alternatives you've considered
One alternative might be to manually manage resource release outside of the fx lifecycle, by storing references to all resources that need to be released and explicitly calling their release methods. However, this approach can quickly become cumbersome and error-prone, especially in larger applications with many resources to manage.
Another alternative could be to use the existing OnStop hook for releasing resources. However, this is not always viable because OnStop is only called if the application successfully starts, which may not always be the case, especially in applications that perform tasks without entering the full start-stop cycle.
Is this a breaking change?
This proposal should not introduce breaking changes to the existing fx API. The addition of fx.App.Release and fx.OnRelease would be backward compatible, offering additional functionality without altering the current behavior of existing applications using the fx library.
The text was updated successfully, but these errors were encountered: