From 7c2468f36b624db7c4a79fd6c84d27d8afaa8c67 Mon Sep 17 00:00:00 2001
From: Armaanjeet Singh Sandhu
 <90208595+ArmaanjeetSandhu@users.noreply.github.com>
Date: Sat, 5 Apr 2025 01:15:15 +0530
Subject: [PATCH] core: Fix handler removal in BaseCallbackManager (Fixes
 #30640) (#30659)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

**Description:**
Fixed a bug in `BaseCallbackManager.remove_handler()` that caused a
`ValueError` when removing a handler added via the constructor's
`handlers` parameter. The issue occurred because handlers passed to the
constructor were added only to the `handlers` list and not automatically
to `inheritable_handlers` unless explicitly specified. However,
`remove_handler()` attempted to remove the handler from both lists
unconditionally, triggering a `ValueError` when it wasn't in
`inheritable_handlers`.

The fix ensures the method checks for the handler’s presence in each
list before attempting removal, making it more robust while preserving
its original behavior.

**Issue:** Fixes #30640

**Dependencies:** None

---------

Co-authored-by: Eugene Yurtsev <eyurtsev@gmail.com>
---
 libs/core/langchain_core/callbacks/base.py       |  6 ++++--
 .../callbacks/test_sync_callback_manager.py      | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100644 libs/core/tests/unit_tests/callbacks/test_sync_callback_manager.py

diff --git a/libs/core/langchain_core/callbacks/base.py b/libs/core/langchain_core/callbacks/base.py
index 83743723071..34a9120c96c 100644
--- a/libs/core/langchain_core/callbacks/base.py
+++ b/libs/core/langchain_core/callbacks/base.py
@@ -1007,8 +1007,10 @@ class BaseCallbackManager(CallbackManagerMixin):
         Args:
             handler (BaseCallbackHandler): The handler to remove.
         """
-        self.handlers.remove(handler)
-        self.inheritable_handlers.remove(handler)
+        if handler in self.handlers:
+            self.handlers.remove(handler)
+        if handler in self.inheritable_handlers:
+            self.inheritable_handlers.remove(handler)
 
     def set_handlers(
         self, handlers: list[BaseCallbackHandler], inherit: bool = True
diff --git a/libs/core/tests/unit_tests/callbacks/test_sync_callback_manager.py b/libs/core/tests/unit_tests/callbacks/test_sync_callback_manager.py
new file mode 100644
index 00000000000..617d0ac5ee0
--- /dev/null
+++ b/libs/core/tests/unit_tests/callbacks/test_sync_callback_manager.py
@@ -0,0 +1,16 @@
+from langchain_core.callbacks.base import BaseCallbackHandler
+from langchain_core.callbacks.manager import BaseCallbackManager
+
+
+def test_remove_handler() -> None:
+    """Test removing handler does not raise an error on removal.
+
+    An handler can be inheritable or not. This test checks that
+    removing a handler does not raise an error if the handler
+    is not inheritable.
+    """
+    handler1 = BaseCallbackHandler()
+    handler2 = BaseCallbackHandler()
+    manager = BaseCallbackManager([handler1], inheritable_handlers=[handler2])
+    manager.remove_handler(handler1)
+    manager.remove_handler(handler2)