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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TaskTimeoutError should be BaseException instead of Exception #241

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

psrok1
Copy link
Member

@psrok1 psrok1 commented Jan 17, 2024

Task timeout function sets alarm that triggers SIGALRM, which is then handled by throwing TaskTimeoutError. This mechanism should crash task that is running too long e.g. because of infinite loop or other conditon that caused the consumer to be hanged.

Unfortunately, TaskTimeoutError derives from Exception base class which means that it will be cached by any try..except clause that catches all Exceptions, which is pretty common pattern in Python, making timeout ineffective. This kind of situation happens in karton-config-extractor and malduck: https://github.com/CERT-Polska/malduck/blob/master/malduck/extractor/extractor.py#L447

This PR changes base class of TaskTimeoutError from Exception to BaseException (like GracefulShutdown).

BaseException is the common base class of all exceptions. One of its subclasses, Exception, is the base class of all the non-fatal exceptions. Exceptions which are not subclasses of Exception are not typically handled, because they are used to indicate that the program should terminate. They include SystemExit which is raised by sys.exit() and KeyboardInterrupt which is raised when a user wishes to interrupt the program.
from https://docs.python.org/3/tutorial/errors.html

@psrok1 psrok1 merged commit 46ffc2e into master Jan 17, 2024
6 checks passed
@psrok1 psrok1 deleted the fix/task-timeout-as-baseexception branch January 17, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants