[Bug] BaseException exceptions are not caught when raised in an activity

[joshua-auchincloss]

What are you really trying to do?

• raise an exception subclassed from BaseException in an activity

Describe the bug

The bug occurs when raising custom exceptions during an activity. the system raises TypeError -> exception causes must derive from BaseException, which overshadows the actual error

Minimal Reproduction

....

# also with normal Exception
class MyException(BaseException):
    def __init__(self, *args):
        super().__init__(*args)

@activity.defn
async def my_act():
    raise MyException("abc") from None


@workflow.defn
class Workflow:
    @workflow.run
    async def run(self):
          workflow.start_activity(
            my_act,
            ....
          )

Environment/Versions

• OS and processor: m1 mac
• Temporal Version: temporal @ 1.20.2, python sdk @ 1.2.0
• Are you using Docker or Kubernetes or building Temporal from source? docker + build from wheel

[cretz]

I think you've written invalid Python code. I haven't tested yet, but I think super().__init__(self, *args) should be super().__init__(*args).

[joshua-auchincloss]

I think you've written invalid Python code. I haven't tested yet, but I think super().__init__(self, *args) should be super().__init__(*args).

Ahh this one's a typo - on mobile for the next few days 😅. Apologies, amending - the source is the later super().__init__(*args), which is the condition for the issue presenting

[cretz]

Ok, I see the issue. We intentionally only catch Exception and its derivatives, not BaseException. From https://docs.python.org/3/library/exceptions.html:

programmers are encouraged to derive new exceptions from the Exception class or one of its subclasses, and not from BaseException.

And from https://docs.python.org/3/tutorial/errors.html#handling-exceptions:

Exceptions which are not subclasses of Exception are not typically handled, because they are used to indicate that the program should terminate.

I'm not sure we want to trap BaseException, but maybe we do. Having said that, we don't want to silently swallow it either (which in effect is what is happening today), so should we crash the entire worker for a base exception?

In the meantime, please derive from Exception and not BaseException in Python. And if another library unfortunately is using BaseException as their base, you will have to catch and rethrow as a normal exception in your activity. But I'll leave this issue open because we should not silently swallow BaseException.

[joshua-auchincloss]

Ok, I see the issue. We intentionally only catch Exception and its derivatives, not BaseException. From https://docs.python.org/3/library/exceptions.html:

programmers are encouraged to derive new exceptions from the Exception class or one of its subclasses, and not from BaseException.

And from https://docs.python.org/3/tutorial/errors.html#handling-exceptions:

Exceptions which are not subclasses of Exception are not typically handled, because they are used to indicate that the program should terminate.

I'm not sure we want to trap BaseException, but maybe we do. Having said that, we don't want to silently swallow it either (which in effect is what is happening today), so should we crash the entire worker for a base exception?

In the meantime, please derive from Exception and not BaseException in Python. And if another library unfortunately is using BaseException as their base, you will have to catch and rethrow as a normal exception in your activity. But I'll leave this issue open because we should not silently swallow BaseException.

Gotcha - yep it's an upstream BaseException, no issues using try-except so thanks for the quick workaround :)

IMO, crashing the worker seems overkill - maybe a logging.warn? Either way, you guys do awesome work so thanks for the help