CatchedException is not a valid exception

[mjpieters]

Describe the bug

CatchedException does not inherit from Exception and so is not a valid exception in Python 3; see PEP 352. A TypeError is raised if you try to raise it or a subclass:

from doit.exceptions import TaskFailed

def task_failure():
    def will_fail():
        raise TaskFailed("This task always fails")
    return {"actions": [will_fail]}

This results in a typeerror on any Python 3.x version:

$ doit failure
.  failure
TaskError - taskid:failure
PythonAction Error
Traceback (most recent call last):
  File "/.../lib/python3.7/site-packages/doit/action.py", line 424, in execute
    returned_value = self.py_callable(*self.args, **kwargs)
  File "/.../dodo.py", line 5, in will_fail
    raise TaskFailed("This task always fails")
TypeError: exceptions must derive from BaseException

########################################
failure <stdout>:

The fix is to inherit from Exception:

class CatchedException(Exception):

Upvote & Fund

• We're using Polar.sh so you can upvote and help fund this issue.
• We receive the funding once the issue is completed & confirmed by you.
• Thank you in advance for helping prioritize & fund our backlog.

[mjpieters]

I may have misunderstood how these objects are meant to be used. I see we need to return, not raise, TaskFailed, for example. I would expect to have to raise them as an exception, especially as they are defined in the doit.exceptions module.

Can both methods be considered valid? If so, then I suspect more work is needed to handle them differently when caught.

[schettino72]

I guess the name might be misleading. CatchedException is not a python Exception but a wrapper that contains an Exception. Its bit of historical name, not really accurate but it is not part of exposed API and not sure what would be a better name 😬

Yes, TaskFailed must be returned not raised. On the other hand you can raise any exception to fail a task. The only difference is that you get simple error message or a full traceback.

[schettino72]

Maybe docs should have an example showing why & how this feature should be used.

[mjpieters]

An example use would be helpful.

Also, throwing a special exception is a common API choice; eg click has a hierarchy of exceptions it’ll catch, and you can throw one deep into a call stack and get a nicely formatted message and no traceback. I’d still love for the TaskFailed and TaskError objects to play such a role.

As for naming: grammatically speaking it’d be CaughtException, and HandledException might be work too. You could consider Error or Exit too (TaskExit?).

Can I also ask why do the Task* objects inherit from this type? It is the whole combination; inheriting from SomethingException, located in a module named exceptions and their function of signalling a task having failed that made me assume they were Python exceptions.

[schettino72]

Can I also ask why do the Task* objects inherit from this type?

A doit task can have 3 different outcomes: success, failure and error. So CatchedException is a base class to save data of an outcome (that might contain a python exception traceback). The lack of CatchedException (None) represents a success. I guess TaskExit or TaskOutcome would be a better name.

If my memory is correct TaskFailed could not subclass python Exception because these objects are passed around as pickled data when multi-process is used. Not sure this is still the case for python 3.

So for your use-case to throw one exception deep into a call we would probably need to create a set of new classes really derived from python Exception.

And since CatchedException is even grammatically wrong (thanks for pointing out), I guess we should rename it. A bit shameful...

[schettino72]

fixed in f0581bc
I used the name BaseFailas it never contains a successful result.