[Bug] async def update validator is accepted but never runs, so every update is accepted
Maintainer antworten meist innerhalb von 1 Tag
Ein zugehöriger Pull Request wurde bereits gemerged.
- #2005 von @avanish-garg — gemerged
Bewertung
- Schwierigkeit
- 2/5
- Geschätzter Aufwand
- 1-3 Stunden
- Anfängerfreundlichkeit
- 78/100
Rechercherichtung
Beginne in temporalio/worker/_workflow_instance.py bei WorkflowInboundImpl.handle_update_validator und verfolge dann die Registrierung von Validatoren über _UpdateDefinition.set_validator sowie die Pfade über den Decorator, set_update_handler und set_dynamic_update_handler. Lies zuerst die zugehörigen Tests zur Update- und Query-Validierung. Erledigt ist die Aufgabe, wenn asynchrone Validatoren bei der Registrierung über diese Pfade hinweg abgelehnt werden, während synchrone Validatoren weiterhin funktionieren.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Beschreibung
What are you really trying to do?
Reject an update with invalid arguments before it is written to history, using an update validator on a workflow whose update handler is async def.
Describe the bug
@my_update.validator accepts an async def function without any error, but the validator never executes, so the update is always accepted.
The inbound interceptor calls the validator without awaiting it (WorkflowInboundImpl.handle_update_validator in temporalio/worker/_workflow_instance.py, handler(*input.args)), unlike handle_signal, handle_query and handle_update_handler, which check inspect.iscoroutinefunction and await. For an async def validator the call only creates a coroutine object, which is dropped. The only symptom is a RuntimeWarning: coroutine ... was never awaited on the worker, so a validator that is meant to reject bad input silently lets everything through.
The README says "Validators cannot be async", and the other definition-time mistakes are all reported when the workflow is defined (@workflow.run on a non-async function raises ValueError, duplicate handlers raise ValueError, a validator/handler signature mismatch raises ValueError since #723, async queries emit a DeprecationWarning). The async validator is the one case with no check at all. pyright and mypy do flag it statically, but nothing does at runtime.
The same happens through the dynamic registration paths, workflow.set_update_handler(name, handler, validator=async_fn) and workflow.set_dynamic_update_handler(handler, validator=async_fn).
Expected: a ValueError when the validator is registered (decorator time, as for the signature mismatch). Actual: no error, and the update with n=-5 below is accepted.
Minimal Reproduction
import asyncio, uuid
from temporalio import workflow
from temporalio.client import WorkflowUpdateFailedError
from temporalio.testing import WorkflowEnvironment
from temporalio.worker import UnsandboxedWorkflowRunner, Worker
@workflow.defn
class W:
def __init__(self) -> None:
self.done = False
self.total = 0
@workflow.run
async def run(self) -> int:
await workflow.wait_condition(lambda: self.done)
return self.total
@workflow.update
async def add_sync(self, n: int) -> int:
self.total += n
return self.total
@add_sync.validator
def _v_sync(self, n: int) -> None:
if n < 0:
raise ValueError("negative")
@workflow.update
async def add_async(self, n: int) -> int:
self.total += n
return self.total
@add_async.validator
async def _v_async(self, n: int) -> None: # accepted by the decorator
if n < 0:
raise ValueError("negative")
@workflow.signal
def finish(self) -> None:
self.done = True
async def main():
async with await WorkflowEnvironment.start_local() as env:
async with Worker(
env.client,
task_queue="tq",
workflows=[W],
workflow_runner=UnsandboxedWorkflowRunner(),
):
h = await env.client.start_workflow(W.run, id=str(uuid.uuid4()), task_queue="tq")
try:
await h.execute_update(W.add_sync, -5)
print("sync validator : ACCEPTED -5 (unexpected)")
except WorkflowUpdateFailedError as e:
print("sync validator : rejected -5 ->", repr(e.cause)[:60])
print("async validator: returned", await h.execute_update(W.add_async, -5))
await h.signal(W.finish)
print("final total:", await h.result())
asyncio.run(main())
Output on current main (646d70f5):
.../temporalio/worker/_workflow_instance.py:3264: RuntimeWarning: coroutine '_bind_method.<locals>.with_object' was never awaited
handler(*input.args)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
sync validator : rejected -5 -> ApplicationError('ValueError: negative')
async validator: returned -5 (update with n=-5 ACCEPTED; total is now negative)
final total: -5
(UnsandboxedWorkflowRunner is only used to keep the script short.)
Environment/Versions
- OS and processor: M-series Mac (macOS 26.3.1, arm64)
- Temporal Version: SDK
mainat646d70f5(pure Python from the repo,temporalio1.34.0 wheel bridge), Python 3.12.14, dev server fromWorkflowEnvironment.start_local()(Temporal CLI 1.9.1, server 1.32.0) - Are you using Docker or Kubernetes or building Temporal from source? No
Additional context
Proposed fix, in the PR that will follow: raise ValueError("Update validator must not be an async function") in _UpdateDefinition.set_validator, which the decorator and both set_update_handler / set_dynamic_update_handler go through. It is a (very small) behavior change, since a workflow that has an async def validator today would now fail at class definition instead of silently accepting every update. A DeprecationWarning like the one for async queries would keep that workflow loading, but it would keep the validation switched off, so I went with raising. Happy to switch to a warning if you prefer.
- Vorherrschende Sprache
- Python
- Sterne
- 1.2k
- Forks
- 269
- Ø Merge
- 1 T. 11 Std.
- Gemergte PRs (30 T.)
- 68
Entwicklungsumgebung
- Kein Dockerfile und keine Docker-Compose-Datei
- Keine Pull-Request-Vorlage
- Beitragsleitfaden lesen
Erste Schritte
- Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
- Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
- Forken Sie das Repository und arbeiten Sie in einem Branch.
- Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.
Mehr aus temporalio/sdk-python
-
bug
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 68/100
temporalio/sdk-python#496 ·
Maintainer antworten meist innerhalb von 1 Tag
-
[Bug] Retried `ActivityHandle.terminate()` can fail after the activity is already terminatedEvtl. vergeben @maciejdudko hat das vor 1 Tag übernommen. Offen
temporalio/sdk-python#2008 · 1 zugewiesene Person ·
Maintainer antworten meist innerhalb von 1 Tag
-
[Bug] Annotated constraints and validators on workflow/activity/handler parameters are ignored by the Pydantic data converterEvtl. vergeben @tconley1428 hat das vor 1 Tag übernommen. Offenbug
temporalio/sdk-python#2006 · 1 Kommentar · 1 zugewiesene Person ·
Maintainer antworten meist innerhalb von 1 Tag
-
Proposal: handle SIGTERM by default to initiate graceful worker shutdownEvtl. vergeben @eamsden hat das vor 15 Tagen übernommen. Offen
temporalio/sdk-python#1948 · 1 zugewiesene Person ·
Maintainer antworten meist innerhalb von 1 Tag
-
Cloud CI Skips Nexus TestsEvtl. wieder frei @tconley1428 hat das vor 73 Tagen übernommen, und es ist kein Pull Request offen. Offen
temporalio/sdk-python#1704 · 1 zugewiesene Person ·
Maintainer antworten meist innerhalb von 1 Tag
Alle Issues in temporalio/sdk-python
Ähnliche Issues
-
enhancement
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 72/100
fossasia/visdom-dev#77 ·
Maintainer antworten meist innerhalb von 4 Tagen
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 82/100
aws-cloudformation/cfn-lint#4890 ·
Maintainer antworten meist innerhalb von 2 Tagen
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 82/100
volcengine/OpenViking#6521 ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 84/100
-
bug
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 77/100
AstrBotDevs/AstrBot#10720 ·
Maintainer antworten meist innerhalb von 1 Tag