fix: 同步功能 ERR_CONNECTION_RESET 修复#144
Conversation
…uning - Add fetchWithRetry() with exponential backoff (1s/2s/4s, max 3 retries) - Handles: ERR_CONNECTION_RESET, ERR_CONNECTION_CLOSED, AbortError, Failed to fetch, NetworkError - Increase sync operation timeout from 30s to 120s - Increase nginx proxy_read/send_timeout from 120s to 300s - Add proxy_connect_timeout 30s and proxy_buffering off - Increase client_max_body_size from 50m to 100m Fixes #133
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a private exponential-backoff retry helper to BackendAdapter, uses it for repository and release sync/fetch calls with 120s timeouts and 3 retries, and updates nginx /api/ proxy settings to increase timeouts, disable buffering, and raise client_max_body_size to 100m. ChangesBackend Resilience with Retry Logic and Timeout Adjustments
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/services/backendAdapter.ts (1)
270-310: ⚡ Quick winConsider applying retry logic to other sync operations for consistency.
The AI config, WebDAV config, and settings sync/fetch operations still use
fetchWithTimeoutwithout retry logic. While the PR scope targets repository/release sync (per issue#133), these operations might also benefit from the same resilience pattern, especially in unstable network conditions.Example: Applying retry to syncAIConfigs
async syncAIConfigs(configs: AIConfig[]): Promise<void> { if (!this._backendUrl) return; - const res = await this.fetchWithTimeout(`${this._backendUrl}/configs/ai/bulk`, { + const res = await this.fetchWithRetry(`${this._backendUrl}/configs/ai/bulk`, { method: 'PUT', headers: this.getAuthHeaders(), body: JSON.stringify({ configs }) - }); + }, 30000, 3); if (!res.ok) await this.throwTranslatedError(res, 'Sync AI configs error'); }Similar changes could be applied to
fetchAIConfigs,syncWebDAVConfigs,fetchWebDAVConfigs,syncSettings, andfetchSettings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/backendAdapter.ts` around lines 270 - 310, Add the same retry/resilience pattern used for repository/release sync to the remaining config/settings operations: update syncAIConfigs, fetchAIConfigs, syncWebDAVConfigs, fetchWebDAVConfigs (and similarly syncSettings/fetchSettings) to call the existing retry wrapper used elsewhere (e.g., the fetch-with-retry helper around fetchWithTimeout) instead of invoking fetchWithTimeout directly; ensure you preserve the same request options (method/headers/body) and error handling (throwTranslatedError) when replacing the direct fetchWithTimeout calls so behavior and messages remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/backendAdapter.ts`:
- Line 104: The throw of lastError with a non-null assertion is unreachable
because the loop already throws when attempt === maxRetries; remove the
redundant line "throw lastError!" (and the unnecessary non-null assertion) from
the end of the retry loop in the function that uses attempt, maxRetries and
lastError (e.g., the retry block in backendAdapter.ts) so control flow is clear
and no unreachable statement remains.
- Around line 91-96: The current isRetryable check in backendAdapter.ts only
inspects lastError.message and misses Node.js undici codes and Safari messages;
update the isRetryable logic used where lastError is evaluated to also check
lastError.cause?.code (e.g., matching /ERR_CONNECTION_/) and
lastError.cause?.errno, and add additional message checks such as 'Load failed'
plus broader TypeError patterns like 'NetworkError'/'Failed to fetch' to cover
Firefox/Chrome/Safari; ensure you reference and update the isRetryable
computation that reads lastError (and any callers) so transient network errors
from both browser and Node runtimes trigger retries.
---
Nitpick comments:
In `@src/services/backendAdapter.ts`:
- Around line 270-310: Add the same retry/resilience pattern used for
repository/release sync to the remaining config/settings operations: update
syncAIConfigs, fetchAIConfigs, syncWebDAVConfigs, fetchWebDAVConfigs (and
similarly syncSettings/fetchSettings) to call the existing retry wrapper used
elsewhere (e.g., the fetch-with-retry helper around fetchWithTimeout) instead of
invoking fetchWithTimeout directly; ensure you preserve the same request options
(method/headers/body) and error handling (throwTranslatedError) when replacing
the direct fetchWithTimeout calls so behavior and messages remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ad13016-7b56-462e-9c3b-04e269d38ab6
📒 Files selected for processing (2)
nginx.confsrc/services/backendAdapter.ts
| await new Promise(resolve => setTimeout(resolve, delay)); | ||
| } | ||
| } | ||
| throw lastError!; |
There was a problem hiding this comment.
Unreachable code: line 104 is never executed.
When attempt === maxRetries, line 97 always throws lastError inside the loop, so execution never reaches line 104. The non-null assertion is unnecessary.
♻️ Proposed fix: Remove unreachable code
await new Promise(resolve => setTimeout(resolve, delay));
}
}
- throw lastError!;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw lastError!; | |
| await new Promise(resolve => setTimeout(resolve, delay)); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/backendAdapter.ts` at line 104, The throw of lastError with a
non-null assertion is unreachable because the loop already throws when attempt
=== maxRetries; remove the redundant line "throw lastError!" (and the
unnecessary non-null assertion) from the end of the retry loop in the function
that uses attempt, maxRetries and lastError (e.g., the retry block in
backendAdapter.ts) so control flow is clear and no unreachable statement
remains.
- Remove ERR_CONNECTION_RESET/ERR_CONNECTION_CLOSED from message checks (these codes are in error.cause.code in Node.js, never in message) - Add Safari 'Load failed' message check - Add Node.js undici cause.code checks: ECONNRESET, ECONNREFUSED, UND_ERR_SOCKET, UND_ERR_CONNECT_TIMEOUT, UND_ERR_HEADERS_TIMEOUT - Add eslint-disable for unreachable throw (TS control flow limitation) Addresses CodeRabbit review comments
1 participant
问题
Docker 部署环境下，同步功能频繁出现
ERR_CONNECTION_RESET错误，导致数据同步大概率失败。(#133)根本原因
修复内容
src/services/backendAdapter.ts
nginx.conf
Summary by CodeRabbit