fix: 清理 usage-doc / big-screen i18n 硬编码 + 修复 Next.js params Promise 报错#595
fix: 清理 usage-doc / big-screen i18n 硬编码 + 修复 Next.js params Promise 报错#595YangQing-Lin wants to merge 14 commits intoding113:devfrom
Conversation
补充 NEXT_PHASE === "phase-production-build" 检查 Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
📝 WalkthroughWalkthrough本PR为Next.js引入了对边缘运行时的修复,防止构建期间出现process.once警告。核心改动包括:AsyncTaskManager添加延迟初始化逻辑在Edge环境下跳过初始化,cloud-price-updater将静态导入改为动态导入并添加Edge运行时短路处理,配合全面的单元测试覆盖。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @YangQing-Lin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求主要解决了 Next.js 应用在 Edge Runtime 环境下因使用 Node.js 专属 API ( Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
这个 PR 的目标是解决在 Edge Runtime 中构建时出现的 process.once API 不支持的告警。您通过在 AsyncTaskManager 中实现懒加载初始化,并在 cloud-price-updater 中使用动态导入,成功地避免了在 Edge 环境中打包和执行 Node.js 专用的 API,这是一个非常清晰且有效的解决方案。
代码质量很高,主要变更点都伴随着全面且健壮的单元测试,这确保了新功能在 Edge 和 Node.js 环境下都能按预期工作,同时也保证了原有功能不受影响。此外,您还添加了详细的文档(docs/edge-runtime-process-once.md)来解释问题的背景、解决方案和验证步骤,这是非常好的实践,极大地提升了代码库的可维护性。
整体来看,这是一个高质量的修复,代码逻辑严谨,测试覆盖充分,文档清晰。我只发现一个可以简化测试代码的小建议。
请注意:本次审查是基于您提交的代码变更(Patch)进行的。PR 的标题和描述似乎与代码变更的内容不符(描述涉及 i18n 和 Next.js params 修复,而代码是关于 Edge Runtime 的修复)。建议您更新一下 PR 的描述以保持同步。
| const prevRuntime = process.env.NEXT_RUNTIME; | ||
| process.env.NEXT_RUNTIME = "edge"; | ||
|
|
||
| const { requestCloudPriceTableSync } = await import("@/lib/price-sync/cloud-price-updater"); | ||
| requestCloudPriceTableSync({ reason: "missing-model", throttleMs: 0 }); | ||
| await flushAsync(); | ||
|
|
||
| expect(asyncTaskManagerLoaded).toBe(false); | ||
|
|
||
| process.env.NEXT_RUNTIME = prevRuntime; |
There was a problem hiding this comment.
为了提升测试代码的简洁性和可维护性,可以移除此处手动的 process.env.NEXT_RUNTIME 保存和恢复逻辑。外层的 describe 块已经通过 beforeEach 和 afterEach hook 对该环境变量进行了管理,afterEach 会在测试结束后自动恢复其原始值。移除冗余代码可以让测试意图更清晰。
| const prevRuntime = process.env.NEXT_RUNTIME; | |
| process.env.NEXT_RUNTIME = "edge"; | |
| const { requestCloudPriceTableSync } = await import("@/lib/price-sync/cloud-price-updater"); | |
| requestCloudPriceTableSync({ reason: "missing-model", throttleMs: 0 }); | |
| await flushAsync(); | |
| expect(asyncTaskManagerLoaded).toBe(false); | |
| process.env.NEXT_RUNTIME = prevRuntime; | |
| process.env.NEXT_RUNTIME = "edge"; | |
| const { requestCloudPriceTableSync } = await import("@/lib/price-sync/cloud-price-updater"); | |
| requestCloudPriceTableSync({ reason: "missing-model", throttleMs: 0 }); | |
| await flushAsync(); | |
| expect(asyncTaskManagerLoaded).toBe(false); |
Additional Comments (1)
The
If the throttle check passes initially (line 74) but then the scheduling flag is set (line 82), and then a second call comes in that hits the throttle check (line 74-76), the first call's flag will remain set but its IIFE never completes, permanently blocking all future sync attempts. Scenario:
The correct fix would be to move the scheduling flag check BEFORE the throttle check, or use a more sophisticated locking mechanism. Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/price-sync/cloud-price-updater.ts
Line: 78:82
Comment:
[P0] **Scheduling flag never cleared on early returns, causing permanent lockout**
The `__CCH_CLOUD_PRICE_SYNC_SCHEDULING__` flag is set to `true` at line 82 but is only cleared in the `finally` block of the async IIFE (line 126). However, there are two early returns before the IIFE starts:
1. Line 74-76: Throttle check returns early
2. Line 79-81: Duplicate scheduling flag check returns early
If the throttle check passes initially (line 74) but then the scheduling flag is set (line 82), and then a *second* call comes in that hits the throttle check (line 74-76), the first call's flag will remain set but its IIFE never completes, permanently blocking all future sync attempts.
**Scenario:**
- Call A: Passes throttle → sets flag → starts IIFE
- Call B (concurrent): Passes throttle (before A updates timestamp) → sees flag set → returns early
- Call C (later): **Hits throttle check** → returns early **without clearing the flag**
- **All future calls are now blocked forever**
```suggestion
// 避免并发请求在 AsyncTaskManager 加载前重复触发(例如多请求同时命中 missing-model)
if (g.__CCH_CLOUD_PRICE_SYNC_SCHEDULING__) {
return;
}
// NOTE: Flag must be cleared in the finally block starting at line 125
// Do NOT add any early returns after this point without moving flag to separate scope
g.__CCH_CLOUD_PRICE_SYNC_SCHEDULING__ = true;
```
The correct fix would be to move the scheduling flag check BEFORE the throttle check, or use a more sophisticated locking mechanism.
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/async-task-manager.ts (1)
29-49: 延迟初始化逻辑实现良好环境守卫逻辑正确地跳过了 Edge/CI/生产构建环境下的 Node-only API 调用。不过,debug 日志中记录了
nextRuntime和ci,但遗漏了同样参与判断的NEXT_PHASE。建议在 debug 日志中补充 NEXT_PHASE
logger.debug("[AsyncTaskManager] Skipping initialization in edge/CI environment", { nextRuntime: process.env.NEXT_RUNTIME, ci: process.env.CI, + nextPhase: process.env.NEXT_PHASE, });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (5)
docs/edge-runtime-process-once.mdsrc/lib/async-task-manager.tssrc/lib/price-sync/cloud-price-updater.tstests/unit/lib/async-task-manager-edge-runtime.test.tstests/unit/price-sync/cloud-price-updater.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use emoji characters in any code, comments, or string literals
Files:
tests/unit/price-sync/cloud-price-updater.test.tstests/unit/lib/async-task-manager-edge-runtime.test.tssrc/lib/async-task-manager.tssrc/lib/price-sync/cloud-price-updater.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All new features must have unit test coverage of at least 80%
Files:
tests/unit/price-sync/cloud-price-updater.test.tstests/unit/lib/async-task-manager-edge-runtime.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text
Use path alias@/to reference files in./src/directory
Format code with Biome using: double quotes, trailing commas, 2-space indent, 100 character line width
Files:
tests/unit/price-sync/cloud-price-updater.test.tstests/unit/lib/async-task-manager-edge-runtime.test.tssrc/lib/async-task-manager.tssrc/lib/price-sync/cloud-price-updater.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer named exports over default exports
Files:
tests/unit/price-sync/cloud-price-updater.test.tstests/unit/lib/async-task-manager-edge-runtime.test.tssrc/lib/async-task-manager.tssrc/lib/price-sync/cloud-price-updater.ts
tests/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for unit testing and happy-dom for DOM testing
Files:
tests/unit/price-sync/cloud-price-updater.test.tstests/unit/lib/async-task-manager-edge-runtime.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 573
File: src/actions/model-prices.ts:275-335
Timestamp: 2026-01-10T06:20:04.478Z
Learning: In the `ding113/claude-code-hub` repository, Server Actions (files under `src/actions/*.ts`) currently return hardcoded Chinese error messages directly. This is a codebase-wide architectural decision that applies to all action files (e.g., model-prices.ts, users.ts, system-config.ts). Changing this pattern requires a coordinated i18n refactor across all Server Actions, which should be handled as a separate unified task rather than piecemeal changes in individual PRs.
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T17:53:25.066Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text
📚 Learning: 2026-01-10T17:53:25.066Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T17:53:25.066Z
Learning: Applies to tests/**/*.test.{ts,tsx,js,jsx} : Use Vitest for unit testing and happy-dom for DOM testing
Applied to files:
tests/unit/price-sync/cloud-price-updater.test.tstests/unit/lib/async-task-manager-edge-runtime.test.ts
📚 Learning: 2026-01-05T03:01:39.354Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/types/user.ts:158-170
Timestamp: 2026-01-05T03:01:39.354Z
Learning: In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined. Example: for numeric limits like limitTotalUsd, use 'number | null | undefined' when null signifies explicitly unlimited (e.g., matches DB schema or special UI logic) and undefined signifies 'inherit default'. This pattern should be consistently reflected in type definitions across related fields to preserve semantic clarity between database constraints and UI behavior.
Applied to files:
src/lib/async-task-manager.tssrc/lib/price-sync/cloud-price-updater.ts
🧬 Code graph analysis (4)
tests/unit/price-sync/cloud-price-updater.test.ts (3)
src/actions/model-prices.ts (1)
processPriceTableInternal(75-195)src/lib/price-sync/cloud-price-updater.ts (2)
syncCloudPriceTableToDatabase(16-46)requestCloudPriceTableSync(56-129)src/lib/logger.ts (1)
logger(168-187)
tests/unit/lib/async-task-manager-edge-runtime.test.ts (3)
src/lib/async-task-manager.ts (1)
AsyncTaskManager(248-249)src/lib/logger.ts (1)
logger(168-187)src/app/v1/_lib/proxy/errors.ts (1)
isClientAbortError(617-636)
src/lib/async-task-manager.ts (1)
src/lib/logger.ts (1)
logger(168-187)
src/lib/price-sync/cloud-price-updater.ts (2)
src/lib/async-task-manager.ts (1)
AsyncTaskManager(248-249)src/lib/logger.ts (1)
logger(168-187)
⏰ 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). (6)
- GitHub Check: 🌐 API Tests
- GitHub Check: Docker Build Test
- GitHub Check: pr-label
- GitHub Check: pr-review
- GitHub Check: check-codex-status
- GitHub Check: pr-description
🔇 Additional comments (17)
src/lib/async-task-manager.ts (3)
51-69: 退出信号处理和定时清理逻辑正确使用
process.once避免热重载时重复注册,定时清理间隔正确存储以便后续清理。
79-141: 任务注册逻辑完善
register()方法在开始时调用initializeIfNeeded()确保延迟初始化,正确处理重复任务取消、错误分类日志和自动清理。
246-249: 单例模式实现正确使用
globalThis缓存避免热重载时重复实例化,符合 Next.js 开发环境最佳实践。docs/edge-runtime-process-once.md (1)
1-73: 文档内容完整且结构清晰文档详细说明了 Edge Runtime
process.once问题的背景、修复方案、验证步骤和回滚指南。快速定位命令有助于防止文档与代码漂移。tests/unit/lib/async-task-manager-edge-runtime.test.ts (6)
1-16: Mock 设置正确Logger 和错误处理工具的 mock 配置完整,覆盖了所有需要的日志级别。
17-43: 测试隔离设置完善正确保存和恢复环境变量,每次测试前重置模块和 mock,清理
globalThis上的单例缓存,确保测试间相互隔离。
45-68: Edge/Node.js 运行时行为测试充分准确验证了在 Edge 运行时跳过
process.once调用,在 Node.js 运行时正确注册三个退出信号处理器。
70-117: 退出信号和定时清理测试覆盖全面验证了 SIGTERM、SIGINT、beforeExit 信号处理器正确触发清理,以及定时器正确调用
cleanupCompletedTasks。
119-237: 任务生命周期和错误处理测试完整覆盖了任务自动清理、未知任务取消、重复任务处理、客户端中止错误与普通错误的日志分支。
239-307: 清理行为测试设计合理正确验证了超时任务(>10分钟)被取消,以及
cleanupAll正确清理所有任务和定时器。通过类型断言访问私有方法在测试中是可接受的做法。src/lib/price-sync/cloud-price-updater.ts (3)
60-76: Edge 运行时守卫和节流逻辑正确Edge 运行时提前返回避免加载 Node-only 模块,节流逻辑使用
globalThis存储上次同步时间戳。
78-128: 调度守卫和异步任务注册逻辑正确调度标志
__CCH_CLOUD_PRICE_SYNC_SCHEDULING__正确防止并发调度,动态导入AsyncTaskManager避免 Edge 运行时静态分析问题,finally块确保标志正确重置。任务去重检查和错误处理都很完善。
16-46: syncCloudPriceTableToDatabase 实现健壮函数结构清晰,错误处理覆盖所有路径(获取失败、解析失败、写入失败)。动态导入
processPriceTableInternal与整体延迟加载策略一致。tests/unit/price-sync/cloud-price-updater.test.ts (4)
1-46: Mock 设置和辅助函数设计合理
asyncTaskManagerLoaded标志配合vi.resetModules()正确追踪模块加载状态,flushAsync辅助函数用于等待异步 IIFE 执行完成。
48-241: syncCloudPriceTableToDatabase 测试覆盖全面测试覆盖了 HTTP 错误、空响应、TOML 解析失败、写入失败(含默认错误消息)、Error 和非 Error 类型异常,以及成功路径。
243-434: requestCloudPriceTableSync 测试设计良好测试充分覆盖了 Edge 运行时短路、重复任务检测、节流(默认和自定义)、调度标志守卫、调度失败处理,以及任务完成后的时间戳更新和日志记录。
268-279: Edge 运行时测试隔离正确测试内部保存和恢复
NEXT_RUNTIME确保不影响其他测试的环境设置。
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR based on a diff-scoped review of the Edge runtime warning fix (static import removal + lazy initialization) and the added unit tests.
PR Size: L
- Lines changed: 720
- Files changed: 5
- Split suggestions: Consider splitting into (1)
src/lib/async-task-manager.tslazy init, (2)src/lib/price-sync/cloud-price-updater.tsEdge no-op + dynamic import, (3) tests + docs.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None identified.
High Priority Issues (Should Fix)
None identified.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Identified the active PR as
#595(fix/edge-runtime-process-once→dev) and pulled the full diff for review. - Calculated PR size as L (
664additions +56deletions =720lines;5files) and applied the labelsize/L. - Completed a diff-scoped, multi-perspective review of the changed files and submitted a PR review summary via
gh pr review(no inline comments posted because no reportable issues met the confidence threshold).
There was a problem hiding this comment.
Code Review Summary
This PR implements a two-layer defense to fix Edge Runtime build warnings caused by process.once Node.js API usage. The changes convert AsyncTaskManager from constructor-based to lazy initialization and make cloud-price-updater use dynamic imports. The implementation is sound and follows existing patterns in the codebase.
Note: PR #589 with the same fix was already merged on 2026-01-11. This PR may be a duplicate.
PR Size: L
- Lines changed: 720 (664 additions, 56 deletions)
- Files changed: 5
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Analysis Notes
Validated and Cleared:
-
Race condition in
initializeIfNeeded(): Not a bug. JavaScript is single-threaded, and the synchronous flag check-and-set pattern is atomic within the event loop. Theinitializedflag is set immediately before any async work begins. -
Scheduling flag management: Correctly implemented. Early returns (Edge runtime check, throttle check) occur before the flag is set. The
finallyblock properly resets the flag after the async IIFE completes. -
Error handling in dynamic import: Adequate. Errors are caught and logged with context (reason and error message). The
finallyblock ensures the scheduling flag is always reset. -
Duplicate task check timing: The check moved inside the async IIFE, but this is functionally equivalent because:
- The
__CCH_CLOUD_PRICE_SYNC_SCHEDULING__flag prevents concurrent scheduling AsyncTaskManager.register()already handles duplicates by canceling old tasks (line 67-73)
- The
-
Test coverage: New tests in
async-task-manager-edge-runtime.test.tsand updates tocloud-price-updater.test.tsappear comprehensive for the changed behavior.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean (follows existing patterns)
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Claude AI
|
提PR的时候没注意,选错分支了,这个分支的变更已经在昨天被合并: PR: 修复 Edge Runtime 下 process.once 构建告警(AsyncTaskManager 导入链) |
Summary
Fix Edge Runtime build warnings caused by
process.onceNode.js API usage inAsyncTaskManagerimport chain. The fix skips initialization in Edge/CI/build environments and uses dynamic imports to prevent static bundling.Note: This PR appears to be a duplicate of #589 which was already merged. Consider closing this PR.
Related Issues:
Problem
During
next build(Turbopack/Next), Edge Runtime warnings appear becauseprocess.onceis a Node.js API not supported in Edge Runtime.The import trace shows:
src/lib/async-task-manager.tssrc/lib/price-sync/cloud-price-updater.tssrc/instrumentation.tsRoot cause:
AsyncTaskManagerregistersprocess.once(...)exit signal hooks during module initialization, which gets pulled into the Edge Instrumentation bundle via the dependency chain.Solution
Two-layer defense to prevent Edge bundle from seeing
process.once:1) AsyncTaskManager: Lazy initialization with environment guards
initializeIfNeeded())NEXT_RUNTIME === "edge",CI === "true", orNEXT_PHASE === "phase-production-build"process.once,setInterval) until firstregister()call2) cloud-price-updater: Dynamic import + Edge runtime no-op
AsyncTaskManagerto prevent bundler from pulling it into Edge bundleimport()inside async function, only in Node.js runtimeNEXT_RUNTIME === "edge"Changes
Core Changes
src/lib/async-task-manager.ts- Lazy initialization with environment guards (+20/-4)src/lib/price-sync/cloud-price-updater.ts- Dynamic import with Edge runtime no-op (+57/-32)Supporting Changes
tests/unit/lib/async-task-manager-edge-runtime.test.ts- Edge runtime test coverage (+308 lines)tests/unit/price-sync/cloud-price-updater.test.ts- Updated tests for dynamic import behavior (+206/-20)docs/edge-runtime-process-once.md- Documentation for the fix (+73 lines)Testing
Automated Tests
Verification
bun run build- No longer produces Edge Runtimeprocess.oncewarningsbun run lint- Passesbun run typecheck- Passesbun run test- PassesChecklist
Description enhanced by Claude AI