-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fix bot shutdown hang on connection error (or other unhandled exceptions) #6261
base: V3/develop
Are you sure you want to change the base?
Fix bot shutdown hang on connection error (or other unhandled exceptions) #6261
Conversation
Added a catch all exception to the try block where red.start() is called to ensure bot shuts down correctly.
On further thought, I realise that a catch all exception will probably not be acceptable, as it will catch exceptions during the running of the bot and not just at start up so I am going to convert this pull request to a draft and look for a better solution to this. |
Errors while the bot is running don't generally propagate this far and are handled earlier by things such as loop's exception handler. If they do propagate here, something more serious happened and we do probably want to shutdown after. If connection errors propagate here then the placement is likely fine. |
So do you think a catch all exception is appropriate or would catching more specific exceptions be better? |
It may be fine, can't really comment on that until I have a chance to test/play with it. |
ed040bb
to
c95a210
Compare
I suspect if any exception doesn't get caught by run_bot() while the bot is starting up then the hang will occur. |
red_exception_handler() hangs if it tries to call shutdown_handler() it's self so just call sys.exit() instead
I had a little more of a poke around and I think I found out the real cause of the hang: if red_exception_handler() catches an exception and calls shutdown_handler(), then the code never falls through back into main(). |
Description of the changes
Change red_exception_handler() from calling shutdown_handler() to sys.exit()
fixes #5780
Have the changes in this PR been tested?
Yes