fix(v2/devserver): don't dispatch runtime:ready from browser clients#5796
fix(v2/devserver): don't dispatch runtime:ready from browser clients#5796Trainingdlu wants to merge 2 commits into
Conversation
The injected runtime unconditionally sends `runtime:ready` on init. The desktop frontends consume it before dispatching, but the dev server only filters `drag`, so browser clients send it straight to the dispatcher, which rejects any message whose first character it does not recognise. The result is two spurious error lines per page load, the second of which reads "Unknown message from front end" and points at application code that never sent the message.
github-actions
Bot
added
Documentation
WalkthroughThe dev-server WebSocket handler now ignores ChangesDev-server runtime filtering
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
v2/internal/frontend/devserver/devserver.go (1)
199-203: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for
runtime:readyfiltering.Cover both cases:
runtime:readymust not reach the dispatcher, while ordinary application messages must continue to dispatch normally.🤖 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 `@v2/internal/frontend/devserver/devserver.go` around lines 199 - 203, Add a regression test around the devserver message filtering logic covering both branches: verify that a "runtime:ready" message is not delivered to the dispatcher, and verify that a normal application message still is. Reuse the existing devserver test setup, dispatcher spy/mock, and message-dispatch entry point rather than changing the filtering behavior.
🤖 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 `@v2/internal/frontend/devserver/devserver.go`:
- Around line 199-203: Add a regression test around the devserver message
filtering logic covering both branches: verify that a "runtime:ready" message is
not delivered to the dispatcher, and verify that a normal application message
still is. Reuse the existing devserver test setup, dispatcher spy/mock, and
message-dispatch entry point rather than changing the filtering behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a7f7fb4-fae7-4760-9bb6-0f5ce3e60095
📒 Files selected for processing (2)
v2/internal/frontend/devserver/devserver.gowebsite/src/pages/changelog.mdx
There was a problem hiding this comment.
Pull request overview
This PR fixes a v2 devserver noise issue where browser clients were causing runtime:ready to be forwarded into the dispatcher, resulting in repeated “Unknown message from front end” errors, even though the message is a runtime control signal.
Changes:
- Filter
runtime:readyin the devserver websocket IPC handler (alongside the existingdragfilter) so it is not dispatched as an application message. - Add a changelog entry describing the fix and linking to #5795.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
v2/internal/frontend/devserver/devserver.go |
Prevents runtime:ready from reaching dispatcher.ProcessMessage for browser websocket clients. |
website/src/pages/changelog.mdx |
Documents the fix in the Unreleased changelog under “Fixed”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Fixes #5795.
The injected runtime always ends init with
WailsInvoke("runtime:ready"). The desktop frontends consume it before dispatching; the dev server's websocket loop only special-casesdrag, so browser clients pass it on todispatcher.ProcessMessage, which routes on the first byte and has no case forr. Full trace in the issue.This filters it alongside
drag:Why discard it rather than replicate the desktop handling
DevWebServerembedsfrontend.Frontendand doesn't overrideExecJS, so the flags the desktop frontends set onruntime:readyonly ever reach the desktop webview, never websocket clients. Browsers can't do window dragging anyway (that's whydragis discarded), andCanResolveFilePaths()is false there since it depends onchrome.webview.postMessageWithAdditionalObjects.Same reason this doesn't extend to
resize:, sinceenableResizeis only ever set throughExecJSand a browser never emits it.draghad a// We do not support drag in browserscomment in v2.9.1 saying much the same thing. It went missing in #4215 when the loop moved to gorilla/websocket, which is probably why runtime:ready never got added next to it.Type of change
How Has This Been Tested?
Built a
vanillaproject against this branch with areplacedirective.Before: two error lines per browser page load. After: none.
Control:
window.go.main.App.Greet("test")from the browser console still returns, andwindow.runtime.LogInfo(...)still reaches the terminal, so the filter isn't swallowing anything it shouldn't.go test ./internal/... ./pkg/...passes, 24 packages.No unit test included. The
devserverpackage has no test files and the adjacentdragfilter is untested too, so covering this would mean standing up a websocket + dispatcher + bindings harness for one condition. Can add one if you want it.Test Configuration
The CLI reports v2.12.0; the application under test was built against this branch via a
replacedirective.Checklist:
website/src/pages/changelog.mdxwith details of this PR (v3 changelog entries are added automatically)Notes
Control messages consumed by the host are matched as exact strings, and each transport has to know its own list.
runtime:readyis emitted regardless of transport, so all four need to account for it; the dev server was the one that didn't. Giving these a shared prefix the dispatcher could ignore would kill the whole class, but that's a protocol change and doesn't belong in this PR.Not to be confused with #3714, which reports the same
Unknown message from front enderror from this same loop but for an unrelated reason (large payloads split across websocket frames are never reassembled). That one is a real functional bug and is not addressed here.Summary by CodeRabbit
Bug Fixes
Documentation