mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-05-24 17:06:00 +00:00
fix(task-tool): defend _find_usage_recorder against non-list callbacks
Address review feedback. The previous commit handled the two common shapes LangChain hands to async tool runs — a plain `list[BaseCallbackHandler]` and a `BaseCallbackManager` subclass — but iterated any other shape directly, which would still raise `TypeError` if e.g. a single handler instance leaked through without a list wrapper. Treat any non-list, non-manager `config["callbacks"]` value as "no recorder" rather than crash. Docstring now lists all four shapes explicitly. New tests cover the single-handler-object case, `runtime is None`, `callbacks is None`, and `runtime.config` being a non-dict — all required to be silent no-ops. Refs: bytedance/deer-flow#3107 (BUG-002)
This commit is contained in:
@@ -102,10 +102,16 @@ def _schedule_deferred_subagent_cleanup(task_id: str, trace_id: str, max_polls:
|
|||||||
def _find_usage_recorder(runtime: Any) -> Any | None:
|
def _find_usage_recorder(runtime: Any) -> Any | None:
|
||||||
"""Find a callback handler with ``record_external_llm_usage_records`` in the runtime config.
|
"""Find a callback handler with ``record_external_llm_usage_records`` in the runtime config.
|
||||||
|
|
||||||
LangChain may pass ``config["callbacks"]`` as either a plain list of handlers
|
LangChain may pass ``config["callbacks"]`` in three different shapes:
|
||||||
or as a ``BaseCallbackManager`` instance (e.g. ``AsyncCallbackManager`` on
|
|
||||||
async tool runs). Callback managers are not iterable; unwrap their
|
- ``None`` (no callbacks registered): no recorder.
|
||||||
``handlers`` list before searching.
|
- A plain ``list[BaseCallbackHandler]``: iterate it directly.
|
||||||
|
- A ``BaseCallbackManager`` instance (e.g. ``AsyncCallbackManager`` on async
|
||||||
|
tool runs): managers are not iterable, so we unwrap ``.handlers`` first.
|
||||||
|
|
||||||
|
Any other shape (e.g. a single handler object accidentally passed without a
|
||||||
|
list wrapper) cannot be iterated safely; treat it as "no recorder" rather
|
||||||
|
than raise.
|
||||||
"""
|
"""
|
||||||
if runtime is None:
|
if runtime is None:
|
||||||
return None
|
return None
|
||||||
@@ -117,6 +123,8 @@ def _find_usage_recorder(runtime: Any) -> Any | None:
|
|||||||
callbacks = callbacks.handlers
|
callbacks = callbacks.handlers
|
||||||
if not callbacks:
|
if not callbacks:
|
||||||
return None
|
return None
|
||||||
|
if not isinstance(callbacks, list):
|
||||||
|
return None
|
||||||
for cb in callbacks:
|
for cb in callbacks:
|
||||||
if hasattr(cb, "record_external_llm_usage_records"):
|
if hasattr(cb, "record_external_llm_usage_records"):
|
||||||
return cb
|
return cb
|
||||||
|
|||||||
@@ -63,3 +63,29 @@ def test_find_usage_recorder_handles_empty_manager():
|
|||||||
manager = AsyncCallbackManager(handlers=[])
|
manager = AsyncCallbackManager(handlers=[])
|
||||||
runtime = _make_runtime(manager)
|
runtime = _make_runtime(manager)
|
||||||
assert _find_usage_recorder(runtime) is None
|
assert _find_usage_recorder(runtime) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_usage_recorder_returns_none_for_none_runtime():
|
||||||
|
assert _find_usage_recorder(None) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_usage_recorder_returns_none_when_callbacks_is_none():
|
||||||
|
runtime = _make_runtime(None)
|
||||||
|
assert _find_usage_recorder(runtime) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_usage_recorder_returns_none_for_single_handler_object():
|
||||||
|
"""A single handler instance (not wrapped in a list or manager) should not crash.
|
||||||
|
|
||||||
|
LangChain's contract is that ``config["callbacks"]`` is a list-or-manager,
|
||||||
|
but we treat any other shape defensively rather than letting a ``for`` loop
|
||||||
|
blow up at runtime.
|
||||||
|
"""
|
||||||
|
runtime = _make_runtime(_RecorderHandler())
|
||||||
|
assert _find_usage_recorder(runtime) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_usage_recorder_returns_none_when_config_not_dict():
|
||||||
|
"""Defensive: a runtime without a dict-shaped config should not raise."""
|
||||||
|
runtime = SimpleNamespace(config="not-a-dict")
|
||||||
|
assert _find_usage_recorder(runtime) is None
|
||||||
|
|||||||
Reference in New Issue
Block a user