mirror of
https://github.com/langchain-ai/langgraph.git
synced 2026-09-12 12:47:53 +02:00
fix(langgraph): remove strict add_handler type check that breaks OTel instrumentation (#7544)
## Summary
Removes the `add_handler()` overrides on `_GraphCallbackManager` and
`_AsyncGraphCallbackManager` that reject handlers not inheriting from
`GraphCallbackHandler`. This fixes a regression in 1.1.7 where
`opentelemetry-instrumentation-langchain` (and likely other libraries
that patch `BaseCallbackManager.__init__`) crash with `TypeError:
handlers must inherit GraphCallbackHandler` at invocation time.
## Why this is safe
The strict type check is redundant — `_configure_graph_callbacks` and
`_filter_graph_handlers` already filter handlers to
`GraphCallbackHandler` instances at construction time. Non-graph
handlers that enter via external patches (like OTel's monkey-patch) are
harmless because `handle_event("on_interrupt", ...)` /
`handle_event("on_resume", ...)` will simply no-op on handlers that
don't implement those methods.
## What changed
- Deleted `add_handler()` override from `_GraphCallbackManager` (was
lines 248-255)
- Deleted `add_handler()` override from `_AsyncGraphCallbackManager`
(was lines 324-331)
- No other changes — 18 lines removed, 0 added
## Test plan
- [x] All 8 existing `test_graph_callbacks.py` tests pass (`make test
TEST=tests/test_graph_callbacks.py`)
- [x] `make lint` passes
- [x] `make format` passes (no changes needed)
- [x] Verified fix locally: `LangchainInstrumentor().instrument()` +
`create_react_agent()` + `graph.ainvoke()` no longer raises `TypeError`
- [x] Verified the graph lifecycle callbacks (`on_interrupt`,
`on_resume`) still work correctly
Closes #7543
---------
Co-authored-by: Eugene Yurtsev <eyurtsev@gmail.com>
This commit is contained in:
co-authored by
Eugene Yurtsev
parent
e002711ede
commit
aa94790f36
@@ -245,15 +245,6 @@ class _GraphCallbackManager(BaseCallbackManager):
|
||||
run_id=run_id,
|
||||
)
|
||||
|
||||
def add_handler(
|
||||
self,
|
||||
handler: BaseCallbackHandler,
|
||||
inherit: bool = True, # noqa: FBT001,FBT002
|
||||
) -> None:
|
||||
if not isinstance(handler, GraphCallbackHandler):
|
||||
raise TypeError("handlers must inherit GraphCallbackHandler")
|
||||
super().add_handler(handler, inherit=inherit)
|
||||
|
||||
def copy(
|
||||
self,
|
||||
*,
|
||||
@@ -321,15 +312,6 @@ class _AsyncGraphCallbackManager(BaseCallbackManager):
|
||||
run_id=run_id,
|
||||
)
|
||||
|
||||
def add_handler(
|
||||
self,
|
||||
handler: BaseCallbackHandler,
|
||||
inherit: bool = True, # noqa: FBT001,FBT002
|
||||
) -> None:
|
||||
if not isinstance(handler, GraphCallbackHandler):
|
||||
raise TypeError("handlers must inherit GraphCallbackHandler")
|
||||
super().add_handler(handler, inherit=inherit)
|
||||
|
||||
def copy(
|
||||
self,
|
||||
*,
|
||||
|
||||
@@ -275,3 +275,70 @@ def test_graph_callbacks_accept_base_callback_manager() -> None:
|
||||
|
||||
assert "__interrupt__" in first
|
||||
assert len(graph_handler.interrupt_events) == 1
|
||||
|
||||
|
||||
def test_non_graph_handler_via_add_handler_does_not_crash() -> None:
|
||||
"""Non-GraphCallbackHandler added via add_handler should not raise.
|
||||
|
||||
Libraries like opentelemetry-instrumentation-langchain monkey-patch
|
||||
BaseCallbackManager.__init__ and inject handlers via add_handler().
|
||||
These handlers inherit from BaseCallbackHandler, not
|
||||
GraphCallbackHandler. They must be silently accepted — graph lifecycle
|
||||
events will simply not be dispatched to them.
|
||||
"""
|
||||
from langgraph.callbacks import _GraphCallbackManager
|
||||
|
||||
manager = _GraphCallbackManager()
|
||||
plain_handler = _LangChainCustomEventHandler()
|
||||
|
||||
manager.add_handler(plain_handler, inherit=True)
|
||||
assert plain_handler in manager.handlers
|
||||
|
||||
|
||||
def test_non_graph_handler_does_not_receive_lifecycle_events() -> None:
|
||||
"""Non-GraphCallbackHandler added alongside a GraphCallbackHandler
|
||||
should not interfere with lifecycle event dispatch."""
|
||||
graph = _build_interrupt_graph()
|
||||
graph_handler = _GraphEventHandler()
|
||||
plain_handler = _LangChainCustomEventHandler()
|
||||
|
||||
config = {
|
||||
"configurable": {"thread_id": "graph-callback-mixed-handlers"},
|
||||
"callbacks": [plain_handler, graph_handler],
|
||||
}
|
||||
|
||||
first = graph.invoke({"answer": None}, config)
|
||||
assert "__interrupt__" in first
|
||||
|
||||
assert len(graph_handler.interrupt_events) == 1
|
||||
assert plain_handler.events == []
|
||||
|
||||
resumed = graph.invoke(Command(resume="done"), config)
|
||||
assert resumed == {"answer": "done"}
|
||||
assert len(graph_handler.resume_events) == 1
|
||||
assert plain_handler.events == []
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@NEEDS_CONTEXTVARS
|
||||
async def test_non_graph_handler_does_not_receive_lifecycle_events_async() -> None:
|
||||
"""Async variant: non-GraphCallbackHandler should not interfere."""
|
||||
graph = _build_interrupt_graph()
|
||||
graph_handler = _GraphEventHandler()
|
||||
plain_handler = _LangChainCustomEventHandler()
|
||||
|
||||
config = {
|
||||
"configurable": {"thread_id": "graph-callback-mixed-handlers-async"},
|
||||
"callbacks": [plain_handler, graph_handler],
|
||||
}
|
||||
|
||||
first = await graph.ainvoke({"answer": None}, config)
|
||||
assert "__interrupt__" in first
|
||||
|
||||
assert len(graph_handler.interrupt_events) == 1
|
||||
assert plain_handler.events == []
|
||||
|
||||
resumed = await graph.ainvoke(Command(resume="done"), config)
|
||||
assert resumed == {"answer": "done"}
|
||||
assert len(graph_handler.resume_events) == 1
|
||||
assert plain_handler.events == []
|
||||
|
||||
Reference in New Issue
Block a user