Skip to content

Commit f94bce7

Browse files
committed
fix: tell a stopped run apart from one the service killed
asyncio delivers a user pressing stop and a server-imposed execution ceiling as the same CancelledError, and the flow span reported both as cancelled with OTel status UNSET. That kept every timeout out of error-rate alerting, while the client was served a terminal error and the job row was written FAILED. The span was the only place claiming nothing had gone wrong. The two are now separate outcomes. A user stop stays cancelled with status UNSET, because a withdrawn request is not a service fault. Anything else that cancels a run reports aborted with span status ERROR, which is what the rest of the system already says about it. The discriminator already existed: the producers stamp a marker on the exception args, and the job service reads it to choose CANCELLED over FAILED. It was a bare string repeated at five sites, so the span could not have trusted it. It is one constant now, and it lives in lfx.constants rather than next to the graph exceptions: importing lfx.graph.exceptions from a langflow service module runs lfx/graph/__init__, which imports Graph, and that made langflow.main fail to import at all. lfx.constants imports nothing but pathlib.
1 parent d330b30 commit f94bce7

7 files changed

Lines changed: 78 additions & 19 deletions

File tree

src/backend/base/langflow/services/background_execution/runner.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from datetime import datetime, timedelta, timezone
2323
from typing import TYPE_CHECKING, Any
2424

25+
from lfx.constants import USER_CANCELLED_MESSAGE
2526
from lfx.log.logger import logger
2627

2728
from langflow.services.background_execution.live_bus import LiveFrame
@@ -122,7 +123,7 @@ async def _wrapped() -> None:
122123
paused = True
123124
await logger.adebug(f"Background job {job_id} suspended for human input")
124125
except asyncio.CancelledError as exc:
125-
user_tagged = bool(exc.args) and exc.args[0] == "LANGFLOW_USER_CANCELLED"
126+
user_tagged = bool(exc.args) and exc.args[0] == USER_CANCELLED_MESSAGE
126127
if not user_tagged and not await self._stop_requested(job_id):
127128
raise
128129
await logger.adebug(f"Background job {job_id} stopped")
@@ -319,7 +320,7 @@ async def _stop_requested(self, job_id: UUID) -> bool:
319320
@staticmethod
320321
def _user_cancelled() -> asyncio.CancelledError:
321322
exc = asyncio.CancelledError()
322-
exc.args = ("LANGFLOW_USER_CANCELLED",)
323+
exc.args = (USER_CANCELLED_MESSAGE,)
323324
return exc
324325

325326
@staticmethod

src/backend/base/langflow/services/job_queue/service.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
if TYPE_CHECKING:
99
from collections.abc import Coroutine
1010

11+
from lfx.constants import USER_CANCELLED_MESSAGE
1112
from lfx.log.logger import logger
1213

1314
from langflow.events.event_manager import EventManager
@@ -456,7 +457,7 @@ async def cleanup_job(self, job_id: str) -> None:
456457
# User-initiated cancellation so we explicitly called task.cancel() above
457458
await logger.adebug(f"Task for job_id {job_id} was successfully cancelled.")
458459
# Re-raise with user cancellation message code
459-
exc.args = ("LANGFLOW_USER_CANCELLED",)
460+
exc.args = (USER_CANCELLED_MESSAGE,)
460461
raise
461462
# System-initiated cancellation for other reasons
462463
await logger.adebug(f"Task for job_id {job_id} was cancelled by system.")

src/backend/base/langflow/services/jobs/service.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from datetime import datetime, timezone
1212
from uuid import UUID, uuid4
1313

