fix(mobile): return browser launch errors - #5916
Conversation
github-actions
Bot
added
Documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughMobile ChangesMobile API contract and desktop behavior
Android launch validation and propagation
iOS launch validation and propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GoCaller as application.Android.OpenURL or application.IOS.OpenURL
participant NativeBridge as WailsBridge.openURL or ios_open_url
participant PlatformHandler as Android or iOS URL handler
GoCaller->>NativeBridge: URL string
NativeBridge->>PlatformHandler: validated URL launch
PlatformHandler-->>NativeBridge: launch result or failure
NativeBridge-->>GoCaller: error status
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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: 1
🤖 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/mobile_features_ios.m`:
- Around line 113-115: Update the URL-opening flow around the main-queue block
and dispatch_semaphore_wait to use synchronized cancellation state shared by
both paths. Have the main-queue block check and claim the state before calling
openURL, and have the timeout path mark the operation cancelled before
returning, ensuring a queued block cannot launch the URL after timeout.
🪄 Autofix
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 Plus
Run ID: 45a6fdf8-7b65-4a7a-91b6-6392de3d160e
📒 Files selected for processing (15)
v3/ANDROID.mdv3/IOS.mdv3/examples/mobile/build/android/app/src/main/java/com/wails/app/WailsBridge.javav3/examples/mobile/native_features_android.gov3/examples/mobile/native_features_ios.gov3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.javav3/internal/commands/mobile_openurl_test.gov3/pkg/application/mobile.gov3/pkg/application/mobile_features_android.gov3/pkg/application/mobile_features_ios.gov3/pkg/application/mobile_features_ios.hv3/pkg/application/mobile_features_ios.mv3/pkg/application/mobile_features_source_test.gov3/pkg/application/mobile_stub.gov3/pkg/application/mobile_stub_test.go
| long waitResult = dispatch_semaphore_wait( | ||
| completed, dispatch_time(DISPATCH_TIME_NOW, 30 * NSEC_PER_SEC)); | ||
| if (waitResult != 0) return mfDup(@"timed out waiting for application to open URL"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent a URL launch after a timeout.
If the main queue is blocked for 30 seconds, this function returns a timeout error but leaves the queued block active. When the main queue resumes, the block still calls openURL. The caller then receives a failure result while the external URL launches later.
Add synchronized cancellation state. The main-queue block must check and claim that state before it calls openURL. The timeout path must mark the operation cancelled before it returns.
🤖 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/pkg/application/mobile_features_ios.m` around lines 113 - 115, Update the
URL-opening flow around the main-queue block and dispatch_semaphore_wait to use
synchronized cancellation state shared by both paths. Have the main-queue block
check and claim the state before calling openURL, and have the timeout path mark
the operation cancelled before returning, ensuring a queued block cannot launch
the URL after timeout.
2 participants
Description
Mobile
OpenURLcurrently returns nothing on Android and iOS. Android postsstartActivityand only logs exceptions, while iOS discards theopenURLcompletion result. Callers therefore cannot distinguish a successful browser launch from an invalid URL, missing handler, or rejected launch.This changes the shared contract to:
Android now validates URI syntax, performs the activity launch on the main thread, and returns launch exceptions through the existing JNI string bridge. iOS validates the URL and returns the asynchronous
UIApplication.openURLcompletion result to Go. Both platforms use bounded waits so a stalled UI thread cannot block the caller indefinitely.The desktop
Mobilestub returns an unsupported-platform error. The generated Android host, checked-in mobile example, documentation, and example error handling are updated together. Focused source-contract tests protect the native bridge signatures and failure paths.This was surfaced by PKCE browser flows: silently failing to launch a browser leaves authentication waiting for a callback that can never arrive. The behavior is general to all Wails mobile applications and does not depend on a particular authentication library.
Compatibility note
Mobile support is experimental, but this changes the exported
MobileManagermethod signature and therefore requires custom implementations to add theerrorresult. Ordinary callers that invokeOpenURL(...)as a statement continue to compile.Existing generated Android projects contain the old
void openURL(String)host method. Because generated Java hosts are not updated automatically, those projects must regenerate the Android host or synchronizeWailsBridge.javawhen upgrading.No matching open issue was found.
Type of change
How Has This Been Tested?
go test -count=1 ./internal/commands ./pkg/applicationHonest caveats:
Android and iOS launches were not exercised on a device from this Linux host.
The standalone mobile example module currently requests an unrelated
go mod tidy, so its module-wide test was not allowed to modify metadata.Windows
macOS
Linux
Test Configuration
Checklist:
Summary by CodeRabbit
New Features
Documentation
Tests