Skip to content

Bugfix: 修复llm调用抛出GeneratorExit时无trace上报的问题 - #283

Open
weimch wants to merge 1 commit into
mainfrom
bugfix/trace_llm_failed
Open

Bugfix: 修复llm调用抛出GeneratorExit时无trace上报的问题#283
weimch wants to merge 1 commit into
mainfrom
bugfix/trace_llm_failed

Conversation

@weimch

@weimch weimch commented Aug 4, 2026

Copy link
Copy Markdown
Contributor
  • 原因:服务部署时,如果客户端直接断开连接,则运行中的Agent会收到协程被取消的异常,之前异常时没有上报,现在修复这个问题

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

我已审查 pr.diff(266 行),并核对了 _llm_processor.pyrunners.py_base_agent.py 的上下文以及 _trace.py/_metrics.py 中被调用的 trace/metrics 函数。下面是结论。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/runners.py:608-625:Runner 的 trace 流转重构缺少测试覆盖
    • 本 PR 将 trace_runner 从 try 末尾迁入 finally 并新增 runner_trace_recorded 标志,引入了三条新执行路径:正常完成、RunCancelledException(设标志后 finally 跳过)、以及非取消异常 / consumer 提前 aclose(finally 兜底记录)。_llm_processor.py 对等改动已补 test_partial_stream_close_traces_request_with_empty_response,但 Runner 侧无任何测试验证 runner_trace_recorded 在取消 / 异常 / 提前关闭下不重复、不漏记 trace。建议补一个用例:streaming 下 consumer 提前关闭时 trace_runner 恰好被调用一次、且与取消路径互斥。

💡 Suggestion

  • trpc_agent_sdk/runners.py:608-625:异常路径下 invocation span 未标记 ERROR
    • 迁入 finally 后,非取消异常也会走到 trace_runner,但 trace_runner(见 _trace.py:106)只写正常属性、不调用 set_status(ERROR),与取消路径的 trace_cancellation 设 ERROR 不一致,排障时失败请求的 span 可能显示成功。建议在 finally 中根据是否存在未处理异常对 invocation_span 标记 ERROR status。

总结

本次改动将 trace_call_llm / trace_runner / trace_cancellation / trace_agent 统一收敛到 finally 并通过 trace.use_span(..., end_on_exit=False) 重新进入对应 span 记录属性,正确解决了 async generator 被取消或消费者提前关闭时 trace 丢失的问题,逻辑自洽、与现有 span 生命周期一致。未发现 Critical 阻塞问题;主要缺口在 Runner 侧新流转路径的测试覆盖与异常路径 span 状态标记。

测试建议

  • 建议补 Runner 侧测试:streaming 模式下消费者提前 aclose() 生成器时,trace_runner 被调用且恰好一次;以及 RunCancelledException 路径下 trace_runner 不被调用(仅 trace_cancellation)。
  • 可选:验证非取消异常向上传播时 invocation span 的最终状态(与上述 Suggestion 配套)。

@weimch
weimch force-pushed the bugfix/trace_llm_failed branch from 1c36203 to ab7d3f5 Compare August 5, 2026 02:22
- 原因:服务部署时,如果客户端直接断开连接,则运行中的Agent会收到协程被取消的异常,之前异常时没有上报,现在修复这个问题
@weimch
weimch force-pushed the bugfix/trace_llm_failed branch from ab7d3f5 to b9c13ad Compare August 5, 2026 07:24
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

我已经完整审查了 pr.diff 及相关上下文(_llm_processor.pyrunners.py_base_agent.py_trace.py_metrics.py 及对应测试)。下面是结论。

发现的问题

⚠️ Warning

  • trpc_agent_sdk/telemetry/_trace.py:482-488:移除了错误响应上的 span.add_event("exception", ...) 调用
    • 此次重构将 trace_call_llm 错误分支改为调用 mark_span_error,并删除了原本写入的 exception event(error.type/error.message 属性保留,仅 event 被移除)。这是面向可观测性的行为变更:依赖该 exception span event 做错误归因、告警或异常采样的下游监控/导出器将不再收到该事件。
    • 若该移除是有意为之(注释表明为了让导出器继续以 llm_response 作为 output),建议在变更说明中显式提示下游监控可能需要改为基于 error.type 属性或 span status 判定,避免静默断链。

💡 Suggestion

  • trpc_agent_sdk/agents/core/_llm_processor.py:203-219finally 块中通过 trace.use_span(call_llm_span, end_on_exit=False) 包裹 trace_call_llm,与 _base_agent.pyrunners.py 中的相同模式重复出现三处。
    • 该模式是为了在 async generator 被 aclose() 时仍能把 trace 写到正确 span,逻辑正确;但三处用法完全一致,后续若调整(例如增加结束态属性)需同步改三处,建议抽取一个小的 with_current_span(span) 辅助函数统一调用,降低维护成本。

总结

整体逻辑自洽:GeneratorExit 在 runner / agent / call_llm 三层均被捕获并通过 mark_span_error 标记 span,trace 调用统一移入 finally 保证在流式提前关闭时仍会上报,测试覆盖了中断、异常、function_call、thought 拼接等关键路径。未发现必须修复的阻塞性问题,仅有一处可观测性兼容性变更需向下游提示。

测试建议

  • 建议补充一条用例:call_llm_asyncgenerate_async 抛出非 GeneratorExit 异常、且已收到多个 partial 后被外层 aclose()(即异常与 GeneratorExit 叠加)时,验证 trace_call_llmreport_call_llmerror_type 取值与上报顺序,覆盖 except Exceptionfinally 在 re-raise 路径下仍正常完成 trace 的场景。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants