Invert default exception handling: stop swallowing by default; catch only recoverable exceptions and propagate the rest #78

New issue

Open

opened 2026-07-07 11:45:02 +00:00 by Graa · 0 comments

Graa commented

2026-07-07 11:45:02 +00:00

Description

The library's default exception-handling posture is inverted. Across the runtime, the prevailing pattern is a broad except Exception that catches everything and neutralizes it — logging and continuing, converting the error into a string that is injected into graph state as if it were a valid response, or returning an {"error": ...} dict that a downstream caller silently discards. Swallowing is the default; propagation is the exception.

This is backwards. The correct posture is the opposite:

Never swallow by default. Only the specific exception cases that can actually be handled/recovered should be caught. Everything else must propagate to the caller.

Catching should be an explicit, whitelisted decision per recovery path — not a blanket catch-all that masks bugs, network failures, budget breaches, and programming errors alike.

Current behavior (evidence)

There are ~101 broad except Exception / bare-except sites in src/. The problematic ones don't just log-and-reraise — they catch-all and neutralize. Representative swallow-and-continue sites:

langgraph/nodes.py:409 (Node._execute_agent) — catches all exceptions and does agent_response = f"Error processing message: {str(e)}", injecting the error text into graph state as a legitimate agent response.
langgraph/nodes.py:231 (Node.execute) — catch-all returns {"error": str(e), "failed_node": self.name}, which _execute_from_node then silently drops (falls back to the original input message).
langgraph/pure_graph.py:988, and further broad catches at 531, 1265, 1273, 1608, 1899, 2058, 2374, 2380.
langgraph/bridge.py — 235, 256, 389, 410, 516, 648.
agents/tool.py — 244, 493, 537, 577, 896, 907; agents/llm.py — 714, 1308, 1466, 1587, 1748; agents/base.py:88, agents/factory.py:294, agents/llm_client.py:102.

The codebase already acknowledges this as an anti-pattern — the module docstring of _safe_node_token_int in langgraph/nodes.py explicitly notes that an escaping error would "be caught by Node.execute()'s broad except Exception, and silently discard the LLM response." The defensive helper exists precisely to route around the catch-all, which is a symptom of the inverted default rather than a fix for it.

Root cause

The default is "catch everything, keep going." A genuine error (a bug, an unreachable provider, a budget breach, a malformed config) is indistinguishable at the catch site from a recoverable, expected condition, so all of them are absorbed. Failures become invisible: the graph proceeds with the original input or an error-string masquerading as output.

Proposed fix (policy)

Invert the default to an allowlist model:

Catch narrowly, by type. Replace catch-all handlers with handlers that name the specific, expected, recoverable exception type(s) for that path. A well-defined taxonomy already exists in core/exceptions.py (CleverAgentsException → ConfigurationError, ExecutionError(kind, reason), RoutingError, TemplateError, ApplicationError, MissingUsageMetadataError, StreamRoutingError, …), so narrowing is feasible today with no new infrastructure.
Propagate everything else. Anything not on a path's allowlist must propagate unchanged to the caller. No error should ever be converted into normal response content or a silently-consumed dict.
Where a broad catch is genuinely required (e.g. an outermost boundary that must not crash a host process), it must (a) be justified in a comment, (b) log at ERROR with full context/traceback, and (c) re-raise or surface a typed error — never translate the failure into a value the pipeline treats as success.
Add a lint guard so the inverted default cannot silently return. The repo already sprinkles # pylint: disable=broad-exception-caught at many sites; flip the posture by keeping broad-exception-caught enabled and requiring per-site justification, so new catch-alls are caught in review.

Definition of Done

Every broad except Exception / bare-except in src/ is either (a) narrowed to a specific recoverable type, or (b) retained at a justified boundary that logs and re-raises/surfaces a typed error — none convert an exception into success-shaped output.
No code path injects an error string or an {"error": ...} dict into graph state / response content in place of propagating a typed error.
broad-exception-caught is enforced by lint; remaining broad catches carry an inline justification.
Regression tests: a raised, non-recoverable exception (e.g. a programming error, an unreachable provider) propagates out of graph/agent/tool execution rather than being masked — for both the non-streaming and streaming paths.

Relationship to other issues

Parent Epic: #77 (LLM Agent Runtime Stabilization) — this is the systemic/policy umbrella for the "proper error propagation" theme.
#71 (ConnectionError retry exhaustion silently swallowed) — a specific instance of this inverted default; its three-layer swallow analysis (nodes.py:409 → nodes.py:231 → pure_graph.py:988) is exactly the pattern this ticket generalizes. #71's fix should conform to the policy defined here.
#39 (closed — fix(runtime): … proper error propagation) — fixed specific runtime.py/_execute_llm sites (wrong error type, no cleanup). This ticket extends the same principle across the whole runtime as a default posture rather than one-off fixes.

Non-conflict with specs / ADRs

This does not contradict any spec or ADR:

