cleveragents/cleveractors-core

Fork 0

fix(nodes): propagate ExecutionError from retry mechanism instead of swallowing it #72

Merged

CoreRasurae merged 1 commit from fix/nodes-executionerror-propagation into master

2026-07-07 16:06:09 +00:00

CoreRasurae commented

2026-07-06 17:55:00 +00:00

Member

Summary

Fixes a bug where ExecutionError(kind="timeout") raised by call_with_retry() when all LLM communication retries are exhausted was systematically swallowed by broad except Exception handlers in Node._execute_agent() and Node.execute(). The timeout was converted into an error string in graph state or — in the streaming path — an empty response, making the retry mechanism ineffective.

Changes

src/cleveractors/langgraph/nodes.py: Added targeted except ExecutionError as e handlers that check e.kind == "timeout" and re-raise. Non-timeout ExecutionError instances (tool errors, etc.) continue to be caught gracefully, preserving existing behavior for non-retry errors.
BDD tests (features/nodes_coverage_gaps.feature): Two new scenarios verify timeout propagation and non-timeout graceful handling.
CHANGELOG.md: Added entry documenting the fix.

Closes #71
ADR-2032: LLM Agent Communication Retry Mechanisms (defines the retry contract that was being violated)

Verification

✅ Lint, format, typecheck, security, deadcode: all pass
✅ 2742 BDD scenarios pass
✅ 319 Robot integration tests pass
✅ Coverage: 96.9% (threshold: 96.5%)

## Summary Fixes a bug where `ExecutionError(kind="timeout")` raised by `call_with_retry()` when all LLM communication retries are exhausted was systematically swallowed by broad `except Exception` handlers in `Node._execute_agent()` and `Node.execute()`. The timeout was converted into an error string in graph state or — in the streaming path — an empty response, making the retry mechanism ineffective. ## Changes - **`src/cleveractors/langgraph/nodes.py`**: Added targeted `except ExecutionError as e` handlers that check `e.kind == "timeout"` and re-raise. Non-timeout `ExecutionError` instances (tool errors, etc.) continue to be caught gracefully, preserving existing behavior for non-retry errors. - **BDD tests** (`features/nodes_coverage_gaps.feature`): Two new scenarios verify timeout propagation and non-timeout graceful handling. - **CHANGELOG.md**: Added entry documenting the fix. ## Related - Closes #71 - ADR-2032: LLM Agent Communication Retry Mechanisms (defines the retry contract that was being violated) ## Verification - ✅ Lint, format, typecheck, security, deadcode: all pass - ✅ 2742 BDD scenarios pass - ✅ 319 Robot integration tests pass - ✅ Coverage: 96.9% (threshold: 96.5%)

CoreRasurae added 1 commit

2026-07-06 17:55:00 +00:00

fix(nodes): propagate ExecutionError from retry mechanism instead of swallowing it

CI / typecheck (pull_request) Has been cancelled

Details

CI / build (pull_request) Has been cancelled

Details

CI / quality (pull_request) Has been cancelled

Details

CI / lint (pull_request) Has been cancelled

Details

CI / security (pull_request) Has been cancelled

Details

CI / integration_tests (pull_request) Has been cancelled

Details

CI / coverage (pull_request) Has been cancelled

Details

CI / unit_tests (pull_request) Has been cancelled

Details

CI / status-check (pull_request) Has been cancelled

Details

bcc3b5e459

