BUG-HUNT: [error-handling] Broad exception handling in LangChainChatProvider #2884

New issue

Open

opened 2026-04-05 02:40:05 +00:00 by freemo · 1 comment

freemo commented

2026-04-05 02:40:05 +00:00

Owner

Metadata

Branch: fix/error-handling-langchain-chat-provider-stream-changes
Commit Message: fix(providers): replace broad Exception catch in LangChainChatProvider.stream_changes with specific exception types
Milestone: v3.7.0
Parent Epic: #1669

Background and Context

The stream_changes method (and the private _execute_with_streaming helper) in LangChainChatProvider use a broad except Exception: block. While these handlers do re-raise the exception (unlike the generate_changes handler tracked in #1761), they still intercept a wide class of exceptions — including runtime errors such as MemoryError, RecursionError, and unexpected third-party library errors — before calling the progress callback and re-raising. This violates the project's fail-fast and error-propagation principles (CONTRIBUTING.md) and makes it harder to reason about which exceptions are expected vs. truly unexpected.

Note

: The generate_changes broad exception handler is tracked separately in #1761. This issue focuses on stream_changes (line ~231) and _execute_with_streaming (line ~272).

Current Behavior

stream_changes (line ~231) and _execute_with_streaming (line ~272) in src/cleveragents/providers/llm/langchain_chat_provider.py use a broad except Exception: catch-all:

# stream_changes — _stream() inner function (~line 231)
            except Exception:
                if progress_callback:
                    progress_callback(100)
                raise

# _execute_with_streaming (~line 272)
        except Exception:
            progress_callback(100)
            raise

Although the exception is re-raised, the broad except Exception intercepts any exception — including unexpected runtime errors (MemoryError, RecursionError) and programming errors — before the progress callback is called. This can mask the true origin of failures and makes it harder to distinguish expected LangChain/network errors from unexpected system-level errors.

Expected Behavior

Only exceptions that are expected and can be meaningfully handled (e.g., LangChain-specific errors, network timeouts, graph execution errors) should be caught.
Unexpected runtime errors (MemoryError, RecursionError, etc.) should propagate immediately without interception.
If a broad catch is truly necessary as a last resort, it must log the full traceback via logger.exception(...) before re-raising.
The progress callback cleanup logic should be moved to a finally block to ensure it always runs regardless of exception type, eliminating the need for a broad catch entirely.

Suggested Fix

Replace the broad except Exception: with a finally: block for the progress callback cleanup, and catch only specific expected exception types:

# Preferred approach — use finally for cleanup
try:
    with self._usage_tracker(llm) as tracker:
        ...
finally:
    if progress_callback:
        progress_callback(100)

Or, if specific exception catching is needed:

except (LangChainError, NetworkError, TimeoutError) as exc:
    if progress_callback:
        progress_callback(100)
    raise

Evidence

File: src/cleveragents/providers/llm/langchain_chat_provider.py
Class: LangChainChatProvider
Locations:
- stream_changes → inner _stream() function, line ~231
- _execute_with_streaming, line ~272
Severity: Medium — the exception is re-raised so data is not silently lost, but unexpected errors are intercepted before propagating cleanly
Related: #1761 (same class, generate_changes method — swallows exception entirely, higher severity)

Subtasks

Identify all specific exception types that graph.stream(...) and self._usage_tracker(llm) can raise (LangChain exceptions, network errors, timeouts, graph execution errors)
Refactor _stream() in stream_changes: move progress callback to a finally block and remove the broad except Exception: entirely, or replace with specific exception types
Refactor _execute_with_streaming: apply the same finally-block pattern or specific exception types
Add logger.exception(...) logging if a catch-all is retained as a last resort
Tests (Behave): Add BDD scenarios covering the exception path in stream_changes and _execute_with_streaming, verifying that the progress callback is always called and that unexpected exceptions propagate cleanly
Tests (Robot): Add integration test for the error path in LangChainChatProvider.stream_changes
Verify coverage ≥ 97% via nox -s coverage_report
Run nox (all default sessions) and fix any errors

Definition of Done

All subtasks above are completed and checked off
The broad except Exception: in stream_changes._stream() and _execute_with_streaming is replaced with specific exception types or a finally block
No unexpected exceptions are silently intercepted — all non-LangChain/network errors propagate immediately
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional detail
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/error-handling-langchain-chat-provider-stream-changes` - **Commit Message**: `fix(providers): replace broad Exception catch in LangChainChatProvider.stream_changes with specific exception types` - **Milestone**: v3.7.0 - **Parent Epic**: #1669 ## Background and Context The `stream_changes` method (and the private `_execute_with_streaming` helper) in `LangChainChatProvider` use a broad `except Exception:` block. While these handlers do re-raise the exception (unlike the `generate_changes` handler tracked in #1761), they still intercept a wide class of exceptions — including runtime errors such as `MemoryError`, `RecursionError`, and unexpected third-party library errors — before calling the progress callback and re-raising. This violates the project's fail-fast and error-propagation principles (CONTRIBUTING.md) and makes it harder to reason about which exceptions are expected vs. truly unexpected. > **Note**: The `generate_changes` broad exception handler is tracked separately in #1761. This issue focuses on `stream_changes` (line ~231) and `_execute_with_streaming` (line ~272). ## Current Behavior `stream_changes` (line ~231) and `_execute_with_streaming` (line ~272) in `src/cleveragents/providers/llm/langchain_chat_provider.py` use a broad `except Exception:` catch-all: ```python # stream_changes — _stream() inner function (~line 231) except Exception: if progress_callback: progress_callback(100) raise # _execute_with_streaming (~line 272) except Exception: progress_callback(100) raise ``` Although the exception is re-raised, the broad `except Exception` intercepts any exception — including unexpected runtime errors (`MemoryError`, `RecursionError`) and programming errors — before the progress callback is called. This can mask the true origin of failures and makes it harder to distinguish expected LangChain/network errors from unexpected system-level errors. ## Expected Behavior - Only exceptions that are expected and can be meaningfully handled (e.g., LangChain-specific errors, network timeouts, graph execution errors) should be caught. - Unexpected runtime errors (`MemoryError`, `RecursionError`, etc.) should propagate immediately without interception. - If a broad catch is truly necessary as a last resort, it must log the full traceback via `logger.exception(...)` before re-raising. - The progress callback cleanup logic should be moved to a `finally` block to ensure it always runs regardless of exception type, eliminating the need for a broad catch entirely. ## Suggested Fix Replace the broad `except Exception:` with a `finally:` block for the progress callback cleanup, and catch only specific expected exception types: ```python # Preferred approach — use finally for cleanup try: with self._usage_tracker(llm) as tracker: ... finally: if progress_callback: progress_callback(100) ``` Or, if specific exception catching is needed: ```python except (LangChainError, NetworkError, TimeoutError) as exc: if progress_callback: progress_callback(100) raise ``` ## Evidence - **File**: `src/cleveragents/providers/llm/langchain_chat_provider.py` - **Class**: `LangChainChatProvider` - **Locations**: - `stream_changes` → inner `_stream()` function, line ~231 - `_execute_with_streaming`, line ~272 - **Severity**: Medium — the exception is re-raised so data is not silently lost, but unexpected errors are intercepted before propagating cleanly - **Related**: #1761 (same class, `generate_changes` method — swallows exception entirely, higher severity) ## Subtasks - [ ] Identify all specific exception types that `graph.stream(...)` and `self._usage_tracker(llm)` can raise (LangChain exceptions, network errors, timeouts, graph execution errors) - [ ] Refactor `_stream()` in `stream_changes`: move progress callback to a `finally` block and remove the broad `except Exception:` entirely, or replace with specific exception types - [ ] Refactor `_execute_with_streaming`: apply the same `finally`-block pattern or specific exception types - [ ] Add `logger.exception(...)` logging if a catch-all is retained as a last resort - [ ] Tests (Behave): Add BDD scenarios covering the exception path in `stream_changes` and `_execute_with_streaming`, verifying that the progress callback is always called and that unexpected exceptions propagate cleanly - [ ] Tests (Robot): Add integration test for the error path in `LangChainChatProvider.stream_changes` - [ ] Verify coverage ≥ 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions) and fix any errors ## Definition of Done - [ ] All subtasks above are completed and checked off - [ ] The broad `except Exception:` in `stream_changes._stream()` and `_execute_with_streaming` is replaced with specific exception types or a `finally` block - [ ] No unexpected exceptions are silently intercepted — all non-LangChain/network errors propagate immediately - [ ] A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional detail - [ ] The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly - [ ] The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: ca-new-issue-creator

freemo added this to the v3.7.0 milestone

2026-04-05 02:40:15 +00:00

freemo added a new dependency

2026-04-05 02:40:56 +00:00

#1669 Bug Hunting Session

freemo commented

2026-04-05 02:45:01 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Medium (confirmed) — Broad exception handling in LangChainChatProvider is a code quality issue. Per CONTRIBUTING.md, exceptions should only be caught when they can be meaningfully handled.
MoSCoW: Should Have — Error handling quality is important for debugging and reliability.

Valid bug-hunt finding.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Medium (confirmed) — Broad exception handling in LangChainChatProvider is a code quality issue. Per CONTRIBUTING.md, exceptions should only be caught when they can be meaningfully handled. - **MoSCoW**: Should Have — Error handling quality is important for debugging and reliability. Valid bug-hunt finding. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner