fix(k8s): 修正 fetch 超时配置单位#1284
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough在 ChangesFetch 超时配置更新
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Kubernetes deployment configuration by setting explicit millisecond values for fetch timeout environment variables (FETCH_CONNECT_TIMEOUT, FETCH_HEADERS_TIMEOUT, and FETCH_BODY_TIMEOUT). The reviewer points out that these values duplicate the default configurations in the application schema, creating redundancy and a risk of configuration drift. Additionally, the reviewer notes that the added comment incorrectly references .env.example, where these variables are not present, and suggests keeping these environment variables commented out in the deployment manifest unless customization is required.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # Fetch 超时单位为毫秒,需与 .env.example 和 env.schema.ts 保持一致。 | ||
| - name: FETCH_CONNECT_TIMEOUT | ||
| value: "30" | ||
| #- name: FETCH_HEADERS_TIMEOUT | ||
| # value: "600" | ||
| #- name: FETCH_BODY_TIMEOUT | ||
| # value: "600" | ||
| value: "30000" | ||
| - name: FETCH_HEADERS_TIMEOUT | ||
| value: "600000" | ||
| - name: FETCH_BODY_TIMEOUT | ||
| value: "600000" |
There was a problem hiding this comment.
问题分析
- 配置冗余与漂移风险：这些环境变量的值（
30000、600000、600000）与src/lib/config/env.schema.ts中定义的默认值完全一致。在 Kubernetes 部署中显式设置这些默认值会导致配置冗余。如果未来在代码中调整了默认超时时间，而忘记同步更新 Kubernetes 清单，就会导致配置漂移。 - 注释描述不准确：注释中提到“需与
.env.example保持一致”，但实际上deploy/.env.example文件中并没有包含这些FETCH_*相关的环境变量。
改进建议
建议将这些配置项注释掉，仅作为自定义参考保留。这样既能利用代码中的默认值，避免配置漂移，又能在需要自定义超时时间时方便运维人员修改。同时修正注释中关于 .env.example 的不准确描述。
# Fetch 超时配置（单位为毫秒）。默认值已在 env.schema.ts 中定义，此处注释以供参考或自定义。
# - name: FETCH_CONNECT_TIMEOUT
# value: "30000"
# - name: FETCH_HEADERS_TIMEOUT
# value: "600000"
# - name: FETCH_BODY_TIMEOUT
# value: "600000"
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR fixes a critical configuration bug in the Kubernetes deployment where fetch timeout values were incorrectly specified in seconds instead of milliseconds, causing 503 errors. The fix correctly updates the values to match the expected milliseconds unit used in .env.example.
PR Size: XS
- Lines changed: 11 (6 additions, 5 deletions)
- Files changed: 1 (
deploy/k8s/app/deployment.yaml)
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 |
Review Notes
Process Note (Non-blocking): This PR targets main branch, while CLAUDE.md specifies that all PRs should target dev. Given this is a hotfix for a production issue (503 errors), targeting main may be intentional. Consider whether this should merge to dev first per the standard workflow.
Configuration Validation:
FETCH_CONNECT_TIMEOUT: 30 → 30000 ✓ (matches.env.example)FETCH_HEADERS_TIMEOUT: 600 → 600000 ✓ (matches.env.example)FETCH_BODY_TIMEOUT: 600 → 600000 ✓ (matches.env.example)
Comment Quality: The added comment accurately documents the millisecond unit requirement and cross-references related files. This is good practice for configuration files.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - N/A (config change)
- Type safety - N/A (config change)
- Documentation accuracy - Clean
- Test coverage - N/A (config change)
- Code clarity - Good
Automated review by Claude AI
1 participant
Summary
Corrects the timeout configuration units in the K8s deployment manifest to match the application's millisecond-based expectations (undici interprets all timeout values in milliseconds).
Problem
The K8s deployment manifest configured undici timeout values incorrectly:
FETCH_CONNECT_TIMEOUT:"30"(interpreted as 30ms instead of 30000ms = 30s)FETCH_HEADERS_TIMEOUTandFETCH_BODY_TIMEOUT: were commented out in Update claude-code-hub k8s app deployment.yaml #1174 with a note to fixFETCH_CONNECT_TIMEOUTseparatelyThis caused a high probability of 503 errors in K8s deployments because connections would timeout before upstream providers could respond.
Related Issues/PRs:
FETCH_HEADERS_TIMEOUTandFETCH_BODY_TIMEOUTand noted thatFETCH_CONNECT_TIMEOUTneeded a follow-up fix (was set to"30"instead of"30000").Solution
Updates
deploy/k8s/app/deployment.yamlto use correct millisecond-based values that match.env.exampleandsrc/lib/config/env.schema.ts:FETCH_CONNECT_TIMEOUT:"30"→"30000"(30 seconds in milliseconds)FETCH_HEADERS_TIMEOUT: uncommented with"600000"(600 seconds in milliseconds)FETCH_BODY_TIMEOUT: uncommented with"600000"(600 seconds in milliseconds)Changes
Core Changes
deploy/k8s/app/deployment.yaml— Fixed all three fetch timeout environment variables to use correct millisecond valuesTesting
Manual Testing
Checklist
Description enhanced by Claude AI