When all LLM communication retries are exhausted, call_with_retry() raises
ExecutionError(kind=\"timeout\"). Two broad except Exception handlers in
Node._execute_agent() and Node.execute() systematically caught this,
converting the timeout into an error string in graph state or — in the
streaming path — an empty response.

Added targeted except ExecutionError as e handlers that check
e.kind == \"timeout\" and re-raise. Non-timeout ExecutionError instances
(tool errors, etc.) continue to be caught and converted to error strings
for graceful graph continuation.

Two new BDD scenarios verify:
- ExecutionError(kind=\"timeout\") propagates with correct kind/reason
- Non-timeout ExecutionError is still caught gracefully

ISSUES CLOSED: #71

CoreRasurae added a new dependency

2026-07-06 17:55:51 +00:00

#71 ConnectionError retry exhaustion silently swallowed, producing empty response instead of propagating ExecutionError

CoreRasurae added the

Type

Bug

label

2026-07-06 17:57:33 +00:00

hurui200320 approved these changes

2026-07-07 05:07:06 +00:00

hurui200320 left a comment

PR Review: !72 (Ticket #71)

Verdict: Approve

The fix correctly addresses the reported bug. ExecutionError(kind="timeout") raised when the LLM retry mechanism exhausts its budget is now propagated from both Node._execute_agent() and Node.execute() instead of being swallowed by broad except Exception handlers. The implementation is narrower than the ticket's initial "propagate all ExecutionError" sketch, but that narrower scope is intentional and safer: it preserves the existing graceful handling of non-timeout execution errors (e.g., tool parsing failures) while ensuring retry-exhaustion timeouts surface to the caller as required by ADR-2032.

No critical or major correctness issues were found. I also checked the PR and ticket comment threads; there are no existing reviewer comments or author responses that would remove items from scope.

Critical Issues

None.

Major Issues

None.

Minor Issues

None.

Nits

None.

Summary

The PR adds a targeted except ExecutionError as e clause in front of the existing broad except Exception handlers in Node.execute() and Node._execute_agent(). When e.kind == "timeout", the exception is re-raised so it can propagate through PureLangGraph._execute_from_node() (whose existing handler at pure_graph.py:983 already expects limit-enforcement errors to surface) and up to the router with the correct HTTP mapping. Non-timeout ExecutionError instances continue down the original graceful path and are converted to error strings in graph state.

The logic is sound:

The retry mechanism in call_with_retry() raises ExecutionError(kind="timeout", reason="...") per ADR-2032 D-7.
LLMAgent.process_message() already preserves kind/reason when re-raising an inner ExecutionError, so the timeout identity survives the agent layer.
The new handlers sit in the correct exception-precedence order (ExecutionError before Exception) and the current_graph_name ContextVar is still reset in the finally block on the timeout path.
The BDD scenarios added verify both the propagation path and the preservation of graceful handling for non-timeout errors.

The only behavior change worth being aware of is that non-timeout ExecutionError no longer reaches the node-level retry_policy loop in Node.execute(). In practice this is correct behavior—execution-limit/failure errors should not be retried like transient exceptions—but it is a subtle narrowing of the retry_policy contract. No current node type appears to rely on the old behavior. Overall, the change is minimal, focused, and fixes the reported regression without introducing new risks.

## PR Review: !72 (Ticket #71) ### Verdict: Approve The fix correctly addresses the reported bug. `ExecutionError(kind="timeout")` raised when the LLM retry mechanism exhausts its budget is now propagated from both `Node._execute_agent()` and `Node.execute()` instead of being swallowed by broad `except Exception` handlers. The implementation is narrower than the ticket's initial "propagate all `ExecutionError`" sketch, but that narrower scope is intentional and safer: it preserves the existing graceful handling of non-timeout execution errors (e.g., tool parsing failures) while ensuring retry-exhaustion timeouts surface to the caller as required by ADR-2032. No critical or major correctness issues were found. I also checked the PR and ticket comment threads; there are no existing reviewer comments or author responses that would remove items from scope. ### Critical Issues None. ### Major Issues None. ### Minor Issues None. ### Nits None. ### Summary The PR adds a targeted `except ExecutionError as e` clause in front of the existing broad `except Exception` handlers in `Node.execute()` and `Node._execute_agent()`. When `e.kind == "timeout"`, the exception is re-raised so it can propagate through `PureLangGraph._execute_from_node()` (whose existing handler at `pure_graph.py:983` already expects limit-enforcement errors to surface) and up to the router with the correct HTTP mapping. Non-timeout `ExecutionError` instances continue down the original graceful path and are converted to error strings in graph state. The logic is sound: - The retry mechanism in `call_with_retry()` raises `ExecutionError(kind="timeout", reason="...")` per ADR-2032 D-7. - `LLMAgent.process_message()` already preserves `kind`/`reason` when re-raising an inner `ExecutionError`, so the timeout identity survives the agent layer. - The new handlers sit in the correct exception-precedence order (`ExecutionError` before `Exception`) and the `current_graph_name` ContextVar is still reset in the `finally` block on the timeout path. - The BDD scenarios added verify both the propagation path and the preservation of graceful handling for non-timeout errors. The only behavior change worth being aware of is that non-timeout `ExecutionError` no longer reaches the node-level `retry_policy` loop in `Node.execute()`. In practice this is correct behavior—execution-limit/failure errors should not be retried like transient exceptions—but it is a subtle narrowing of the `retry_policy` contract. No current node type appears to rely on the old behavior. Overall, the change is minimal, focused, and fixes the reported regression without introducing new risks.

CoreRasurae force-pushed fix/nodes-executionerror-propagation from bcc3b5e459

CI / typecheck (pull_request) Has been cancelled

Details

CI / build (pull_request) Has been cancelled

Details

CI / quality (pull_request) Has been cancelled

Details

CI / lint (pull_request) Has been cancelled

Details

CI / security (pull_request) Has been cancelled

Details

CI / integration_tests (pull_request) Has been cancelled

Details

CI / coverage (pull_request) Has been cancelled

Details

CI / unit_tests (pull_request) Has been cancelled

Details

CI / status-check (pull_request) Has been cancelled

Details

to b29ee0996e

CI / typecheck (pull_request) Has been cancelled

Details

CI / coverage (pull_request) Has been cancelled

Details

CI / quality (pull_request) Has been cancelled

Details

CI / security (pull_request) Has been cancelled

Details

CI / build (pull_request) Has been cancelled

Details

CI / lint (pull_request) Has been cancelled

Details

CI / unit_tests (pull_request) Has been cancelled

Details

CI / integration_tests (pull_request) Has been cancelled

Details

CI / status-check (pull_request) Has been cancelled

Details

2026-07-07 09:55:04 +00:00

Compare

CoreRasurae added 1 commit

2026-07-07 12:02:54 +00:00

fix(retry,llm): handle provider APIConnectionError as retryable transient failure

CI / status-check (pull_request) Has been cancelled

Details

CI / build (pull_request) Has been cancelled

Details

CI / typecheck (pull_request) Has been cancelled

Details

CI / integration_tests (pull_request) Has been cancelled

Details

CI / lint (pull_request) Has been cancelled

Details

CI / quality (pull_request) Has been cancelled

Details

CI / unit_tests (pull_request) Has been cancelled

Details

CI / coverage (pull_request) Has been cancelled

Details

CI / security (pull_request) Has been cancelled

Details

d0f18ec2e7

Extend _is_http_comms_error() in retry.py to recognize provider SDK
connection errors (APIConnectionError, APITimeoutError) by class name,
so they are retried with exponential backoff instead of propagating
immediately.

Add safety net in process_message() to wrap non-retried connection
errors as ExecutionError(kind='timeout') for correct propagation
through the node layer (issue #71).

Also apply non-blocking review suggestion: change the guard in
_execute_agent() and execute() from 'e.kind == "timeout"' to
'if e.kind' — matching the ExecutionError contract where non-empty
kind means limit breach (depth, model_calls, tool_calls, timeout,
cost).

CoreRasurae commented

2026-07-07 12:03:05 +00:00

Author

Member

Updated with two additional commits:

d0f18ec — Applied the non-blocking review suggestion: changed e.kind == "timeout" to if e.kind in both _execute_agent() and execute() to future-proof against any limit enforcement moving into the agent/node layer (depth, model_calls, tool_calls, cost).
d0f18ec (same commit) — Extended _is_http_comms_error() in retry.py to detect provider SDK connection errors (APIConnectionError, APITimeoutError) by class name. Added safety net in process_message() to wrap non-retried connection errors as ExecutionError(kind="timeout"). The user log output (APIConnectionError: Connection error.) showed that the original fix did not handle these because provider SDK exception types are not subclasses of httpx.HTTPError.

**Updated with two additional commits:** 1. `d0f18ec` — Applied the non-blocking review suggestion: changed `e.kind == "timeout"` to `if e.kind` in both `_execute_agent()` and `execute()` to future-proof against any limit enforcement moving into the agent/node layer (depth, model_calls, tool_calls, cost). 2. `d0f18ec` (same commit) — Extended `_is_http_comms_error()` in retry.py to detect provider SDK connection errors (`APIConnectionError`, `APITimeoutError`) by class name. Added safety net in `process_message()` to wrap non-retried connection errors as `ExecutionError(kind="timeout")`. The user log output (`APIConnectionError: Connection error.`) showed that the original fix did not handle these because provider SDK exception types are not subclasses of `httpx.HTTPError`.

CoreRasurae force-pushed fix/nodes-executionerror-propagation from d0f18ec2e7

CI / status-check (pull_request) Has been cancelled

Details

CI / build (pull_request) Has been cancelled

Details

CI / typecheck (pull_request) Has been cancelled

Details

CI / integration_tests (pull_request) Has been cancelled

Details

CI / lint (pull_request) Has been cancelled

Details

CI / quality (pull_request) Has been cancelled