Skip to content

fix: 处理问题 #179#194

Open
liaoyl830 wants to merge 1 commit into
shenminglinyi:masterfrom
liaoyl830:fix/issue-179
Open

fix: 处理问题 #179#194
liaoyl830 wants to merge 1 commit into
shenminglinyi:masterfrom
liaoyl830:fix/issue-179

Conversation

@liaoyl830

@liaoyl830 liaoyl830 commented Jun 8, 2026

Copy link
Copy Markdown

Summary

  • Wait for the Tauri desktop backend /health endpoint before initApiClient() resolves
  • Apply the same readiness wait to the IPC-reported backend port and the 8005 fallback port
  • Use one shared 30s startup deadline across port polling and health polling
  • Add a fetch timeout fallback for WebView runtimes without AbortSignal.timeout

Verification

  • Runtime browser verification with Vite + Edge headless + fake Tauri IPC/backend:
    • delayed-health Tauri scenario: /health returned 200 before startup API calls; apiBeforeHealthCount: 0
    • no-AbortSignal.timeout scenario: fallback AbortController path still waited for /health; apiBeforeHealthCount: 0
    • browser/Vite mode: page rendered normally with the mock API backend
  • npm --prefix '/e/Claude Projects/PlotPilot Pr/PlotPilot/frontend' run build
  • git diff --check -- frontend/src/api/config.ts

Closes #179

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced backend connection initialization with improved timeout handling and error detection.
    • Added automatic fallback port support to improve connectivity reliability.
    • Improved health monitoring with better timeout warning messages.
  • Chores

    • Refactored initialization timing logic for more robust backend readiness detection.

@liaoyl830 liaoyl830 requested a review from shenminglinyi as a code owner June 8, 2026 17:30
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refactors Tauri desktop backend initialization in frontend/src/api/config.ts. New timeout helpers and deadline utilities replace fixed wait parameters, and a single health check is replaced with deadline-based polling to verify backend readiness before the application continues.

Changes

Tauri Backend Readiness Polling

Layer / File(s) Summary
Timing utilities and health polling helpers
frontend/src/api/config.ts
Adds internal sleep(), deadline-aware fetchWithTimeout() with AbortSignal support, and pollHealthEndpoint() that retries /health checks until a deadline or success.
Tauri startup initialization control flow
frontend/src/api/config.ts
initApiClient() now establishes a shared deadline, waits for backend port from Tauri IPC (with fallback to 8005), sets axios baseURL, and performs polling health checks that log a warning if the deadline expires without a healthy response.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A deadline clock now ticks for thee,
No more one-shot, but polling spree!
Health checks retry with patient grace,
Till backend's ready for the race. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description has content but does not follow the required template structure. It lacks the template sections (变更类型, 架构影响, 测试 with test commands, 风险说明) and includes only a summary and verification notes without the structured format expected by the repository. Restructure the description to match the template: check the fix box under 变更类型, add 1-3 sentence 变更说明, specify 架构影响 for frontend layer, include actual test commands under 测试, and add 风险说明. Provide concrete test results for 'npm run build' and health/port polling verification.
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is written in Chinese and references issue #179, but lacks a clear, concise description of the main technical change. The English summary describes significant refactoring (polling-based health checks, deadline management, timeout helpers), but the title provides no insight into these changes. Translate and clarify the title to describe the primary change in English, such as 'Refactor backend readiness checks with polling-based health verification and shared startup deadline' or similar, making it meaningful to teammates scanning the history.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
frontend/src/api/config.ts (1)

171-172: 💤 Low value

Consider logging failed health attempts at debug level.

The empty catch block makes troubleshooting startup issues harder. A debug-level log would help diagnose whether failures are network-related, server-side, or timeout-based without cluttering production logs.

💡 Suggested enhancement
     } catch {
-      // 后端端口已分配但 HTTP 服务仍在启动,继续等待
+      // 后端端口已分配但 HTTP 服务仍在启动,继续等待
+      // console.debug('[API] Health check pending, retrying...')
     } finally {
🤖 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 `@frontend/src/api/config.ts` around lines 171 - 172, The empty catch block in
the health-check retry loop in frontend/src/api/config.ts should log failures at
debug level to aid troubleshooting: inside the catch add a debug log (use the
existing logger.debug if available, otherwise console.debug) that includes the
caught error, the target URL/service name, and the retry attempt/timestamp so
transient startup/network failures are visible without polluting higher-level
logs.
🤖 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.

Nitpick comments:
In `@frontend/src/api/config.ts`:
- Around line 171-172: The empty catch block in the health-check retry loop in
frontend/src/api/config.ts should log failures at debug level to aid
troubleshooting: inside the catch add a debug log (use the existing logger.debug
if available, otherwise console.debug) that includes the caught error, the
target URL/service name, and the retry attempt/timestamp so transient
startup/network failures are visible without polluting higher-level logs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ea43a97-190a-4b9f-a9ae-4586b9cfc3f6

📥 Commits

Reviewing files that changed from the base of the PR and between 1c7df5e and 11ba4db.

📒 Files selected for processing (1)
  • frontend/src/api/config.ts

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 个并行请求失败(HTTP ?)

1 participant