[Feature Request] Fail workflow upon pydantic.ValidationError when using the pydantic_data_converter

[teije01]

Is your feature request related to a problem? Please describe.

When using the pydantic_data_converter from temporalio.contrib.pydantic (great feature by the way), it may happen that the payload cannot be decoded due to an incorrect payload. This results in a pydantic.ValidationError being raised upon decoding the payload.

The current behavior is for these failures to be re-raised as a RuntimeError. Then the default behavior for these errors is to result in a Workflow Task Failed (Failed decoding arguments) while keeping the workflow running. However, there is no way that such a workflow will ever complete with an incorrect payload.

In this issue https://community.temporal.io/t/force-workflow-failure-when-workflow-cannot-decode-arguments/10680/4 I found that you can specify which exceptions are treated as workflow failure, meaning there is a way to make the workflow fail directly upon workflow failures by using the experimental workflow_failure_exception_types.

Specifying the pydantic.ValidationError does not work as it is re-raised by temporalio as a RuntimeError

    workflow_failure_exception_types = [
        pydantic.ValidationError,  # does not work, is re-raised as RuntimeError upon decoding
    ]
    async with Worker(
        client,
        task_queue="task-queue",
        workflow_failure_exception_types=workflow_failure_exception_types,
    ):

I'm unsure about the side-effects of treating a RuntimeError as a workflow_failure_exception_types, because it is such a general exception.

Describe the solution you'd like

I would like to see a workflow failing directly upon 'Failed decoding arguments' while not having to specify a RuntimeError as an error in workflow_failure_exception_types.

I believe if the pydantic payload converter would raise a temporalio.exceptions.FailureError, would this then also fail the workflow? because that would seem most preferable.

Or otherwise, if the ValidationError would be re-raised instead of a RuntimeError, this would allow us to fail a workflow more specifically:

    workflow_failure_exception_types = [
        pydantic.ValidationError,
    ]

[dandavison]

Thanks for this excellent write-up @teije01.

[cretz]

👍 I think the best solution may be to stop wrapping the exception that comes back from argument conversion, or maybe at least only wrap if self._is_workflow_failure_exception is false for it. We already made #329 (as part #321), but that was for ApplicationError only because it predates "workflow failure exception types" concept.

[teije01]

Thanks! #321 does indeed seem like it would solve this more specific converter feature as well. This approach would make most sense to me for decoding failures that are not transient.

maybe at least only wrap if self._is_workflow_failure_exception is false for it.

That would make sense to me

[CtrlAltGiri]

@cretz I want to start contributing to this project.

Would love to pick this up since it's a simple change.

[cretz]

Yes, the change is fairly easy, feel free to submit a PR. Simply do not wrap the exception (i.e. re-raise the original) if the exception "is workflow failure exception".

[teije01]

@cretz I want to start contributing to this project.

Would love to pick this up since it's a simple change.

Let me know if you have time for it, I don't mind picking it up otherwise