fix(dev): fix frontend dev server startup race#5776
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDevelopment startup now includes a ChangesFrontend readiness
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Watcher
participant CLI
participant ToolWaitPort
participant FrontendDevServer
Watcher->>CLI: run wails3 tool waitport
CLI->>ToolWaitPort: pass host, port, and timeout
ToolWaitPort->>FrontendDevServer: poll configured port
FrontendDevServer-->>ToolWaitPort: connection result
ToolWaitPort-->>Watcher: continue or return timeout
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
This was my constant struggle while I develop Windows App. I don't know, maybe because of Windows Defender or something else, but on Windows frontend part start kind of long and I was contantly getting this race: This PR fixes my issues completely. |
![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
🤖 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 `@v3/pkg/application/application_dev.go`:
- Around line 25-26: Update the frontend readiness loop around client.Get in
application startup to enforce a request timeout, using an http.Client with a
finite Timeout or a context deadline for each request. Preserve the existing
retry behavior while ensuring an unresponsive frontend cannot block startup
indefinitely.
- Line 25: Update the retry documentation associated with the loop in
application startup to state that the server is retried a maximum of 30 times,
matching the loop’s 30 iterations.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54c051b6-72c9-4c79-b358-c53a6af957ca
📒 Files selected for processing (2)
v3/internal/commands/dev.gov3/pkg/application/application_dev.go
dc1e1d3 to
f84439e
Compare
savely-krasovsky
changed the title
|
@leaanthony could you review please? This is SO persistent on Windows, I am tired to restart |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
v3/internal/commands/watcher.go (1)
32-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a robust match for idempotency.
If a user customizes the
waitportcommand in their configuration (e.g., adding a port flag or modifying the timeout towails3 tool waitport --timeout 30), the exact string match will fail. This would cause a duplicate readiness task to be injected.Consider matching on the command prefix or using
strings.Containsto ensure idempotency even with customized arguments.♻️ Proposed refactor
for _, execute := range config.ExecStruct { - if execute.Cmd == frontendDevServerReadyCommand { + if strings.HasPrefix(execute.Cmd, "wails3 tool waitport") { return } }🤖 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 `@v3/internal/commands/watcher.go` around lines 32 - 36, Update the command comparison in the ExecStruct loop to recognize customized waitport commands, including added flags or timeout arguments, instead of requiring an exact match with frontendDevServerReadyCommand. Use a robust prefix or containment match while preserving the early return that prevents duplicate readiness-task injection.
🤖 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 `@v3/internal/commands/build_assets/config.yml`:
- Around line 59-60: The static template should no longer define the hardcoded
waitport task; remove it from v3/internal/commands/build_assets/config.yml lines
59-60 and rely on watcher.go’s dynamic injection when FRONTEND_DEVSERVER_URL is
active. Update the related assertions in v3/internal/commands/dev_config_test.go
lines 22-26 to expect three tasks and remove the explicit waitport-task match.
---
Nitpick comments:
In `@v3/internal/commands/watcher.go`:
- Around line 32-36: Update the command comparison in the ExecStruct loop to
recognize customized waitport commands, including added flags or timeout
arguments, instead of requiring an exact match with
frontendDevServerReadyCommand. Use a robust prefix or containment match while
preserving the early return that prevents duplicate readiness-task injection.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac16d6dc-5c26-4f29-a8ee-c50795729fe7
📒 Files selected for processing (10)
v3/cmd/wails3/main.gov3/internal/commands/build_assets/config.ymlv3/internal/commands/dev_config_test.gov3/internal/commands/tool_waitport.gov3/internal/commands/tool_waitport_test.gov3/internal/commands/watcher.gov3/internal/commands/watcher_test.gov3/pkg/application/application.gov3/pkg/application/application_dev.gov3/pkg/application/application_production.go
| - cmd: wails3 tool waitport --timeout 60 | ||
| type: once |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the hardcoded waitport command from the static template.
Hardcoding the waitport command in the default configuration breaks wails3 dev for projects that do not use a frontend development server. In these projects, WAILS_VITE_PORT will be unset, causing the hardcoded command to fail and halt the watcher's startup entirely.
Since watcher.go correctly and dynamically injects this task only when a frontend dev server is active (by checking FRONTEND_DEVSERVER_URL), relying exclusively on the dynamic injection prevents startup failures in non-dev-server setups.
v3/internal/commands/build_assets/config.yml#L59-L60: Remove the hardcodedwaitporttask so that projects safely default to dynamic injection.v3/internal/commands/dev_config_test.go#L22-L26: Update the assertions to expect an array length of 3 instead of 4, and remove the explicit match for thewaitporttask to align with the template changes.
📍 Affects 2 files
v3/internal/commands/build_assets/config.yml#L59-L60(this comment)v3/internal/commands/dev_config_test.go#L22-L26
🤖 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 `@v3/internal/commands/build_assets/config.yml` around lines 59 - 60, The
static template should no longer define the hardcoded waitport task; remove it
from v3/internal/commands/build_assets/config.yml lines 59-60 and rely on
watcher.go’s dynamic injection when FRONTEND_DEVSERVER_URL is active. Update the
related assertions in v3/internal/commands/dev_config_test.go lines 22-26 to
expect three tasks and remove the explicit waitport-task match.
…gic and test coverage
aaae62b to
7cb3e56
Compare
savely-krasovsky
changed the title
savely-krasovsky
changed the title
1 participant
Description
Fixes a race in
wails3 devwhere the native application may exit before the frontend dev server becomes ready.The watcher starts the frontend task in the background and immediately launches the application. Previously, the application stopped waiting after 10 attempts at 500 ms intervals. Frontends that took slightly longer than five seconds to start caused a fatal error.
Fixes # (issue)
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Please paste the output of
wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdxwith details of this PR (v3 changelog entries are added automatically)Summary by CodeRabbit
wails3 tool waitportto wait for a host/port to become reachable.