Skip to content

Do not wrap workflow-failure exceptions from converters in workflows #882

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jun 20, 2025
Merged

Do not wrap workflow-failure exceptions from converters in workflows #882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
194e87a
Allow pydantic validation error to fail workflow
666ares Jun 1, 2025
e4cd102
Fix formating
666ares Jun 1, 2025
c1a2ab6
Change type hint on workflow params
666ares Jun 1, 2025
4dffbe1
Merge branch 'main' into main
cretz Jun 4, 2025
fa21fc8
Merge branch 'main' into main
dandavison Jun 16, 2025
9a1ca86
Merge branch 'main' into main
cretz Jun 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions temporalio/worker/_workflow_instance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1792,6 +1792,8 @@ def _convert_payloads(
# Don't wrap payload conversion errors that would fail the workflow
raise
except Exception as err:
if self._is_workflow_failure_exception(err):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this has backwards compatibility concerns since it changes existing behavior based on raised exception, but I think it's ok in this instance.

But can we change the PR title to be a bit more generic so when it shows up in release notes it doesn't look like it's Pydantic specific? Maybe something like "do not wrap workflow-failure exceptions from converters in workflows"?

raise
raise RuntimeError("Failed decoding arguments") from err

def _instantiate_workflow_object(self) -> Any:
Expand Down
27 changes: 26 additions & 1 deletion tests/worker/test_workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
)
from urllib.request import urlopen

import pydantic
from google.protobuf.timestamp_pb2 import Timestamp
from typing_extensions import Literal, Protocol, runtime_checkable

Expand Down Expand Up @@ -5194,6 +5195,30 @@ async def run_scenario(
)


class Foo(pydantic.BaseModel):
bar: str


@workflow.defn(failure_exception_types=[pydantic.ValidationError])
class FailOnBadPydanticInputWorkflow:
@workflow.run
async def run(self, params: Foo) -> None:
pass


async def test_workflow_fail_on_bad_pydantic_input(client: Client):
async with new_worker(client, FailOnBadPydanticInputWorkflow) as worker:
with pytest.raises(WorkflowFailureError) as err:
await client.execute_workflow(
"FailOnBadPydanticInputWorkflow",
{"bar": 123}, # This should fail validation
id=f"wf-{uuid.uuid4()}",
task_queue=worker.task_queue,
)
assert isinstance(err.value.cause, ApplicationError)
assert "1 validation error for Foo" in err.value.cause.message


@workflow.defn(failure_exception_types=[Exception])
class FailOnBadInputWorkflow:
@workflow.run
Expand All @@ -5211,7 +5236,7 @@ async def test_workflow_fail_on_bad_input(client: Client):
task_queue=worker.task_queue,
)
assert isinstance(err.value.cause, ApplicationError)
assert "Failed decoding arguments" in err.value.cause.message
assert "Expected value to be str, was <class 'int'>" in err.value.cause.message


@workflow.defn
Expand Down
Loading