fix(langchain): filter GraphBubbleUp control-flow exceptions in _handle_error - #6495
Closed
wahajahmed010 wants to merge 1 commit into
Closed
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
…le_error LangGraph raises GraphBubbleUp/GraphInterrupt for normal control flow (human-in-the-loop interrupts via langgraph.types.interrupt()). Sentry's SentryLangchainCallback was capturing these as errors via on_tool_error, creating false-positive issues. Add a lazy module-level import of GraphBubbleUp from langgraph.errors, and skip _handle_error for isinstance checks against this type. Fixes: getsentry#6384 Co-authored-by: AI assistant (OpenClaw Buck)
Comment thread
sentry_sdk/integrations/langchain.py
Comment on lines
+282
to
+283
| if isinstance(error, _graph_bubble_up): | ||
| return |
Copy link
Copy Markdown
There was a problem hiding this comment.
Bug: When handling GraphBubbleUp exceptions, the code returns early without cleaning up the associated span from self.span_map, causing a memory leak.
Severity: MEDIUM
When a _graph_bubble_up exception is handled, call a cleanup function like self._exit_span(span, run_id) before returning. This will ensure the span is properly closed and removed from the map without being captured as a Sentry error.
Prompt for AI AgentReview the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: sentry_sdk/integrations/langchain.py#L282-L283 Potential issue: In the Langchain integration's `_handle_error` function, when an error is an instance of `_graph_bubble_up` (a control-flow exception used in LangGraph), the function returns early. This is correct for not capturing it as a Sentry issue, but it skips the necessary cleanup logic that closes the span and removes it from `self.span_map`. In applications that use features like human-in-the-loop interrupts, this will be triggered frequently, causing the `span_map` to grow indefinitely. This results in a memory leak, as the garbage collection for the map is disabled by default.
Did we get this right? 👍 / 👎 to inform future reviews.
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Closing in favor of #6608. |
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LangGraph raises GraphBubbleUp / GraphInterrupt for normal control flow (e.g. langgraph.types.interrupt() for human-in-the-loop). Sentry's SentryLangchainCallback._handle_error was capturing these as errors via on_tool_error, creating false-positive Sentry issues.
Changes
Testing
Fixes #6384
AI assistance disclosure: this PR was co-authored by an AI assistant (OpenClaw Buck) under human supervision.