RealOrangeOne / django-tasks

A reference implementation and backport of background workers and tasks in Django
https://pypi.org/project/django-tasks/
BSD 3-Clause "New" or "Revised" License
314 stars 22 forks source link

Allow `ImmediateBackend` to throw its errors #50

Closed RealOrangeOne closed 2 months ago

RealOrangeOne commented 3 months ago

If a task fails using the ImmediateBackend, this exception is currently swallowed and isn't accessible anywhere. This should be rectified by adding a logger.exception call to ensure the error is seen, and captured by error monitoring. This matches the behaviour of the database backend.

RealOrangeOne commented 3 months ago

Currently, Exception is caught. This may need extending to BaseException with cut-outs for KeyboardInterrupt

jribbens commented 3 months ago

Currently, Exception is caught. This may need extending to BaseException with cut-outs for KeyboardInterrupt

That would mean it catches GeneratorExit and SystemExit, why would you want that?

RealOrangeOne commented 3 months ago

If a task throws SystemExit for whatever reason, I don't want the process to terminate. It should be caught as if the task failed, and control continue.

KeyboardInterrupt only needs cutting out as a quality of life fix. There might be others like it, but I don't think SystemExit is one of them.

jribbens commented 3 months ago

Catching and squashing SystemExit is a bold strategy ;-) But if you really want to do that then explicitly catching Exception and SystemExit would seem the appropriate way.