-
-
Notifications
You must be signed in to change notification settings - Fork 41
[Fix] 修复核心服务错误处理和生命周期管理问题 #552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add chatluna/after-chat-error event for proper error cleanup - Fix message delay middleware to handle both success and error cases - Migrate memory plugins from ctx.on to ctx.before for correct event timing - Update lunar variable plugin to use proper event handler syntax - Ensure chat session cleanup occurs regardless of success or failure These changes resolve issues where chat sessions could remain in inconsistent states after errors, particularly fixing timeout Promise handling and ensuring proper resource cleanup.
- Comment out plugin not found warning to reduce noise - Remove plugin cleanup loop in stop() method to prevent disposal issues - Improve service shutdown process reliability
Walkthrough扩展聊天错误处理以携带调用上下文并新增 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as 用户
participant App as Chat(App)
participant Ctx as Koishi Ctx
participant Chain as LLM Chain
participant MW as message_delay 中间件
U->>App: 调用 chat(arg)
App->>Ctx: ctx.before('chatluna/chat', ...) 获取 promptVariables / chain
Ctx-->>App: 返回 promptVariables, chain
App->>Chain: processChat(arg, wrapper)
alt 成功
Chain-->>App: 返回 responseMessage
App->>App: 如果有 postHandler -> handlePostProcessing(responseMessage) -> 更新 displayResponse 与 additionalArgs
App->>Ctx: 触发 'chatluna/after-chat'
Ctx->>MW: after-chat
MW->>MW: completeBatch()
App-->>U: 返回 displayResponse
else 抛错
Chain--xApp: 抛出 error
App->>App: handleChatError(arg, wrapper, error)
App->>Ctx: 触发 'chatluna/after-chat-error'(error, conversationId, sourceMessage, promptVariables, chatInterface, chain)
Ctx->>MW: after-chat-error
MW->>MW: completeBatch()
App--xU: 继续抛出 error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/middlewares/chat/message_delay.ts (1)
133-154: 阻塞:先调用 resolveWaiters 再 delete 会误删新批次,导致死锁/消息丢失当
resolveWaiters执行时(如waitForBatchCompletion的 waiter),会立即batches.set(conversationId, ...)生成“下一批”。紧接着当前代码的batches.delete(conversationId)会把这个新批次删掉,后续就无法继续处理,表现为卡死或消息丢失。应先删除“旧批次”,再唤醒 waiters,并对 delete 做同一性校验以避免竞态。建议修复(最小改动,保持现有语义):
- const completeBatch = async (conversationId: string) => { - const batch = batches.get(conversationId) - if (batch?.resolveWaiters.length > 0) { + const completeBatch = async (conversationId: string) => { + const batch = batches.get(conversationId) + if (!batch) return + if (batch.resolveWaiters.length > 0) { logger.debug( `Completing batch for ${conversationId}, messages: ${batch.messages.length}` ) - if (batch.timeout) { - batch.timeout() - } - batch.resolveWaiters.forEach((resolve) => resolve()) - batches.delete(conversationId) + if (batch.timeout) { + batch.timeout() + // 释放引用,防误用 + batch.timeout = undefined + } + // 先删除旧批次,避免 waiter 里 set 的新批次被误删 + if (batches.get(conversationId) === batch) { + batches.delete(conversationId) + } + for (const resolve of batch.resolveWaiters) resolve() } else if (batch) { logger.debug(`Cleaning up batch for ${conversationId}`) if (batch.timeout) { batch.timeout() } if (batch.processorResolve) { batch.processorResolve(ChainMiddlewareRunStatus.STOP) } - batches.delete(conversationId) + if (batches.get(conversationId) === batch) { + batches.delete(conversationId) + } } }此修复为必须合入项。
🧹 Nitpick comments (1)
packages/core/src/services/chat.ts (1)
162-163: 注释掉的警告日志可能会隐藏插件卸载问题注释掉插件未找到的警告日志可能会让开发者难以发现插件卸载时的问题。虽然这样可以减少日志噪音,但建议考虑:
- 使用 debug 级别日志而非完全注释掉
- 或者在特定场景下(如服务停止时)跳过警告
- // this.ctx.logger.warn('Plugin %c not found', platformName) + this.ctx.logger.debug('Plugin %c not found during uninstall', platformName)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/core/src/llm-core/chat/app.ts(3 hunks)packages/core/src/llm-core/memory/authors_note/index.ts(1 hunks)packages/core/src/llm-core/memory/lore_book/index.ts(1 hunks)packages/core/src/middlewares/chat/message_delay.ts(2 hunks)packages/core/src/services/chat.ts(1 hunks)packages/variable-extension/src/plugins/lunar.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/llm-core/chat/app.ts (1)
packages/core/src/llm-core/chain/base.ts (1)
ChatLunaLLMCallArg(37-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
packages/core/src/llm-core/memory/lore_book/index.ts (1)
12-55: 事件钩子从on改为before的变更看起来合理将事件注册从
ctx.on('chatluna/before-chat', ...)改为ctx.before('chatluna/chat', ...)符合整体 PR 的事件重构方向。回调参数保持不变,功能逻辑也未改变。packages/core/src/llm-core/memory/authors_note/index.ts (1)
7-39: 事件钩子迁移符合预期从
ctx.on('chatluna/before-chat', ...)改为ctx.before('chatluna/chat', ...),与 lore_book 的变更保持一致,是统一事件处理模式的一部分。packages/variable-extension/src/plugins/lunar.ts (1)
16-32: 农历插件的事件钩子更新正确事件注册从
ctx.on('chatluna/before-chat', ...)改为ctx.before('chatluna/chat', ...),与其他文件的变更保持一致。农历数据填充逻辑保持不变。packages/core/src/llm-core/chat/app.ts (3)
52-65: 新的错误处理机制设计良好
handleChatError方法的签名变更增加了更多上下文信息(arg 和 wrapper),这样能够在错误事件中提供更丰富的调试信息。新增的chatluna/after-chat-error事件可以让其他模块统一处理聊天错误。
112-112: 错误处理调用正确更新catch 块中正确地传递了新增的参数(arg 和 wrapper)给
handleChatError方法。
483-490: 新增的错误事件声明完整
chatluna/after-chat-error事件的声明包含了所有必要的参数,可以为错误处理提供充分的上下文信息。packages/core/src/services/chat.ts (1)
341-344: 确认:stop() 不需要显式遍历卸载插件 — 插件在其它位置自动清理
ChatLunaPlugin 构造函数通过 ctx.on('dispose') 调用 ctx.chatluna.uninstallPlugin(this),各 register* 调用使用 this.ctx.effect(...) 注册资源/清理;uninstallPlugin 会调用 _chatInterfaceWrapper?.dispose(platform) 并从 _plugins 删除插件。故 stop() 仅 dispose chatInterfaceWrapper 与 platformService 是预期行为,无需在此处再次遍历卸载插件。packages/core/src/middlewares/chat/message_delay.ts (2)
156-164: 事件参数顺序正确;建议(可选)为回调加异常捕获仓库中 emit('chatluna/after-chat-error', error, arg.conversationId, arg.message) 位于 packages/core/src/llm-core/chat/app.ts,参数顺序为 (error, conversationId, ...),因此 message_delay.ts 中的 async (_, conversationId) 签名是正确的。仍建议将 completeBatch 的调用包在 try/catch 中以防未捕获异常。
Likely an incorrect or invalid review comment.
145-152: 忽略此建议:completeBatch的无等待者分支只在after-chat-error或显式清理时调用,正常延时回调会在定时器内删除批次并返回 CONTINUE,不会丢弃积压消息。Likely an incorrect or invalid review comment.
- Replace map() with flatMap() to properly handle undefined values - Return empty arrays instead of undefined for non-text messages - Add null coalescing operator for content attribute safety Closes #553
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/renders/mixed-voice.ts (1)
83-96: 对齐 voice.ts:为 content 增加空值兜底,避免 marked.lexer 接收 undefined 崩溃
- 目前
marked.lexer(message.attrs['content'])在 content 缺失时可能抛错;voice.ts 已对返回分支使用?? '',建议这里也同时对 lexer 入参与返回分支兜底,保持一致性与健壮性。应用以下补丁:
- const tokens = renderTokens( - marked.lexer(message.attrs['content']) - ) + const tokens = renderTokens( + marked.lexer(message.attrs['content'] ?? '') + ) @@ - if (tokens.length === 0 || tokens[0].length === 0) { - return message.attrs['content'] - } + if (tokens.length === 0 || tokens[0].length === 0) { + return message.attrs['content'] ?? '' + }
🧹 Nitpick comments (1)
packages/core/src/renders/mixed-voice.ts (1)
81-98: 抽公用拆分逻辑,避免日后漂移
- mixed-voice 与 voice 两处
_splitMessage基本相同,建议提取到共享 util,降低重复与差异化回归的风险。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/renders/mixed-voice.ts(1 hunks)packages/core/src/renders/voice.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#548
File: packages/core/src/llm-core/chat/app.ts:0-0
Timestamp: 2025-09-17T00:25:27.195Z
Learning: 在 ChatInterface 类中,响应式 watch 调用已经通过 ctx.effect() 包装来自动处理清理工作,避免内存泄漏。字段 _chain 和 _embeddings 的类型已更新为可空类型。
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: CodeFactor
🔇 Additional comments (3)
packages/core/src/renders/mixed-voice.ts (1)
83-86: flatMap 丢弃非文本:方向正确
- 使用 flatMap 并对非文本直接返回 [],能避免 undefined 被保留,管线更干净。
packages/core/src/renders/voice.ts (2)
50-53: flatMap 与非文本丢弃实现稳健
- 非文本返回 [] 避免脏数据进入语音渲染;实现清晰可读。
48-65: 可选:合并重复的空串过滤以减少一次遍历经核查,packages/core/src/renders/voice.ts 与 packages/core/src/renders/mixed-voice.ts 的 _splitMessage 在返回后都做了 .filter(Boolean),且两处调用方随后都做了 .trim().split('\n\n').filter(text => text.length > 0)。建议合并过滤以减少一次数组遍历——推荐保留调用方的精确过滤并移除 _splitMessage 末尾的 .filter(Boolean)。
注意:仓内针对 marked.lexer 的全量调用未在本次搜索中找到,请确认上游是否始终传入字符串;若不能保证,在 _splitMessage 内加入类型/空值保护。
…deprecated providers - Completely rewrote DuckDuckGo Lite provider to use proper response parsing with regex extraction - Added CallbackSearchResult and CallbackNextSearch interfaces for better type safety - Implemented performSearch method with modern fetch API and proper error handling - Added html-entities dependency for proper HTML entity decoding - Removed deprecated free_google_api provider and related configuration - Updated default search engine from free-google-api to bing-web - Fixed response parsing to handle DuckDuckGo's JavaScript callback format correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/search-service/src/index.ts (1)
251-256: Schema 仍包含 freeSearchBaseURL,接口/PR 目标不一致接口 Config 已不含该字段,但 Schema 仍暴露该配置,易致用户侧出现“幽灵配置”。请移除该段并清理 i18n 文案。
应用此变更删除该配置块:
- Schema.object({ - freeSearchBaseURL: Schema.string().default( - 'https://search.dingyi222666.top' - ) - }),
🧹 Nitpick comments (9)
packages/search-service/src/providers/duckduckgo_lite.ts (9)
1-1: 移除未使用的导入
sleep未使用,建议删除以消除噪音与潜在 lint 失败。-import { Context, Schema, sleep } from 'koishi' +import { Context, Schema } from 'koishi'
66-79: 硬编码 UA/客户端提示头易失效,建议可配置化或降级Chrome 版本号与 sec-* 头硬编码,未来被风控拦截概率高。建议:
- 允许通过 Config 覆盖 UA/headers;
- 或提供最小集通用头,并在 429/阻断时降级重试。
84-85: 空查询处理建议返回空结果而非抛错上层已做错误汇聚,空查询直接返回
[]更平滑,避免无意义异常链。- if (!query) throw new Error('Query cannot be empty!') + if (!query?.trim()) return []
96-101: catch 日志与异常类型安全
error.message在 TS 严格模式下类型不安全;同时建议记录完整错误对象。- } catch (error) { - logger.error(`DuckDuckGo search failed: ${error.message}`) - throw error + } catch (e: unknown) { + if (e instanceof Error) { + logger.error('DuckDuckGo search failed: %s', e.message) + } else { + logger.error('DuckDuckGo search failed: %o', e) + } + throw e }
120-154: 区域/语言参数硬编码,导致非英语环境结果劣化
dl/ct/bing_market固定为 EN/US,和locale/region不一致。建议从options派生。- dl: 'en', - ct: 'US', - bing_market: options.marketRegion, + // derive language/country + // locale like: en-us / zh-CN + dl: (options.locale.split(/[-_]/)[0] || 'en').toLowerCase(), + ct: ((options.marketRegion.split('-')[1] || options.locale.split(/[-_]/)[1] || 'US')).toUpperCase(), + bing_market: options.marketRegion || `${(options.locale.split(/[-_]/)[0] || 'en').toLowerCase()}-${(options.locale.split(/[-_]/)[1] || 'US').toUpperCase()}`,
156-163: 为外部请求增加超时/可中断机制避免请求悬挂影响链路。可用 AbortController + config 超时。
- const response = await this._plugin.fetch( - `https://links.duckduckgo.com/d.js?${searchParams.toString()}`, - { - headers: this.COMMON_HEADERS - } - ) + const controller = new AbortController() + const timeout = setTimeout( + () => controller.abort(), + this.config.puppeteerTimeout ?? 60000 + ) + const response = await this._plugin.fetch( + `https://links.duckduckgo.com/d.js?${searchParams.toString()}`, + { headers: this.COMMON_HEADERS, signal: controller.signal } + ) + clearTimeout(timeout)
182-189: 解析健壮性:增加 JSON.parse 容错建议对解析失败抛出更可诊断的错误。
- const searchResults = JSON.parse(match[1].replace(/\t/g, ' ')) as ( + let searchResults: (CallbackSearchResult | CallbackNextSearch)[] + try { + searchResults = JSON.parse(match[1].replace(/\t/g, ' ')) as ( | CallbackSearchResult | CallbackNextSearch - )[] + )[] + } catch (e) { + throw new Error('Failed to parse DuckDuckGo JSON payload.') + }
208-219: 结果过滤与 URL 规范化避免非 http(s) 协议/无效 URL 泄入下游;同时对 URL 也做解码。
- if (result.u && result.t && result.a) { - results.push({ - title: decode(result.t), - url: result.u, - description: decode(result.a) - }) - } + if (result.u && result.t && result.a) { + const url = decode(result.u) + if (!/^https?:\/\//i.test(url)) continue + try { new URL(url) } catch { continue } + results.push({ + title: decode(result.t), + url, + description: decode(result.a), + }) + }
257-259: _getVQD 异常信息类型安全同上,使用
unknown守卫,避免e.message未定义。- } catch (e) { - const err = `Failed to get the VQD for query "${query}". Error: ${e.message}` - throw new Error(err) + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : String(e) + throw new Error(`Failed to get the VQD for query "${query}". Error: ${msg}`) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/search-service/package.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
packages/search-service/src/index.ts(1 hunks)packages/search-service/src/plugin.ts(0 hunks)packages/search-service/src/providers/duckduckgo_lite.ts(3 hunks)packages/search-service/src/providers/free_google_api.ts(0 hunks)
💤 Files with no reviewable changes (2)
- packages/search-service/src/plugin.ts
- packages/search-service/src/providers/free_google_api.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/search-service/src/providers/duckduckgo_lite.ts (3)
packages/search-service/src/types.ts (1)
SearchResult(1-6)packages/search-service/src/index.ts (1)
logger(18-18)packages/search-service/src/provide.ts (1)
search(57-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (3)
packages/search-service/src/providers/duckduckgo_lite.ts (3)
6-7: 确认依赖声明新增
html-entities用于解码,请确认已在对应 package.json 中声明依赖(非 devDependency)。
262-267: 提供者 schema 声明:LGTM与注册名一致,便于 UI 显示。
277-280: 按需注册 provider:LGTM按配置开关加载,符合预期。
…essage handling - Fix tool registration names: use underscores instead of hyphens for consistency - web-search -> web_search - web-browser -> web_browser - Move intermediate tool message handling from plugin_chat_chain to chat app - Fix tool comparison logic to use tool name instead of object reference - Add webdriver detection avoidance to Bing web search provider - Improve logging format for active tools debug output - Remove unused imports and clean up tool call handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/search-service/src/chain/browsing_chain.ts (2)
198-204: _selectTool 未做空值判定,工具不存在时会抛出 TypeError当 this.tools 中找不到对应 name 时,chatLunaTool 为 undefined,继续访问 chatLunaTool.tool 会直接抛异常。建议显式校验并抛出可诊断错误。
可参考:
private async _selectTool(name: string): Promise<StructuredTool> { const chatLunaTool = this.tools.value.find((t) => t.name === name) if (!chatLunaTool) { throw new Error(`Tool "${name}" not found in tools registry.`) } return chatLunaTool.tool.createTool({ embeddings: this.embeddings }) }
375-383: searchTool 返回解析未做防御性处理,JSON.parse 失败将使整个搜索流程中断第三方搜索工具偶发返回非严格 JSON(或为空),这里缺少 try/catch,会直接抛错并终止 _search。
建议:
let parsedSearchResults: { title: string; description: string; url: string }[] = [] try { parsedSearchResults = JSON.parse(rawSearchResults) ?? [] } catch (e) { logger?.warn(`search result is not valid JSON: ${String(e)}`) return // 跳过本次条目,避免打断整个流程 }
🧹 Nitpick comments (9)
packages/search-service/src/providers/bing_web.ts (3)
1-1: 避免全局禁用 eslint 规则不建议对整文件
/* eslint-disable no-proto */。后续将__proto__黑魔法替换为更安全方式后,可去掉此禁用。-/* eslint-disable no-proto */
51-51: 移除页面上下文中的调试日志
console.log(liElements)仅打印到浏览器控制台,生产无用且可能暴露痕迹。- console.log(liElements)
2-2: 若删除固定休眠,则移除sleep依赖导入避免无用依赖与误用。
-import { Context, Schema, sleep } from 'koishi' +import { Context, Schema } from 'koishi'packages/search-service/src/chain/browsing_chain.ts (5)
366-374: Abort 监听器未清理,存在累积监听与潜在内存泄漏风险在 Promise.race 中为 signal 添加的 abort 监听没有移除;当未触发 abort 时,这些未清理的监听会累积。建议使用 once: true 或封装复用。
示例封装:
const waitForAbort = (signal: AbortSignal) => new Promise<never>((_, reject) => { if (signal?.aborted) return reject(new ChatLunaError(ChatLunaErrorCode.ABORTED)) const onAbort = () => reject(new ChatLunaError(ChatLunaErrorCode.ABORTED)) signal?.addEventListener('abort', onAbort, { once: true }) }) // 用法 await Promise.race([ searchTool.invoke(question).then((t) => t as string), waitForAbort(signal), ])Also applies to: 392-404, 418-441
510-512: closeBrowser 缺少防守性检查,非 PuppeteerBrowserTool 场景下可能抛错如果实际获取到的并非 PuppeteerBrowserTool 实例,这里会因不存在 closeBrowser 而报错。建议守护式调用。
if (typeof (webBrowserTool as any)?.closeBrowser === 'function') { await (webBrowserTool as any).closeBrowser() }
495-498: 英文提示语存在语法错误与用户体验问题当前字符串含有 “the your's question”等错误,建议修正以免误导或降低专业感。
建议文案:
"I understand. I will respond to your question using the same language as your input. What's your question?"
445-457: 搜索结果字符串拼接依赖 for...in 的枚举顺序,不够稳定为确保上下文可预测,建议按固定字段顺序格式化,或使用 JSON.stringify 并限制字段。
示例:
return `title: ${result.title}, url: ${result.url}, description: ${result.description}`或:
return JSON.stringify({ title: result.title, url: result.url, description: result.description })
336-341: 添加对旧工具名(连字符)的兼容回退仓库已注册 'web_search' 与 'web_browser'(packages/search-service/src/index.ts),PuppeteerBrowserTool 实现了 closeBrowser(packages/search-service/src/tools/puppeteerBrowserTool.ts);为防外部仍使用旧名 'web-search'/'web-browser' 导致运行时失败,建议在 select 时加入回退。
位置:packages/search-service/src/chain/browsing_chain.ts(约 336–341 行)
- const searchTool = await this._selectTool('web_search') + const searchTool = + await this._selectTool('web_search') + .catch(() => this._selectTool('web-search')) - const webBrowserTool = (await this._selectTool( - 'web_browser' - )) as PuppeteerBrowserTool + const webBrowserTool = (await this._selectTool('web_browser') + .catch(() => this._selectTool('web-browser'))) as PuppeteerBrowserToolpackages/core/src/llm-core/chain/plugin_chat_chain.ts (1)
196-196: 工具匹配逻辑改变可能影响正确性从按对象引用匹配改为按名称匹配工具可能会产生问题。如果不同的工具实例具有相同的名称但不同的配置或行为,这种改变可能导致错误的工具被识别为"相同"。
建议使用更稳定的标识符进行匹配,如工具的 ID:
- (tool) => tool.name === differenceTool[0].name + (tool) => tool.id === differenceTool[0].id这样可以确保精确匹配,避免名称冲突的问题。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/core/src/llm-core/chain/plugin_chat_chain.ts(3 hunks)packages/core/src/llm-core/chat/app.ts(5 hunks)packages/search-service/src/chain/browsing_chain.ts(1 hunks)packages/search-service/src/index.ts(3 hunks)packages/search-service/src/providers/bing_web.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/search-service/src/index.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T17:40:57.183Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#546
File: packages/core/src/llm-core/chat/app.ts:114-119
Timestamp: 2025-09-15T17:40:57.183Z
Learning: 在 ChatLuna 的 packages/core/src/llm-core/chat/app.ts 中,displayResponse 的构造只需要保留 content 和 additional_kwargs,tool_calls、tool_call_id、name、id 等元数据在代码库的其他地方处理,无需在 displayResponse 中保留这些字段。
Applied to files:
packages/core/src/llm-core/chat/app.ts
🧬 Code graph analysis (1)
packages/core/src/llm-core/chat/app.ts (2)
packages/core/src/llm-core/chain/base.ts (1)
ChatLunaLLMCallArg(37-48)packages/core/src/llm-core/agent/types.ts (1)
AgentStep(250-253)
🔇 Additional comments (9)
packages/core/src/llm-core/chain/plugin_chat_chain.ts (3)
202-204: LGTM!逻辑修正合理修复了工具激活时的处理逻辑。之前当
differenceTool[1] === true时会跳过将工具添加到oldActiveTools的操作,现在正确地将激活的工具添加到活跃工具列表中。
250-252: 日志格式改进改进了执行器重建时的日志格式,添加了空格并将工具列表连接为单个字符串,提高了日志的可读性。
1-1: 结论:移除的导入不会导致编译错误。packages/core/src/llm-core/chain/plugin_chat_chain.ts 中未发现对 ToolMessage 或 AgentStep 的任何引用;AgentStep 的类型定义位于 packages/core/src/llm-core/agent/types.ts。
packages/core/src/llm-core/chat/app.ts (6)
27-27: 导入恢复正确重新添加了
ToolMessage和AgentStep的导入,这与 AI 摘要中提到的在plugin_chat_chain.ts中移除这些导入是一致的。这些导入在当前文件中的新功能(并行中间步骤处理)中是必需的。Also applies to: 32-32
53-66: 错误处理增强符合 PR 目标
handleChatError方法的签名扩展很好地符合了 PR 的目标——改进错误处理和事件生命周期管理。新增的chatluna/after-chat-error事件提供了更丰富的错误上下文信息,有助于调试和监控。
113-113: 错误处理调用更新正确正确地将新的参数传递给
handleChatError方法,确保错误处理时能够获得完整的调用上下文。
121-128: 响应处理逻辑改进引入
responseMessage变量来存储原始响应,然后基于它创建displayResponse。这种分离很好地支持了原始响应与显示响应的不同处理需求。根据从长期记忆中获取的学习,
displayResponse的构造只需要保留content和additional_kwargs,这与当前的实现一致。
521-528: 新事件定义完善
chatluna/after-chat-error事件的定义提供了丰富的错误上下文信息,包括错误对象、对话 ID、源消息、提示变量、聊天接口和链。这将有助于其他模块更好地处理和响应错误情况。
155-196: 并行中间步骤实现正确 — 类型验证通过AgentStep/AgentAction 在 packages/core/src/llm-core/agent/types.ts 中定义,AgentAction 包含可选字段 toolCallId,且 output_parser 与 openai/index.ts 中对该字段有设置与检查;step.action.toolCallId 的使用与类型一致,批准变更。
…providers - Enhanced error handling in ChatInterface with proper try-catch blocks - Fixed proxy agent checks in request utils to handle null values correctly - Removed unnecessary sleep delays in Bing web search provider - Improved DuckDuckGo Lite provider with better HTML parsing and error handling - Cleaned up tool logging format in plugin chat chain
- Add null safety to voice renderer content processing - Wrap Bing web search in try-finally block for proper page cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (3)
packages/search-service/src/providers/bing_web.ts (1)
73-76: 页面关闭放在 finally 已修复资源泄漏,LGTM;建议吞掉 close 异常极端情况下 page.close() 也可能抛错(不应覆盖原始错误)。
- } finally { - await page.close() - } + } finally { + try { + await page.close() + } catch {} + }packages/search-service/src/providers/duckduckgo_lite.ts (2)
97-164: HTML 结果解析逻辑解析 HTML 结果的正则表达式看起来合理,但需要注意几个问题:
- 正则表达式使用
result-link和result-snippet类名,这些可能会随着 DuckDuckGo 的 HTML 结构变化而失效- 链接和片段的匹配逻辑假设它们有 1:1 的对应关系,但实际可能不是这样
建议增加更多的错误处理和备用解析策略,以应对 HTML 结构的变化。
216-223: VQD 提取方法未被使用
extractVqd方法被保留但在当前实现中未被使用。如果这是为了未来的功能准备,建议添加注释说明其用途。可以考虑将暂时不用的方法标记为私有或添加注释说明其预期用途。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/core/src/llm-core/chain/plugin_chat_chain.ts(3 hunks)packages/core/src/llm-core/chat/app.ts(6 hunks)packages/core/src/renders/voice.ts(1 hunks)packages/core/src/utils/request.ts(3 hunks)packages/search-service/src/providers/bing_web.ts(2 hunks)packages/search-service/src/providers/duckduckgo_lite.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/llm-core/chain/plugin_chat_chain.ts
- packages/core/src/renders/voice.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T17:40:57.183Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#546
File: packages/core/src/llm-core/chat/app.ts:114-119
Timestamp: 2025-09-15T17:40:57.183Z
Learning: 在 ChatLuna 的 packages/core/src/llm-core/chat/app.ts 中,displayResponse 的构造只需要保留 content 和 additional_kwargs,tool_calls、tool_call_id、name、id 等元数据在代码库的其他地方处理,无需在 displayResponse 中保留这些字段。
Applied to files:
packages/core/src/llm-core/chat/app.ts
🧬 Code graph analysis (2)
packages/core/src/llm-core/chat/app.ts (2)
packages/core/src/llm-core/chain/base.ts (1)
ChatLunaLLMCallArg(37-48)packages/core/src/llm-core/agent/types.ts (1)
AgentStep(250-253)
packages/search-service/src/providers/duckduckgo_lite.ts (3)
packages/search-service/src/index.ts (3)
Config(162-194)Config(196-385)logger(18-18)packages/core/src/services/chat.ts (1)
ChatLunaPlugin(571-779)packages/search-service/src/types.ts (1)
SearchResult(1-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (19)
packages/core/src/utils/request.ts (3)
19-26: 已批准此代码更改!函数参数从全局代理地址迁移到每次调用的 proxyAddress 参数是正确的,这与 Undici 的最佳实践一致,能够为每个请求提供更精细的代理控制。检查
proxyAddress是否为 null 而不是依赖全局状态的逻辑改进是合理的。
100-106: 已批准此代码更改!条件检查的改进使用了明确的 null 检查
init?.['dispatcher'] == null而不是布尔否定,这与 Undici 的最佳实践一致。proxyAddress !== 'null'的字符串比较确保了字符串 'null' 不被误认为有效的代理地址。
126-135: 已批准此代码更改!条件守卫改为对
options?.agent进行 null 检查,与新模式保持一致。移除不必要的类型转换并在提供有效的 proxyAddress 且尚未存在 agent 时附加代理 agent 的逻辑是正确的。packages/search-service/src/providers/bing_web.ts (2)
15-37: 反爬伪装过度且易失效;改为“最小、可配置、与网络层 UA 一致”的策略当前直接改 navigator.proto、在页面上下文覆写 UA、注入大量 window.chrome 桩,指纹风险高且与请求头 UA 不一致。建议仅屏蔽 navigator.webdriver,并将 UA 在 Node 侧通过 page.setUserAgent 对齐;同时加配置开关(如 this.config.searchStealth)。
- try { - // webdriver - await page.evaluateOnNewDocument(() => { - const newProto = navigator['__proto__'] - delete newProto.webdriver - navigator['__proto__'] = newProto - - window['chrome'] = {} - window['chrome'].app = { - InstallState: 'hehe', - RunningState: 'haha', - getDetails: 'xixi', - getIsInstalled: 'ohno' - } - window['chrome'].csi = function () {} - window['chrome'].loadTimes = function () {} - window['chrome'].runtime = function () {} - - Object.defineProperty(navigator, 'userAgent', { - get: () => - 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/121.0.4044.113 Safari/537.36' - }) - }) + try { + // 可选“隐身”:最低限度且与网络层 UA 一致 + if (this.config.searchStealth) { + await page.evaluateOnNewDocument(() => { + try { + Object.defineProperty(navigator, 'webdriver', { get: () => undefined }) + } catch {} + Object.defineProperty(window, 'chrome', { value: { runtime: {} }, configurable: true }) + }) + const ua = await page.browser().userAgent() + await page.setUserAgent(ua.replace('Headless', '')) + }
39-46: 避免 networkidle0 潜在长时间挂起;改用 domcontentloaded + 明确超时,并等待结果选择器Bing 页面存在长连/异步请求,networkidle0 过严易卡。建议设置合理超时并在 evaluate 前等待结果 DOM。
- await page.goto( - `https://cn.bing.com/search?form=QBRE&q=${encodeURIComponent( - query - )}`, - { - waitUntil: 'networkidle0' - } - ) + await page.goto( + `https://cn.bing.com/search?form=QBRE&q=${encodeURIComponent(query)}`, + { waitUntil: 'domcontentloaded', timeout: this.config.timeout ?? 10_000 } + ) + await page.waitForSelector('#b_results .b_algo', { timeout: this.config.timeout ?? 10_000 })packages/search-service/src/providers/duckduckgo_lite.ts (11)
1-6: 导入了新的 HTML 实体解码库添加了
html-entities库的decode函数导入,用于解码 HTML 实体。
8-16: 定义了 DuckDuckGo 搜索的默认基础 URL合理地定义了不同搜索类型的基础 URL 常量,包括 lite 版本的 URL。
21-21: VQD 正则表达式已保留但未在当前实现中使用代码保留了 VQD 提取的正则表达式,这在某些 DuckDuckGo 搜索场景中可能会用到,但当前的 lite 搜索实现并未实际使用它。
24-40: 新增了实例属性和通用请求头设置添加了
useLite、searchType等配置属性以及通用的请求头设置,为不同搜索模式做准备。User-Agent 设置合理,有助于避免被服务端识别为机器人。
42-58: 简化了主搜索方法的实现主搜索方法现在专注于参数验证和错误处理,将具体搜索逻辑委托给
performLiteSearch方法。这种设计提高了代码的可读性和可维护性。
63-92: 实现了 lite 搜索的具体逻辑使用了正确的 DuckDuckGo lite 搜索 URL 格式,基于搜索结果可知 lite.duckduckgo.com/lite 是有效的搜索端点。
dc参数用于指定结果数量,q参数传递查询字符串,这符合 DuckDuckGo 的 API 参数规范。
103-109: ** 正则表达式的跨行匹配问题**当前的正则表达式可能无法正确处理跨行的内容,特别是在 HTML 结构复杂的情况下。
169-183: URL 清理和重定向处理逻辑URL 清理逻辑正确处理了 DuckDuckGo 的重定向 URL 格式(
/l/?uddg=),这是一个很好的实现细节。
188-197: HTML 实体解码和文本标准化使用
html-entities库进行 HTML 实体解码是正确的做法,能够正确处理搜索结果中的特殊字符。
202-211: URL 标准化逻辑URL 标准化逻辑处理了协议缺失的情况,确保所有 URL 都有有效的 HTTP/HTTPS 前缀。
238-240: 提供者注册逻辑提供者的注册逻辑简洁明了,正确检查配置并添加到搜索管理器中。
packages/core/src/llm-core/chat/app.ts (3)
27-27: 引入 ToolMessage 合理,和后续并行工具调用写入历史的实现一致。
保持与 LangChain 消息模型对齐,有助于工具调用轨迹回放。
137-143: 仅保留 content 与 additional_kwargs 的显示消息构造与既有约定一致。
与“displayResponse 仅需 content 和 additional_kwargs”的团队约定一致,避免在显示层混入工具元数据。
32-32: 确认:AgentStep 导入路径正确(无需更改)packages/core/src/llm-core/agent/types.ts 定义了 AgentStep,且 packages/core/src/llm-core/agent/index.ts 包含
export * from './types',因此在 packages/core/src/llm-core/chat/app.ts 从../agent导入是有效的。
…d config - Fix handleChatError method signature to accept undefined wrapper - Improve tool message handling logic in chat processing - Add null safety to Bing web search element selectors - Remove deprecated freeSearchBaseURL configuration - Filter out empty search results in Bing provider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/search-service/src/index.ts (1)
106-109: 空指针风险:keywordExtractModel 可能为 null此处直接访问
.value会在summaryModel==='empty'时抛异常;与上面的browserModelRef用法不一致。- const summaryModel = computed( - () => keywordExtractModel.value ?? params.model - ) + const summaryModel = computed( + () => keywordExtractModel?.value ?? params.model + )
🧹 Nitpick comments (5)
packages/search-service/src/providers/bing_web.ts (2)
39-45: goto 缺少显式超时且域名/地区硬编码;接入超时并用 mkt 对齐地区
- 接入
this.config.puppeteerTimeout。- 用
mkt=...而非固定cn.bing.com,复用bingSearchLocation。- await page.goto( - `https://cn.bing.com/search?form=QBRE&q=${encodeURIComponent( - query - )}`, - { - waitUntil: 'networkidle0' - } - ) + await page.goto( + `https://www.bing.com/search?form=QBRE&mkt=${encodeURIComponent(this.config.bingSearchLocation ?? 'zh-CN')}&q=${encodeURIComponent(query)}`, + { + waitUntil: 'networkidle0', + timeout: this.config.puppeteerTimeout + } + )
48-68: 提取逻辑可再稳一点:等待结果选择器;标题/描述 trim;图片补充 data-src 回退避免空列表和脏数据,提升解析稳健性。
- const summaries = await page.evaluate(() => { - const liElements = Array.from( - document.querySelectorAll('#b_results > .b_algo') - ) + const summaries = await page.evaluate(() => { + const liElements = Array.from( + document.querySelectorAll('#b_results .b_algo') + ) return liElements .map((li) => { - const abstractElement = - li.querySelector('.b_caption > p') + const abstractElement = li.querySelector('.b_caption > p') const linkElement = li.querySelector('a') const imageElement = li.querySelector('img') - const href = linkElement?.getAttribute('href') ?? '' - const title = linkElement?.textContent ?? '' - const description = abstractElement?.textContent ?? '' - const image = imageElement?.getAttribute('src') ?? '' + const href = linkElement?.getAttribute('href') ?? '' + const title = (linkElement?.textContent ?? '').trim() + const description = (abstractElement?.textContent ?? '').trim() + const image = + imageElement?.getAttribute('src') ?? + imageElement?.getAttribute('data-src') ?? + '' return { url: href, title, description, image } }) .filter((summary) => summary.url && summary.title) })在 goto 之后、evaluate 之前补充:
await page.waitForSelector('#b_results .b_algo', { timeout: this.config.puppeteerTimeout })packages/search-service/src/index.ts (1)
196-233: 为“最小隐身”开关补充配置项(与 bing_web.ts 建议配套)建议将隐身策略置于可配置项,默认关闭。
export const Config: Schema<Config> = Schema.intersect([ ChatLunaPlugin.Config, Schema.object({ @@ - puppeteerTimeout: Schema.number().default(60000), - puppeteerIdleTimeout: Schema.number().default(300000), + puppeteerTimeout: Schema.number().default(60000), + puppeteerIdleTimeout: Schema.number().default(300000), + puppeteerStealth: Schema.boolean().default(false),并在
export interface Config中同步:puppeteerTimeout: number puppeteerIdleTimeout: number + puppeteerStealth?: booleanpackages/core/src/llm-core/chat/app.ts (2)
53-66: ****根据过往评审记录,您已表示不希望修改此部分。解决方案是在使用前明确检查值或将值作为必需参数传递给函数,确保类型安全并防止运行时错误。当前实现在
arg.variables可能为undefined时直接传递给需要ChainValues类型的事件,可能存在类型不匹配问题。
82-89: ****此处存在变量未初始化就使用的潜在问题。虽然您之前表示此修改"不行",但TypeScript 的严格空值检查模式防止您在代码中引入不需要的行为,比如那个臭名昭著的 TypeError Cannot read properties of undefined。
当
createChatLunaLLMChainWrapper()抛出异常时,wrapper变量未被赋值就传递给handleChatError,这会导致运行时问题。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/search-service/src/locales/en-US.schema.ymlis excluded by!**/*.ymlpackages/search-service/src/locales/zh-CN.schema.ymlis excluded by!**/*.yml
📒 Files selected for processing (3)
packages/core/src/llm-core/chat/app.ts(6 hunks)packages/search-service/src/index.ts(3 hunks)packages/search-service/src/providers/bing_web.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-17T00:25:27.195Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#548
File: packages/core/src/llm-core/chat/app.ts:0-0
Timestamp: 2025-09-17T00:25:27.195Z
Learning: 在 ChatInterface 类中,响应式 watch 调用通过 ctx.effect() 包装来自动处理清理工作,避免内存泄漏。字段 _chain 和 _embeddings 的类型已更新为可空类型 (| undefined),并添加 ctx.on('dispose') 处理器提供额外的清理保障。这种使用 Koishi effect 系统的方式比手动管理 stop 句柄更优雅。
Applied to files:
packages/core/src/llm-core/chat/app.ts
📚 Learning: 2025-09-17T00:25:27.195Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#548
File: packages/core/src/llm-core/chat/app.ts:0-0
Timestamp: 2025-09-17T00:25:27.195Z
Learning: 在 ChatInterface 类中,响应式 watch 调用已经通过 ctx.effect() 包装来自动处理清理工作,避免内存泄漏。字段 _chain 和 _embeddings 的类型已更新为可空类型。
Applied to files:
packages/core/src/llm-core/chat/app.ts
📚 Learning: 2025-09-15T17:40:57.183Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#546
File: packages/core/src/llm-core/chat/app.ts:114-119
Timestamp: 2025-09-15T17:40:57.183Z
Learning: 在 ChatLuna 的 packages/core/src/llm-core/chat/app.ts 中,displayResponse 的构造只需要保留 content 和 additional_kwargs,tool_calls、tool_call_id、name、id 等元数据在代码库的其他地方处理,无需在 displayResponse 中保留这些字段。
Applied to files:
packages/core/src/llm-core/chat/app.ts
🧬 Code graph analysis (1)
packages/core/src/llm-core/chat/app.ts (2)
packages/core/src/llm-core/chain/base.ts (1)
ChatLunaLLMCallArg(37-48)packages/core/src/llm-core/agent/types.ts (1)
AgentStep(250-253)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (12)
packages/search-service/src/providers/bing_web.ts (2)
1-1: 移除全局 ESLint 禁令,消除对 proto 的依赖文件级
/* eslint-disable no-proto */不再必要,应通过删除对navigator.__proto__的操作来避免放宽规则。-/* eslint-disable no-proto */
70-73: 页面生命周期处理已正确用 finally 保障关闭保证不泄漏 page 资源,👍。
packages/search-service/src/index.ts (1)
214-214: 默认搜索引擎改为 bing-web:LGTM与迁移方向一致;请同步用户文档与配置示例。
packages/core/src/llm-core/chat/app.ts (9)
27-27: ****此导入将用于处理工具调用历史,新增的导入是正确的。
32-32: ****新增 AgentStep 导入用于处理并行中间步骤,这是必要的。
105-116: 验证变量初始化的安全性当前代码在 Line 108 进行了
arg.variables = arg.variables ?? {}的空值合并初始化,这是一个良好的防护措施。Line 110-113 的postHandler.variables遍历也被正确防护。
130-135: ****根据过往评审,您已明确表示"无需"修改 maxToken 的处理方式。当前实现会始终使用预设配置的 maxToken 值。
137-137: 引入响应消息变量用于后续处理正确提取
responseMessage变量,这为后续的显示响应和历史记录处理提供了清晰的分离。
139-143: 改进显示响应的构建现在从
responseMessage中提取content和additional_kwargs,这确保了显示响应保持必要的消息信息。
164-167: 合理的历史记录保存逻辑根据
rawOnCensor配置选择保存原始响应还是显示响应是合理的设计。
531-538: ****根据过往评审记录,您已表示"无需"修改事件签名中 chain 参数的可选性。
169-204: 请手动确认 AIMessage 构造函数的tool_calls参数签名
当前 grep 仅在本仓库中定位到数据库保存时对tool_calls的引用,未能获知 LangChain 中AIMessage对tool_calls结构的具体定义,请检查你本地或项目中安装的 LangChain 版本文档,确保所需字段(如可能的type)已正确提供。
- Change tool name filters from hyphen to underscore format - Fixes tool registration to match actual tool names
这个 PR 修复了核心服务中的错误处理和生命周期管理问题,提升了系统的稳定性和可靠性。
新功能
无
Bug 修复
其他更改