14+
from lfx.constants import USER_CANCELLED_MESSAGE
1415
from lfx.graph.exceptions import GraphPausedException
1516
from sqlalchemy.exc import IntegrityError, OperationalError
1617
from sqlmodel import col, func, select
@@ -857,7 +858,7 @@ async def execute_with_status(self, job_id: UUID, run_coro_func, *args, **kwargs
857858

858859
except asyncio.CancelledError as exc:
859860
# Check the message code to determine if this was user-initiated or system-initiated
860-
if exc.args and exc.args[0] == "LANGFLOW_USER_CANCELLED":
861+
if exc.args and exc.args[0] == USER_CANCELLED_MESSAGE:
861862
# User-initiated cancellation, update status to CANCELLED
862863
await logger.awarning(f"Job {job_id} was cancelled by user")
863864
await self.update_job_status(job_id, JobStatus.CANCELLED, finished_timestamp=True)

src/backend/base/langflow/services/task/service.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
from typing import TYPE_CHECKING, Any
66
from uuid import UUID, uuid4
77

8+
from lfx.constants import USER_CANCELLED_MESSAGE
9+
810
from langflow.exceptions.api import WorkflowResourceError, WorkflowServiceUnavailableError
911
from langflow.services.base import Service
1012
from langflow.services.deps import get_queue_service
@@ -98,7 +100,7 @@ async def revoke_task(self, task_id: UUID | str) -> bool:
98100
try:
99101
await job_queue_service.cleanup_job(str(task_id))
100102
except asyncio.CancelledError as e:
101-
if str(e) != "LANGFLOW_USER_CANCELLED":
103+
if str(e) != USER_CANCELLED_MESSAGE:
102104
raise
103105
return True
104106
return True

src/backend/tests/unit/services/telemetry/test_flow_execution_span.py

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,17 +203,20 @@ async def main():
203203
"""
204204
)
205205

206-
# CancelledError is a BaseException, so an `except Exception` handler never sees it. A user
207-
# pressing stop and the v2 execution ceiling both cancel the driver this span wraps.
206+
# CancelledError is a BaseException, so an `except Exception` handler never sees it. Both a user
207+
# pressing stop and a server-imposed ceiling arrive as this one type, and they mean opposite
208+
# things to an operator, so the span has to tell them apart.
208209
CANCELLED_PROBE = (
209210
PROVIDER_SETUP
210211
+ """
212+
from lfx.constants import USER_CANCELLED_MESSAGE
213+
211214
async def main():
212215
graph = build_graph()
213216
raised = False
214217
try:
215218
with graph.flow_execution_span():
216-
raise asyncio.CancelledError()
219+
raise asyncio.CancelledError(USER_CANCELLED_MESSAGE)
217220
except asyncio.CancelledError:
218221
raised = True
219222
report({"raised": raised})
@@ -222,6 +225,28 @@ async def main():
222225
"""
223226
)
224227

228+
# A wall-clock ceiling, exactly as asyncio.wait_for delivers it: an untagged CancelledError.
229+
ABORTED_PROBE = (
230+
PROVIDER_SETUP
231+
+ """
232+
async def main():
233+
graph = build_graph()
234+
235+
async def forever():
236+
with graph.flow_execution_span():
237+
await asyncio.sleep(10)
238+
239+
timed_out = False
240+
try:
241+
await asyncio.wait_for(forever(), timeout=0.05)
242+
except (asyncio.TimeoutError, TimeoutError):
243+
timed_out = True
244+
report({"timed_out": timed_out})
245+
246+
asyncio.run(main())
247+
"""
248+
)
249+
225250
NO_OTEL_PROBE = """
226251
import asyncio, json, sys
227252
@@ -368,7 +393,7 @@ def test_an_inner_binding_does_not_overwrite_the_surface_that_took_the_request()
368393
assert result["after"] is None
369394

370395

371-
def test_a_cancelled_flow_is_not_recorded_as_a_successful_one():
396+
def test_a_flow_a_user_stopped_is_not_recorded_as_a_successful_one():
372397
result = run_probe(CANCELLED_PROBE)
373398
assert result["raised"] is True
374399

@@ -378,3 +403,15 @@ def test_a_cancelled_flow_is_not_recorded_as_a_successful_one():
378403
# A withdrawn request is not a service fault, so it must not land on the error rate.
379404
assert span["status"] == "UNSET"
380405
assert "error.type" not in span["attrs"]
406+
407+
408+
def test_a_flow_killed_by_a_timeout_is_recorded_as_an_error():
409+
"""The client is served an error and the job row says FAILED, so the span must agree."""
410+
result = run_probe(ABORTED_PROBE)
411+
assert result["timed_out"] is True
412+
413+
assert len(result["spans"]) == 1
414+
span = result["spans"][0]
415+
assert span["attrs"]["status"] == "aborted"
416+
assert span["status"] == "ERROR"
417+
assert span["attrs"]["error.type"] == "CancelledError"

src/lfx/src/lfx/constants.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,10 @@
44

55
# Base path for components - will be in lfx package when components are moved
66
BASE_COMPONENTS_PATH = str(Path(__file__).parent / "components")
7+
8+
# Marks a CancelledError as "a human withdrew this run" rather than "the service killed it".
9+
# asyncio gives one exception type for both, so the distinction rides on args and the producers
10+
# stamp it. It lives here rather than next to the graph exceptions because langflow's service
11+
# modules read it too, and importing lfx.graph.* from them early enough drags lfx/graph/__init__
12+
# (which imports Graph) into a circular import. This module imports nothing but pathlib.
13+
USER_CANCELLED_MESSAGE = "LANGFLOW_USER_CANCELLED"

src/lfx/src/lfx/graph/graph/base.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
from ag_ui.core import RunFinishedEvent, RunStartedEvent
2020

21+
from lfx.constants import USER_CANCELLED_MESSAGE
2122
from lfx.exceptions.component import ComponentBuildError
2223
from lfx.graph.edge.base import CycleEdge, Edge
2324
from lfx.graph.exceptions import GraphPausedException
@@ -876,18 +877,27 @@ def flow_execution_span(self, *, make_current: bool = True):
876877
# driven through Graph.process by the durable runner, which opens its own span.
877878
status = "paused"
878879
raise
879-
except asyncio.CancelledError:
880+
except asyncio.CancelledError as exc:
880881
# CancelledError is a BaseException, so the handler below does not see it and the
881-
# span would otherwise report the run as "ok". Reached by a user pressing stop (the
882-
# job service marks the job CANCELLED and re-raises) and by any asyncio.wait_for
883-
# ceiling wrapped around a span-carrying run: the v2 build driver, a2a, and the
884-
# agentic assistant all have one. The run did not finish, so it gets its own value.
882+
# span would otherwise report the run as "ok". Two very different things arrive as
883+
# the same exception type, and they must not share an outcome:
885884
#
886-
# Span status stays UNSET, which is right for a stop button and arguable for a
887-
# timeout: a server-imposed ceiling is closer to a fault, and an operator alerting
888-
# on span error rate will not see it. Left as one value for now because the two are
889-
# indistinguishable here; the job row (CANCELLED vs FAILED) still tells them apart.
890-
status = "cancelled"
885+
# A user pressing stop withdrew the request. Nothing is wrong with the service, so
886+
# span status stays UNSET and this never reaches error-rate alerting.
887+
#
888+
# Anything else cancelling the run is the service failing to deliver it: an
889+
# asyncio.wait_for execution ceiling (the v2 build driver, a2a and the agentic
890+
# assistant each have one), a worker shutdown, a parent task being torn down. The
891+
# client is served an error and the job row is written FAILED, so the span says
892+
# ERROR too. Reporting these as merely "cancelled" hid every timeout from alerting.
893+
#
894+
# The producers stamp the user case on args; see USER_CANCELLED_MESSAGE.
895+
if exc.args and exc.args[0] == USER_CANCELLED_MESSAGE:
896+
status = "cancelled"
897+
else:
898+
status = "aborted"
899+
span.set_status(otel_trace.Status(otel_trace.StatusCode.ERROR, "CancelledError"))
900+
span.set_attribute("error.type", "CancelledError")
891901
raise
892902
except Exception as exc:
893903
status = "error"

0 commit comments

Comments
 (0)