No ADR mandates catch-all swallowing. ADR-2032 (LLM agent retry) expects ExecutionError(kind="timeout") to propagate once retries are exhausted — the current swallow behavior actively violates its intent.
The Actor Configuration Standard / Package Registry work (Legendary #9, Epic #22) concern configuration and registry semantics, not runtime error handling.
The existing core/exceptions.py taxonomy is the intended vehicle for typed propagation; this ticket makes the runtime use it as designed.

Proposed Branch

fix/error-handling-allowlist-no-default-swallow

Proposed Commit Title

fix(runtime): stop swallowing exceptions by default; catch only recoverable types and propagate the rest

## Description The library's **default exception-handling posture is inverted**. Across the runtime, the prevailing pattern is a broad `except Exception` that catches *everything* and neutralizes it — logging and continuing, converting the error into a string that is injected into graph state as if it were a valid response, or returning an `{"error": ...}` dict that a downstream caller silently discards. Swallowing is the **default**; propagation is the exception. This is backwards. The correct posture is the opposite: > **Never swallow by default. Only the specific exception cases that can actually be handled/recovered should be caught. Everything else must propagate to the caller.** Catching should be an explicit, whitelisted decision per recovery path — not a blanket catch-all that masks bugs, network failures, budget breaches, and programming errors alike. ## Current behavior (evidence) There are **~101 broad `except Exception` / bare-`except` sites** in `src/`. The problematic ones don't just log-and-reraise — they catch-all and neutralize. Representative swallow-and-continue sites: - **`langgraph/nodes.py:409`** (`Node._execute_agent`) — catches **all** exceptions and does `agent_response = f"Error processing message: {str(e)}"`, injecting the error text into graph state as a legitimate agent response. - **`langgraph/nodes.py:231`** (`Node.execute`) — catch-all returns `{"error": str(e), "failed_node": self.name}`, which `_execute_from_node` then silently drops (falls back to the original input message). - **`langgraph/pure_graph.py:988`**, and further broad catches at `531`, `1265`, `1273`, `1608`, `1899`, `2058`, `2374`, `2380`. - **`langgraph/bridge.py`** — `235`, `256`, `389`, `410`, `516`, `648`. - **`agents/tool.py`** — `244`, `493`, `537`, `577`, `896`, `907`; **`agents/llm.py`** — `714`, `1308`, `1466`, `1587`, `1748`; **`agents/base.py:88`**, **`agents/factory.py:294`**, **`agents/llm_client.py:102`**. The codebase already **acknowledges this as an anti-pattern** — the module docstring of `_safe_node_token_int` in `langgraph/nodes.py` explicitly notes that an escaping error would "be caught by `Node.execute()`'s broad `except Exception`, and silently discard the LLM response." The defensive helper exists precisely to route *around* the catch-all, which is a symptom of the inverted default rather than a fix for it. ## Root cause The default is "catch everything, keep going." A genuine error (a bug, an unreachable provider, a budget breach, a malformed config) is indistinguishable at the catch site from a recoverable, expected condition, so all of them are absorbed. Failures become invisible: the graph proceeds with the original input or an error-string masquerading as output. ## Proposed fix (policy) Invert the default to an **allowlist** model: 1. **Catch narrowly, by type.** Replace catch-all handlers with handlers that name the specific, expected, recoverable exception type(s) for that path. A well-defined taxonomy already exists in `core/exceptions.py` (`CleverAgentsException` → `ConfigurationError`, `ExecutionError(kind, reason)`, `RoutingError`, `TemplateError`, `ApplicationError`, `MissingUsageMetadataError`, `StreamRoutingError`, …), so narrowing is feasible today with no new infrastructure. 2. **Propagate everything else.** Anything not on a path's allowlist must propagate unchanged to the caller. No error should ever be converted into normal response content or a silently-consumed dict. 3. **Where a broad catch is genuinely required** (e.g. an outermost boundary that must not crash a host process), it must (a) be justified in a comment, (b) log at ERROR with full context/traceback, and (c) **re-raise or surface a typed error** — never translate the failure into a value the pipeline treats as success. 4. **Add a lint guard** so the inverted default cannot silently return. The repo already sprinkles `# pylint: disable=broad-exception-caught` at many sites; flip the posture by keeping `broad-exception-caught` enabled and requiring per-site justification, so new catch-alls are caught in review. ## Definition of Done - [ ] Every broad `except Exception` / bare-`except` in `src/` is either (a) narrowed to a specific recoverable type, or (b) retained at a justified boundary that logs and re-raises/surfaces a typed error — none convert an exception into success-shaped output. - [ ] No code path injects an error string or an `{"error": ...}` dict into graph state / response content in place of propagating a typed error. - [ ] `broad-exception-caught` is enforced by lint; remaining broad catches carry an inline justification. - [ ] Regression tests: a raised, non-recoverable exception (e.g. a programming error, an unreachable provider) propagates out of graph/agent/tool execution rather than being masked — for both the non-streaming and streaming paths. ## Relationship to other issues - **Parent Epic: #77** (LLM Agent Runtime Stabilization) — this is the systemic/policy umbrella for the "proper error propagation" theme. - **#71** (ConnectionError retry exhaustion silently swallowed) — a *specific instance* of this inverted default; its three-layer swallow analysis (`nodes.py:409` → `nodes.py:231` → `pure_graph.py:988`) is exactly the pattern this ticket generalizes. #71's fix should conform to the policy defined here. - **#39** (closed — `fix(runtime): … proper error propagation`) — fixed specific `runtime.py`/`_execute_llm` sites (wrong error type, no cleanup). This ticket extends the same principle across the whole runtime as a default posture rather than one-off fixes. ## Non-conflict with specs / ADRs This does not contradict any spec or ADR: - No ADR mandates catch-all swallowing. **ADR-2032** (LLM agent retry) *expects* `ExecutionError(kind="timeout")` to propagate once retries are exhausted — the current swallow behavior actively violates its intent. - The Actor Configuration Standard / Package Registry work (Legendary #9, Epic #22) concern configuration and registry semantics, not runtime error handling. - The existing `core/exceptions.py` taxonomy is the intended vehicle for typed propagation; this ticket makes the runtime use it as designed. ## Proposed Branch `fix/error-handling-allowlist-no-default-swallow` ## Proposed Commit Title ``` fix(runtime): stop swallowing exceptions by default; catch only recoverable types and propagate the rest ```

Graa added the

labels

2026-07-07 11:45:02 +00:00