CHANGELOG.md

@@ -35,6 +35,9 @@ and this project adheres to [Clever Semantic Versioning](https://www.w3.org/subm
 
 ### Fixed
 
+- **ConnectionError Retry Exhaustion Silent Swallow (issue #71)** (`nodes.py`): When all LLM communication retries are exhausted and `call_with_retry()` raises `ExecutionError(kind="timeout")`, the exception was systematically caught by broad `except Exception` handlers in `Node._execute_agent()` and `Node.execute()`, converting it into an error string in graph state. The retry mechanism's work (backoff, budget tracking, error reporting) was wasted and the graph silently returned error strings or empty responses instead of propagating the timeout. Fixed by adding targeted `except ExecutionError as e` handlers that check `e.kind == "timeout"` and re-raise, while non-timeout `ExecutionError` instances (tool errors, etc.) continue to be caught and handled gracefully. Two new BDD scenarios verify both propagation and graceful handling paths.
+
+  **Module:** `src/cleveractors/langgraph/nodes.py`. BDD: scenarios in `features/nodes_coverage_gaps.feature`.
 
 - **LLM Agent Synthesis-Round Tool Name Validation (issue #63)** (`llm.py`): When the model hallucinates a tool name not present in the declared tools list during the synthesis round, the invalid call is now rejected with a `ToolMessage` error stating `"Tool '<name>' is not available."` instead of crashing or silently discarding the response. A `_declared_names` set is built once per synthesis round from `self._lc_tools` for O(1) lookup.
 
@@ -158,6 +161,7 @@ and this project adheres to [Clever Semantic Versioning](https://www.w3.org/subm
 
 ### Fixed
 
+- **Timeout Propagation and Connection-Error Retry (issue #71)** (`nodes.py`, `retry.py`, `llm.py`): Fixed `ExecutionError(kind="timeout")` being silently caught by `_execute_agent()` and `execute()` instead of propagating to the caller. Extended `_is_http_comms_error()` to recognize provider SDK connection errors (`APIConnectionError`, `APITimeoutError`) as retryable transient failures. Added safety net in `process_message()` to wrap non-retried connection errors as `ExecutionError(kind="timeout")` for correct propagation.
 - **Canonicalizer depth guard**: Fixed DoS vector where deeply nested lists bypassed the `max_depth` recursion protection. The depth check now fires in `_transform()` before any recursion, covering dicts, lists, and all value types uniformly.
 - **Canonicalizer benchmark OOM**: Fixed exponential tree fixture in `benchmarks/canonicalizer_benchmark.py` where `_make_nested(depth=30, width=2)` created ~2 billion nodes. Changed to `width=1` for a true 30-level linear chain.
 - **Canonicalizer float normalization**: Added negative-zero normalization (`-0.0` → `0.0`) in `_transform()` for strict RFC-8785 compliance.

features/nodes_coverage_gaps.feature

@@ -53,6 +53,36 @@ Feature: Node Execution Without Event Loop, MESSAGE_ROUTER, and History Truncati
     When I exercise execute_agent with a failing agent (nodes_gaps)
     Then the error should be caught and an error response returned (nodes_gaps)
 
+  Scenario: ExecutionError with non-empty kind propagates through _execute_agent
+    Given a fresh nodes coverage gaps test context (nodes_gaps)
+    When I exercise execute_agent with an agent raising ExecutionError with kind "timeout" (nodes_gaps)
+    Then ExecutionError with non-empty kind should propagate to the caller (nodes_gaps)
+
+  Scenario: ExecutionError with kind "cost" also propagates (future-proof)
+    Given a fresh nodes coverage gaps test context (nodes_gaps)
+    When I exercise execute_agent with an agent raising ExecutionError with kind "cost" (nodes_gaps)
+    Then ExecutionError with non-empty kind should propagate to the caller (nodes_gaps)
+
+  Scenario: ExecutionError with empty kind is still caught by _execute_agent
+    Given a fresh nodes coverage gaps test context (nodes_gaps)
+    When I exercise execute_agent with an agent raising ExecutionError with empty kind (nodes_gaps)
+    Then the ExecutionError with empty kind should be caught and converted to error string (nodes_gaps)
+
+  Scenario: _is_http_comms_error recognizes APIConnectionError as retryable
+    Given a fresh nodes coverage gaps test context (nodes_gaps)
+    When I check _is_http_comms_error against APIConnectionError (nodes_gaps)
+    Then it should return True for APIConnectionError (nodes_gaps)
+
+  Scenario: _is_http_comms_error recognizes APITimeoutError as retryable
+    Given a fresh nodes coverage gaps test context (nodes_gaps)
+    When I check _is_http_comms_error against APITimeoutError (nodes_gaps)
+    Then it should return True for APITimeoutError (nodes_gaps)
+
+  Scenario: process_message wraps APIConnectionError as ExecutionError with kind=timeout
+    Given a fresh nodes coverage gaps test context (nodes_gaps)
+    When I exercise process_message with an agent raising APIConnectionError (nodes_gaps)
+    Then the resulting ExecutionError should have kind "timeout" (nodes_gaps)
+
   Scenario: Context diff tracks added and removed keys
     Given a fresh nodes coverage gaps test context (nodes_gaps)
     When I exercise execute_agent with context-mutating agent (nodes_gaps)

features/steps/nodes_coverage_gaps_steps.py

@@ -321,6 +321,214 @@ def step_agent_exception(context):
     _run_async(_run(), context=context)
 
 
+@when(
+    'I exercise execute_agent with an agent raising ExecutionError with kind "{kind}" (nodes_gaps)'
+)
+def step_agent_execution_error_kind(context, kind):
+    from cleveractors.core.exceptions import ExecutionError
+    from cleveractors.langgraph.nodes import GraphState, Node, NodeConfig, NodeType
+
+    mock_agent = MagicMock()
+    mock_agent.name = f"exec_err_{kind or 'empty'}_agent"
+    mock_agent.process_message = AsyncMock(
+        side_effect=ExecutionError(
+            f"Simulated {kind or 'empty'}-kind error",
+            kind=kind,
+            reason="" if not kind else "test_reason",
+        )
+    )
+    mock_agent.capabilities = ["text-generation"]
+
+    cfg = NodeConfig(
+        name=f"exec_err_{kind or 'empty'}_node",
+        type=NodeType.AGENT,
+        agent=mock_agent.name,
+    )
+    node = Node(cfg, agents={mock_agent.name: mock_agent})
+
+    state = GraphState()
+    state.messages = [{"role": "user", "content": "trigger"}]
+
+    exc_info: dict[str, object] = {}
+
+    async def _run():
+        try:
+            await node._execute_agent(state)
+            exc_info["raised"] = False
+        except ExecutionError as e:
+            exc_info["raised"] = True
+            exc_info["kind"] = e.kind
+            exc_info["reason"] = e.reason
+            exc_info["message"] = str(e)
+
+    _run_async(_run(), context=context)
+    label = kind or "empty"
+    context.results[f"exec_err_{label}_propagated"] = exc_info.get("raised", False)
+    context.results[f"exec_err_{label}_kind"] = exc_info.get("kind", "")
+
+
+@then("ExecutionError with non-empty kind should propagate to the caller (nodes_gaps)")
+def step_assert_execution_error_kind_propagates(context):
+    for key, val in context.results.items():
+        if key.startswith("exec_err_") and key.endswith("_propagated"):
+            kind_key = key.replace("_propagated", "_kind")
+            kind_val = context.results.get(kind_key, "")
+            assert val, (
+                f"ExecutionError(kind={kind_val!r}) was swallowed "
+                f"instead of propagating"
+            )
+            assert kind_val, (
+                "Propagated ExecutionError should have non-empty kind, got empty"
+            )
+
+
+@when(
+    "I exercise execute_agent with an agent raising ExecutionError with empty kind (nodes_gaps)"
+)
+def step_agent_execution_error_empty_kind(context):
+    from cleveractors.core.exceptions import ExecutionError
+    from cleveractors.langgraph.nodes import GraphState, Node, NodeConfig, NodeType
+
+    mock_agent = MagicMock()
+    mock_agent.name = "tool_err_agent"
+    mock_agent.process_message = AsyncMock(
+        side_effect=ExecutionError(
+            "Tool execution failed: Tool 'bogus' not in allowed tools list",
+            kind="",
+            reason="",
+        )
+    )
+    mock_agent.capabilities = ["text-generation"]
+
+    cfg = NodeConfig(name="tool_err_node", type=NodeType.AGENT, agent="tool_err_agent")
+    node = Node(cfg, agents={"tool_err_agent": mock_agent})
+
+    state = GraphState()
+    state.messages = [{"role": "user", "content": "trigger"}]
+
+    caught: dict[str, object] = {}
+
+    async def _run():
+        try:
+            result = await node._execute_agent(state)
+            caught["caught"] = True
+            caught["is_dict"] = isinstance(result, dict)
+            msgs = result.get("messages", [])
+            caught["has_err_msg"] = any(
+                "Error processing message" in m.get("content", "") for m in msgs
+            )
+        except Exception:
+            caught["caught"] = False
+            caught["is_dict"] = False
+            caught["has_err_msg"] = False
+
+    _run_async(_run(), context=context)
+    context.results["tool_err_caught"] = caught.get("caught", False)
+    context.results["tool_err_is_dict"] = caught.get("is_dict", False)
+    context.results["tool_err_has_msg"] = caught.get("has_err_msg", False)
+
+
+@then(
+    "the ExecutionError with empty kind should be caught and converted to error string (nodes_gaps)"
+)
+def step_assert_empty_kind_execution_error_caught(context):
+    assert context.results.get("tool_err_caught"), (
+        "ExecutionError(kind='') was not caught by _execute_agent"
+    )
+    assert context.results.get("tool_err_is_dict"), (
+        "Result should be a dict (error response)"
+    )
+    assert context.results.get("tool_err_has_msg"), (
+        "Result should contain an error message string"
+    )
+
+
+@when("I check _is_http_comms_error against APIConnectionError (nodes_gaps)")
+def step_is_http_comms_api_connection(context):
+    from cleveractors.agents.retry import _is_http_comms_error
+
+    FakeErr = type("APIConnectionError", (Exception,), {})
+    context.results["api_conn_retry"] = _is_http_comms_error(FakeErr())
+
+
+@when("I check _is_http_comms_error against APITimeoutError (nodes_gaps)")
+def step_is_http_comms_api_timeout(context):
+    from cleveractors.agents.retry import _is_http_comms_error
+
+    FakeErr = type("APITimeoutError", (Exception,), {})
+    context.results["api_timeout_retry"] = _is_http_comms_error(FakeErr())
+
+
+@then("it should return True for APIConnectionError (nodes_gaps)")
+def step_assert_api_conn_retry(context):
+    assert context.results.get("api_conn_retry") is True, (
+        "_is_http_comms_error should return True for APIConnectionError"
+    )
+
+
+@then("it should return True for APITimeoutError (nodes_gaps)")
+def step_assert_api_timeout_retry(context):
+    assert context.results.get("api_timeout_retry") is True, (
+        "_is_http_comms_error should return True for APITimeoutError"
+    )
+
+
+@when(
+    "I exercise process_message with an agent raising APIConnectionError (nodes_gaps)"
+)
+def step_process_message_api_connection(context):
+    from cleveractors.core.exceptions import ExecutionError
+    from cleveractors.langgraph.nodes import LLMAgent
+    from cleveractors.templates.renderer import TemplateRenderer
+
+    FakeAPIConnectionError = type("APIConnectionError", (Exception,), {})
+
+    with (
+        patch.object(
+            LLMAgent,
+            "_retry_ainvoke",
+            AsyncMock(side_effect=FakeAPIConnectionError("Connection error.")),
+        ),
+        patch.object(LLMAgent, "_ensure_chat_model", MagicMock()),
+    ):
+        agent = LLMAgent(
+            name="api_conn_test",
+            config={
+                "provider": "openai",
+                "model": "gpt-4o-mini",
+                "max_retries": 0,
+                "max_retry_time": -1,
+            },
+            template_renderer=TemplateRenderer(),
+        )
+        agent.name = "api_conn_test"
+
+        caught_exc: dict[str, object] = {}
+
+        async def _run():
+            try:
+                await agent.process_message("test message", {})
+            except ExecutionError as e:
+                caught_exc["raised"] = True
+                caught_exc["kind"] = e.kind
+            except Exception:
+                caught_exc["raised"] = False
+
+        _run_async(_run(), context=context)
+        context.results["pm_api_conn_raised"] = caught_exc.get("raised", False)
+        context.results["pm_api_conn_kind"] = caught_exc.get("kind", "")
+
+
+@then('the resulting ExecutionError should have kind "timeout" (nodes_gaps)')
+def step_assert_pm_api_conn_kind(context):
+    assert context.results.get("pm_api_conn_raised"), (
+        "APIConnectionError should be wrapped as ExecutionError"
+    )
+    assert context.results.get("pm_api_conn_kind") == "timeout", (
+        f"Expected kind='timeout', got '{context.results.get('pm_api_conn_kind')}'"
+    )
+
+
 @when("I exercise execute_agent with context-mutating agent (nodes_gaps)")
 def step_context_diff(context):
     from cleveractors.langgraph.nodes import GraphState, Node, NodeConfig, NodeType

src/cleveractors/agents/llm.py

@@ -1613,7 +1613,18 @@ class LLMAgent(AgentWithMemory):
                     kind=e.kind,
                     reason=e.reason,
                 ) from None
-            raise ExecutionError(f"LLM processing failed: {_err_msg}") from None
+            # Safety net: detect provider-specific connection errors that
+            # weren't retried (e.g. APIConnectionError from openai/anthropic)
+            # and propagate them as timeout limit-breaches so the node layer
+            # does not silently swallow them (issue #71).
+            _kind: str = (
+                "timeout"
+                if type(e).__name__ in {"APIConnectionError", "APITimeoutError"}
+                else ""
+            )
+            raise ExecutionError(
+                f"LLM processing failed: {_err_msg}", kind=_kind
+            ) from None
         finally:
             # Restore original temperature if it was overridden.
             # This prevents the override from leaking into subsequent calls

src/cleveractors/agents/retry.py

@@ -46,6 +46,13 @@ def _is_http_comms_error(e: Exception) -> bool:
         return e.response.status_code in {429, 502, 503, 504}
     if isinstance(e, httpx.HTTPError):
         return True
+    # Provider SDKs (openai, anthropic, etc.) wrap httpx transport errors
+    # into their own exception types (e.g. openai.APIConnectionError).
+    # These are transient connection failures that should be retried just
+    # like their underlying httpx counterparts.
+    exc_name = type(e).__name__
+    if exc_name in {"APIConnectionError", "APITimeoutError"}:
+        return True
     return False
 
 

src/cleveractors/langgraph/nodes.py

@@ -17,6 +17,7 @@ from cleveractors.agents.base import Agent
 from cleveractors.agents.llm import LLMAgent, last_token_usage_var
 from cleveractors.agents.retry import current_graph_name
 from cleveractors.agents.tool import ToolAgent
+from cleveractors.core.exceptions import ExecutionError
 from cleveractors.langgraph.state import GraphState
 from cleveractors.result import MAX_REASONABLE_TOKENS as _MAX_REASONABLE_TOKENS
 
@@ -228,8 +229,23 @@ class Node:  # pylint: disable=too-many-instance-attributes
             self.last_error = None
             return state_updates
 
+        except ExecutionError as e:
+            if e.kind:
+                # Limit-enforcement errors (depth, model_calls, tool_calls,
+                # timeout, cost) must propagate without suppression so the
+                # router can map them to the correct HTTP status code.
+                # (ADR-2029, issue #71, pure_graph.py:983).
+                raise
+            # Non-limit ExecutionError (tool errors, etc.) are still caught
+            # and converted to error state for graceful graph continuation.
+            self.last_error = e
+            self.logger.error("Node %s execution failed: %s", self.name, e)
+            return {
+                "error": str(e),
+                "failed_node": self.name,
+            }
         except Exception as e:  # pylint: disable=broad-exception-caught
-            # Intentionally catch all exceptions to handle node failures gracefully
+            # Intentionally catch non-fatal exceptions to handle node failures gracefully
             self.last_error = e
             self.logger.error("Node %s execution failed: %s", self.name, e)
 
@@ -406,8 +422,19 @@ class Node:  # pylint: disable=too-many-instance-attributes
                     "prompt_tokens": _safe_node_token_int(_last_tok[0]),
                     "completion_tokens": _safe_node_token_int(_last_tok[1]),
                 }
+        except ExecutionError as e:
+            if e.kind:
+                # Limit-enforcement errors (depth, model_calls, tool_calls,
+                # timeout, cost) must propagate without suppression so the
+                # router can map them to the correct HTTP status code.
+                # (ADR-2029, issue #71, pure_graph.py:983).
+                raise
+            # Non-limit ExecutionError (tool errors, etc.) are still caught
+            # and converted to error strings for graceful graph continuation.
+            self.logger.error("Agent %s execution failed: %s", agent.name, e)
+            agent_response = f"Error processing message: {str(e)}"
         except Exception as e:  # pylint: disable=broad-exception-caught
-            # Catch all agent exceptions to prevent node failure.
+            # Catch non-fatal agent exceptions to prevent node failure.
             # _node_token_usage remains None so the error response does not
             # claim non-zero billing data (M2 of review for issue #14).
             self.logger.error("Agent %s execution failed: %s", agent.name, e)