Add server reachability check before loading WebView#34
Add server reachability check before loading WebView#343rob3 merged 4 commits intoimmichFrame:mainfrom
Conversation
Check if the ImmichFrame server is reachable before loading the WebView. If not reachable, show a toast and retry every 5 seconds (up to 36 attempts). This fixes the black screen issue when WiFi is slow to connect after reboot, while also working for VPN users where Android's network validation may report "no internet" even when the server is actually reachable.
…perly handel when it's destroyed
… properly handle when dreaming stops
WalkthroughThe changes add server reachability verification before WebView content loading in MainActivity and ScreenSaverService, implementing a retry mechanism with 5-second intervals (up to 36 attempts) to handle delayed network connectivity after device boot. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as MainActivity/<br/>ScreenSaverService
participant Coroutine as Coroutine<br/>(IO Dispatcher)
participant Helper as Helpers
participant OkHttp as OkHttpClient
participant Server as ImmichFrame<br/>Server
participant UI as UI Thread
User->>App: Trigger URL Load
App->>App: loadWebViewWithRetry()
loop Retry Loop (attempt ≤ maxAttempts)
App->>Coroutine: Launch on IO Dispatcher
Coroutine->>Helper: isServerReachable(url)
Helper->>OkHttp: HEAD Request (5s timeout)
OkHttp->>Server: Check Connection
alt Server Reachable
Server-->>OkHttp: HTTP Response
OkHttp-->>Helper: Success
Helper-->>Coroutine: true
Coroutine->>UI: Switch to Main Dispatcher
UI->>App: loadUrl() on WebView
App-->>User: Content Loads
Note over App: ✓ Success
else Server Unreachable
Server--xOkHttp: No Response / Exception
OkHttp-->>Helper: Exception
Helper-->>Coroutine: false
alt Attempts Remaining
Coroutine->>UI: Show "Connecting..." Toast
Coroutine->>Coroutine: Delay 5 seconds
Note over Coroutine: Retry...
else Max Attempts Exceeded
Coroutine->>UI: Show Failure Toast
UI->>App: loadUrl() Anyway
App-->>User: Load Attempted
Note over App: ✗ Max Retries
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/src/main/java/com/immichframe/immichframe/Helpers.kt (1)
194-206: Consider adding debug logging for troubleshooting network issues.The function correctly returns a boolean reachability status and treats any HTTP response as reachable. However, silently swallowing exceptions makes it harder to diagnose connectivity problems in production.
🔎 View suggested enhancement
fun isServerReachable(url: String): Boolean { return try { val request = Request.Builder() .url(url) .head() .build() reachabilityClient.newCall(request).execute().use { true // any HTTP response = reachable } } catch (e: Exception) { + Log.d("Helpers", "Server reachability check failed for $url: ${e.message}") false } }app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
861-892: LGTM! The retry mechanism correctly implements the PR objectives.The implementation:
- Performs reachability checks on the IO dispatcher to avoid blocking the UI
- Provides user feedback via toasts with attempt counts
- Retries every 5 seconds for up to 36 attempts (3 minutes total), matching the behavior for non-WebView mode
- Falls back to loading the URL after max attempts, ensuring the WebView eventually loads (consistent with the existing error handler at line 571-586)
- Uses
lifecycleScopeto automatically cancel retries if the activity is destroyedNote: This function is duplicated in
ScreenSaverService.kt(lines 624-655). While the author chose this approach for consistency with existing patterns, consider extracting the retry logic to a shared helper function in a future refactor to improve maintainability.app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (1)
624-655: LGTM! The retry mechanism mirrors MainActivity's implementation.The implementation correctly:
- Performs reachability checks on the IO dispatcher
- Provides user feedback via toasts
- Retries every 5 seconds for up to 36 attempts
- Falls back to loading the URL after exhausting retries
- Uses the custom
webViewRetryScopefor proper lifecycle management inDreamServiceNote: As mentioned in the MainActivity review, this function is duplicated. The duplication is acceptable given the different lifecycle management requirements (lifecycleScope vs. custom scope), but could be refactored to a shared helper in future iterations.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/immichframe/immichframe/Helpers.kt(2 hunks)app/src/main/java/com/immichframe/immichframe/MainActivity.kt(3 hunks)app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (1)
loadWebViewWithRetry(624-655)
app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (1)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
loadWebViewWithRetry(861-892)
🪛 detekt (1.23.8)
app/src/main/java/com/immichframe/immichframe/Helpers.kt
[warning] 203-203: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (9)
app/src/main/java/com/immichframe/immichframe/Helpers.kt (2)
13-13: LGTM!The necessary imports for the reachability check are correctly added.
Also applies to: 16-16
189-192: LGTM!The reusable
reachabilityClientwith 5-second timeouts prevents client recreation on each retry and ensures timely failure detection.app/src/main/java/com/immichframe/immichframe/MainActivity.kt (2)
46-46: LGTM!The
lifecycleScopeimport enables lifecycle-aware coroutine management for the retry mechanism.
592-592: LGTM!Replacing direct WebView loading with retry-enabled loading addresses the core issue of silent failures when network connectivity is delayed at boot.
app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (5)
30-32: LGTM!The additional coroutine imports support the custom scope management required for the retry mechanism in
DreamService.
47-47: LGTM!The nullable
webViewRetryScopefield enables proper lifecycle management for the retry coroutines, sinceDreamServicedoesn't provide a built-in lifecycle scope likeActivity.
90-90: LGTM!Initializing the retry scope when the dream starts ensures coroutines are launched in a properly managed context.
113-114: LGTM!Properly canceling and nulling the scope in
onDreamingStopped()prevents memory leaks and ensures retry coroutines are cleaned up when the screen saver stops.
528-528: LGTM!Replacing direct WebView loading with retry-enabled loading ensures consistent behavior with
MainActivityand addresses delayed network connectivity at boot.
|
LGTM, thanks! |
2 participants
Check if the ImmichFrame server is reachable before loading the WebView. If not reachable, show a toast and retry every 5 seconds (up to 36 attempts), same as when WebView is disabled.
Fixes #33. Tested on my Frameo device.
Disclosure: I used Claude Code to debug this problem and code this pull request.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.