fix(v3): restore Android build and runtime message handling#5022
fix(v3): restore Android build and runtime message handling#5022leaanthony wants to merge 1 commit intov3-alphafrom
Conversation
Fixes five regressions in Android support: 1. Restore events.Android struct (IDs 1259-1270) and JS event mappings that were removed from events.go, breaking events_common_android.go 2. Pass flags to runtime.Core() in nativeOnPageFinished to match the updated API signature used by all other platforms 3. Call setupCommonEvents() in the CGO build's run() method (the non-CGO build already did this) 4. Replace the stub handleMessageForAndroid() with real MessageProcessor routing so JS-to-Go runtime calls (bound methods, clipboard, etc.) actually work 5. Handle non-JSON string messages (e.g. "wails:runtime:ready") that the JS bridge sends before attempting JSON parse Fixes #5020 Co-authored-by: Varun Chawla <varun_6april@hotmail.com>
|
WalkthroughThis PR restores Android event definitions and implements complete message processor routing for Android JNI callbacks. It adds support for handling both JSON and non-JSON messages, integrates Android-specific event lifecycle management, and properly initializes the MessageProcessor during platform initialization with context-based request handling. Changes
Sequence DiagramsequenceDiagram
participant AJNICallback as Android JNI<br/>Callback
participant Handler as handleMessageForAndroid
participant MsgProc as MessageProcessor
participant Runtime as Runtime
participant Response as JSON Response
AJNICallback->>Handler: message (string)
alt Non-JSON Message
Handler->>Response: return {"success":true}
else JSON Message
Handler->>Handler: Parse RuntimeRequest
Handler->>MsgProc: HandleRuntimeCallWithIDs(ctx, req)
MsgProc->>Runtime: Execute runtime call
Runtime->>MsgProc: return result
MsgProc->>Handler: result/error
Handler->>Response: Marshal JSON response
end
Response->>AJNICallback: return response string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.1)v3/pkg/application/application_android.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m v3/pkg/events/events.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m v3/pkg/application/application_android_nocgo.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes critical Android build and runtime issues that were preventing the Android platform from functioning. The changes restore missing Android event definitions, update the runtime initialization to match the current API, and implement proper message routing for JavaScript-to-Go calls.
Changes:
- Restores the
events.Androidstruct and JavaScript event mappings (IDs 1259-1270) that were previously removed - Updates
runtime.Core()call innativeOnPageFinishedto pass flags parameter, matching all other platforms - Adds
setupCommonEvents()call in CGO build path to map Android platform events to common events - Replaces stub
handleMessageForAndroid()with fullMessageProcessorimplementation for routing runtime calls - Handles non-JSON string messages (like
"wails:runtime:ready") before attempting JSON parse
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
v3/pkg/events/events.go |
Adds Android event struct with 12 lifecycle/WebView events and their JavaScript mappings |
v3/pkg/application/application_android.go |
Updates runtime.Core() to pass flags, adds setupCommonEvents() call, implements MessageProcessor routing with global state |
v3/pkg/application/application_android_nocgo.go |
Implements MessageProcessor routing with global state, mirrors CGO implementation for non-CGO builds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
v3/pkg/application/application_android.go (1)
526-532:⚠️ Potential issue | 🟠 MajorMemory leak:
C.CStringallocations in error response and success response paths are never freed.
C.CStringallocates memory that must be freed withC.free. The strings created at lines 527 and 532 are passed toC.createJStringand never freed, causing memory leaks on every message handled.🐛 Proposed fix to free C strings
if app == nil { errorResponse := `{"error":"App not initialized"}` - return C.createJString(env, C.CString(errorResponse)) + cErr := C.CString(errorResponse) + defer C.free(unsafe.Pointer(cErr)) + return C.createJString(env, cErr) } // Parse and handle the message response := handleMessageForAndroid(app, goMessage) - return C.createJString(env, C.CString(response)) + cResp := C.CString(response) + defer C.free(unsafe.Pointer(cResp)) + return C.createJString(env, cResp) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/application_android.go` around lines 526 - 532, The C.CString allocations for errorResponse and the success response are never freed causing a leak; update the code around the error path and the call to handleMessageForAndroid so that each C.CString passed into C.createJString is freed afterwards (use C.free with the unsafe.Pointer of the C string, e.g., free the result of C.CString(errorResponse) and the C.CString(response) after calling C.createJString), and ensure any early returns still free their C strings; refer to createJString, C.CString, C.free, handleMessageForAndroid, errorResponse and response to locate where to add the frees.
🧹 Nitpick comments (2)
v3/pkg/application/application_android_nocgo.go (1)
188-192: Non-JSON detection heuristic is reasonable but limited.The check
message[0] != '{'works for the documented use case ("wails:runtime:ready"), but note that it will also silently succeed for any malformed message not starting with{. This is acceptable given the current use case, but consider adding a comment documenting the known non-JSON messages for future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/application_android_nocgo.go` around lines 188 - 192, The non-JSON detection in handleMessageForAndroid currently checks only message[0] != '{' and returns early; update the code by adding a clarifying comment above that if-block documenting the known non-JSON messages (e.g. "wails:runtime:ready") and noting the heuristic’s limitation (it will treat any non-{ start as non-JSON), so future maintainers understand the intent; reference handleMessageForAndroid and the androidLogf call so the comment sits next to that logic.v3/pkg/application/application_android.go (1)
640-688: Significant code duplication with the non-CGO build.The
handleMessageForAndroidfunction is nearly identical betweenapplication_android.goandapplication_android_nocgo.go. Consider extracting the shared logic into a common file (e.g.,application_android_common.gowithout build tags, or with//go:build android) to reduce maintenance burden and ensure consistent behavior.The same JSON escaping issue noted in the non-CGO review (error messages not properly escaped) applies here as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/application_android.go` around lines 640 - 688, The function handleMessageForAndroid duplicates logic across builds and also returns unescaped error strings; extract the shared logic into a single helper (e.g., processAndroidRuntimeMessage(ctx, app, message) or handleMessageForAndroidCommon) placed in a common Android file (without differing build tags) and have both application_android.go and application_android_nocgo.go call it; while refactoring, replace all direct fmt.Sprintf(`{"error":"%s"}`, err.Error())/raw string returns with proper JSON construction (e.g., build a struct like type runtimeResp struct{ Error string `json:"error"`; Success *bool `json:"success,omitempty"` } and json.Marshal it or json.Marshal(map[string]string{"error": err.Error()})) so errors are correctly escaped, and keep existing behaviors such as filling WebviewWindowID and logging via androidLogf and using messageProc.HandleRuntimeCallWithIDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@v3/pkg/application/application_android_nocgo.go`:
- Around line 196-198: The JSON error strings are built by interpolating
err.Error() directly, which can produce invalid JSON; update the error returns
in handleMessageForAndroid (and the other similar return sites around the
current diff) to json.Marshal the error string and embed the marshaled bytes
into the response (e.g. produce {"error":<marshaledError>}) so quotes and
control characters are escaped; if json.Marshal fails, fall back to a safe
literal (or an empty string) and still log the marshal error via androidLogf —
replace the current fmt.Sprintf(`{"error":"%s"}`, err.Error()) returns with
building the response using the json.Marshal result and keep the existing
androidLogf call.
---
Outside diff comments:
In `@v3/pkg/application/application_android.go`:
- Around line 526-532: The C.CString allocations for errorResponse and the
success response are never freed causing a leak; update the code around the
error path and the call to handleMessageForAndroid so that each C.CString passed
into C.createJString is freed afterwards (use C.free with the unsafe.Pointer of
the C string, e.g., free the result of C.CString(errorResponse) and the
C.CString(response) after calling C.createJString), and ensure any early returns
still free their C strings; refer to createJString, C.CString, C.free,
handleMessageForAndroid, errorResponse and response to locate where to add the
frees.
---
Nitpick comments:
In `@v3/pkg/application/application_android_nocgo.go`:
- Around line 188-192: The non-JSON detection in handleMessageForAndroid
currently checks only message[0] != '{' and returns early; update the code by
adding a clarifying comment above that if-block documenting the known non-JSON
messages (e.g. "wails:runtime:ready") and noting the heuristic’s limitation (it
will treat any non-{ start as non-JSON), so future maintainers understand the
intent; reference handleMessageForAndroid and the androidLogf call so the
comment sits next to that logic.
In `@v3/pkg/application/application_android.go`:
- Around line 640-688: The function handleMessageForAndroid duplicates logic
across builds and also returns unescaped error strings; extract the shared logic
into a single helper (e.g., processAndroidRuntimeMessage(ctx, app, message) or
handleMessageForAndroidCommon) placed in a common Android file (without
differing build tags) and have both application_android.go and
application_android_nocgo.go call it; while refactoring, replace all direct
fmt.Sprintf(`{"error":"%s"}`, err.Error())/raw string returns with proper JSON
construction (e.g., build a struct like type runtimeResp struct{ Error string
`json:"error"`; Success *bool `json:"success,omitempty"` } and json.Marshal it
or json.Marshal(map[string]string{"error": err.Error()})) so errors are
correctly escaped, and keep existing behaviors such as filling WebviewWindowID
and logging via androidLogf and using messageProc.HandleRuntimeCallWithIDs.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v3/pkg/application/application_android.gov3/pkg/application/application_android_nocgo.gov3/pkg/events/events.go
| androidLogf("error", "🤖 [handleMessageForAndroid] Failed to parse: %v", err) | ||
| return fmt.Sprintf(`{"error":"%s"}`, err.Error()) | ||
| } |
There was a problem hiding this comment.
Error messages aren't JSON-escaped, risking malformed responses.
If err.Error() contains quotes or control characters, the resulting JSON will be invalid. Consider using json.Marshal for the error string to ensure proper escaping.
🔧 Proposed fix using json.Marshal for error strings
if err := json.Unmarshal([]byte(message), &req); err != nil {
androidLogf("error", "🤖 [handleMessageForAndroid] Failed to parse: %v", err)
- return fmt.Sprintf(`{"error":"%s"}`, err.Error())
+ errStr, _ := json.Marshal(err.Error())
+ return fmt.Sprintf(`{"error":%s}`, errStr)
}Apply similar changes to the other error returns at lines 219-221 and 229-231.
Also applies to: 219-221, 229-231
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@v3/pkg/application/application_android_nocgo.go` around lines 196 - 198, The
JSON error strings are built by interpolating err.Error() directly, which can
produce invalid JSON; update the error returns in handleMessageForAndroid (and
the other similar return sites around the current diff) to json.Marshal the
error string and embed the marshaled bytes into the response (e.g. produce
{"error":<marshaledError>}) so quotes and control characters are escaped; if
json.Marshal fails, fall back to a safe literal (or an empty string) and still
log the marshal error via androidLogf — replace the current
fmt.Sprintf(`{"error":"%s"}`, err.Error()) returns with building the response
using the json.Marshal result and keep the existing androidLogf call.
Deploying wails with
|
| Latest commit: |
75eaaf2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://733bc91c.wails.pages.dev |
| Branch Preview URL: | https://fix-android-build-and-runtim.wails.pages.dev |
|
@leaanthony would you like this testing? I can pull this branch and try it? |
|
When i pull/clean this branch and then run `` |
Summary
events.Androidstruct and JS event mappings removed fromevents.go(IDs 1259–1270)runtime.Core()innativeOnPageFinishedto match the updated API used by all other platformssetupCommonEvents()in the CGO build path (non-CGO already did this)handleMessageForAndroid()with realMessageProcessorrouting so JS→Go calls workwails:runtime:ready) before attempting JSON parseTest plan
go build ./pkg/events/passesgo vet ./pkg/events/passeswails3 task android:packagewails:runtime:readystring message doesn't cause JSON parse errors in logsFixes #5020
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements