✨ feature: add callShutdownHooksOnSigInt function #1834
Conversation
|
Do we need that? Some users may not want to use SIGTERM with app.Shutdown. I think it should be optional, we can do this in the app. What do you think? @hi019 @ReneWerner87 |
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
|
I think this is a good addition. It allows for graceful shutdown by default -- could be useful for things like closing DB connections or removing temporary files, which would need to be done whether the app is killed or shutsdown regularly. Uber's Fx does the same thing: https://pkg.go.dev/go.uber.org/fx#App.Run |
|
Should I add an option to OnShutdown hook? |
|
I added |
|
I'll look at this later today |
| // Handlers define a function to create hooks for Fiber. | ||
| type OnRouteHandler = func(Route) error | ||
| type OnNameHandler = OnRouteHandler | ||
| type OnGroupHandler = func(Group) error | ||
| type OnGroupNameHandler = OnGroupHandler | ||
| type OnListenHandler = func() error | ||
| type OnShutdownHandler = OnListenHandler | ||
| type OnShutdownHandler = func(*os.Signal) |
There was a problem hiding this comment.
This would break backwards compatibility :(. Honestly I think not showing os.Signal is fine for now. Are there any situations where a hook would need to change its functionality based on the signal type?
There was a problem hiding this comment.
I think we can add DisableDefaultGraceful config instead. Otherwise, i dont think hardcoded things are good. What about? @ReneWerner87 @hi019 @Streamer272
There was a problem hiding this comment.
Sure, works for me. What's hardcoded, though?
There was a problem hiding this comment.
Sure, works for me. What's hardcoded, though?
Some users may want to add their custom signals.
| // prepare the server for the start | ||
| app.startupProcess() | ||
|
|
||
| sigChan := make(chan os.Signal) | ||
| signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) |
There was a problem hiding this comment.
I believe that we need to give autonomy to the people to treat signals according to their needs it.
Maybe we can create a method to explicit this signals default.
I created a package treat signals Golang maybe it can help you.
https://github.com/renanbastos93/ossignals
|
I believe graceful shutdown should be implemented by the user... |
|
Hi @Streamer272. v3 now has more detailed and user-defined automatic graceful shutdown processing with #1930. Thanks for the great idea 🚀 |
callShutdownHooksOnSigIntfunction to AppOnShutdownHandlerto take*os.SignalargumentWhen fiber app is killed by SIGINT or SIGTERM, fiber doesn't call shutdown hooks when should.
This pull request solves it by adding a channel to
Listen,ListenTLSandListenMutualTLSand callingcallShutdownHooksOnSigIntas a goroutine that listens on SIGINT and SIGTERM signals thanks to go's build inos/signal.After calling the shutdown hooks, function exists the program with
os.Exit(0).When calling all the on shutdown handlers, they might now want to execute when a specific signel is used to kill the app, so now they take
*os.Signalas argument and can implement their own logic.