search

rust-lang / discord-mods-bot

discord moderation bot
MIT License
72 stars 30 forks source link

Fault tolerance improvements #73

Closed technetos closed 3 years ago

technetos commented 3 years ago

Dont kill the main thread when we get an error

technetos commented 3 years ago

@pietroalbini I am removing this line to prevent the main thread from being killed when an error propogates up. Removing call to exit(1) doesnt have any impact on the deployment does it?

pietroalbini commented 3 years ago

It should not, but I'm wondering why you want to remove it.

technetos commented 3 years ago

It should not, but I'm wondering why you want to remove it.

When AWS restarted the postgres instance the other day, the error made it all the way to the main fn. The error was logged and it killed the main thread, leaving only the jobs thread running and requiring the bot to be restarted. Removing the call to exit(1) allows the bot to continue running when an error propogates all the way to main.

pietroalbini commented 3 years ago

Hmm, how did that happen? My understanding is that exit(1) terminates the process, which can't leave the other threads running.

technetos commented 3 years ago

Hmm, how did that happen? My understanding is that exit(1) terminates the process, which can't leave the other threads running.

Ah, you may be right, from the logs it seemed like the jobs thread was still running but it may have also died. Regardless, Id like to prevent errors from propogating up and killing the running instance.

pietroalbini commented 3 years ago

I don't think removing the std::process::exit(1) call will achieve what you want: if that piece of code is reached the app() function already returned, and the application will exit since the end of main() was reached. The only thing the call to exit does is overriding the exit code to be 1 (properly signaling an error happened).

If you want an error in a command not to close the application you'll need to handle it, probably on the serenity side.

technetos commented 3 years ago

:thinking: Your totally correct. Closing.