fix(webview): surface main-frame load failures in the in-app browser#61
fix(webview): surface main-frame load failures in the in-app browser#61jim-daf wants to merge 1 commit into
Conversation
WebViewFragment in addedit/ is the screen the user types or navigates a URL into so it can be picked for monitoring. The WebViewClient overrides onPageStarted and onPageFinished to drive the progress bar but does not override onReceivedError, so a failed navigation (DNS, certificate, offline, 4xx/5xx) just leaves a blank page with no clue what went wrong. Add an onReceivedError override that hides the progress bar and shows a short Toast with the failure reason. Gated on request.isForMainFrame so a blocked sub-resource does not also pop a Toast. Adds webview_load_failed / webview_load_failed_with_reason strings to keep the message translatable.
📝 WalkthroughWalkthroughWebViewFragment's embedded WebViewClient now handles page-load failures by overriding onReceivedError to dismiss the progress indicator, filter main-frame errors, optionally include the error reason on API 23+, and display a localized toast to the user. ChangesWebView Error Handling
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 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
`@app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.kt`:
- Around line 68-89: The WebViewFragment currently only handles onReceivedError
(main-frame DNS/offline and non-recoverable SSL), but missing handling for HTTP
errors and recoverable SSL errors; implement overrides for
onReceivedHttpError(WebView, WebResourceRequest, WebResourceResponse) to detect
main-frame HTTP 4xx/5xx and show the same progress hide + toast logic (use
getString(R.string.webview_load_failed_with_reason) when response reason/status
available), and override onReceivedSslError(WebView, SslErrorHandler, SslError)
to show a user-facing message (hide progress, show appropriate toast using
getString(...) and include sslError.primaryError or message) and call
handler.cancel() to avoid proceeding on unrecoverable cert issues; reuse the
existing progress and message construction approach from onReceivedError so all
three callbacks provide consistent UX.
- Line 88: In WebViewFragment, the Toast call inside the async WebViewClient
callback uses the Fragment's nullable context directly (Toast.makeText(context,
...)); guard against the Fragment being detached by resolving a safe context
first (e.g., val safeCtx = view?.context ?: context ?: return) or check context
?: return before calling Toast.makeText, then use safeCtx when creating/showing
the Toast to avoid crashes when the callback runs after detach.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99901c7c-e1d1-4082-bac1-4617cfcc3867
📒 Files selected for processing (2)
app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.ktapp/src/main/res/values/strings.xml
| override fun onReceivedError( | ||
| view: WebView?, | ||
| request: WebResourceRequest?, | ||
| error: WebResourceError? | ||
| ) { | ||
| super.onReceivedError(view, request, error) | ||
| // Only surface the main-frame failure, otherwise every blocked | ||
| // sub-resource would pop a toast | ||
| if (request?.isForMainFrame != true) return | ||
| progress?.visibility = View.GONE | ||
| val description = if (Build.VERSION.SDK_INT >= 23) { | ||
| error?.description?.toString().orEmpty() | ||
| } else { | ||
| "" | ||
| } | ||
| val message = if (description.isNotBlank()) { | ||
| getString(R.string.webview_load_failed_with_reason, description) | ||
| } else { | ||
| getString(R.string.webview_load_failed) | ||
| } | ||
| Toast.makeText(context, message, Toast.LENGTH_SHORT).show() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Android WebViewClient callback behavior: which callbacks are invoked for main-frame DNS/offline failures, HTTP 4xx/5xx responses, and SSL certificate errors? Please include official Android docs for onReceivedError, onReceivedHttpError, and onReceivedSslError.
💡 Result:
In Android WebView, error handling is managed through specific WebViewClient callbacks. Below is the behavior for the requested scenarios: Main-Frame DNS/Offline Failures These are considered resource loading errors and are handled by onReceivedError. - Callback: onReceivedError(WebView view, WebResourceRequest request, WebResourceError error) [1][2]. - Behavior: This method is invoked when the WebView encounters an unrecoverable error, such as a DNS lookup failure (ERROR_HOST_LOOKUP), connection failure (ERROR_CONNECT), or when the device is offline [3][2]. It is called for any resource (e.g., iframes, images) [2]. To specifically target main-frame failures, you should check the isForMainFrame property of the WebResourceRequest [4]. HTTP 4xx/5xx Responses HTTP errors are handled by a separate callback introduced in API level 23. - Callback: onReceivedHttpError(WebView view, WebResourceRequest request, WebResourceResponse errorResponse) [1][5]. - Behavior: This is invoked when the server returns an HTTP status code >= 400 [1][4]. Like onReceivedError, this callback triggers for any resource, not just the main page [1][4]. You can use request.isForMainFrame to filter for main-frame errors [4]. Note that the server response body may not be available in the errorResponse parameter [1][4]. SSL Certificate Errors SSL errors are handled by onReceivedSslError. - Callback: onReceivedSslError(WebView view, SslErrorHandler handler, SslError error) [1][3]. - Behavior: This is invoked only for recoverable SSL certificate errors [1][5]. The host application must explicitly call handler.cancel or handler.proceed to continue or stop the load [1][3]. For non-recoverable SSL errors (e.g., server fails the client), the WebView will instead call onReceivedError with the ERROR_FAILED_SSL_HANDSHAKE constant [1][5]. Official Documentation References - onReceivedError: https://developer.android.com/reference/android/webkit/WebViewClient#onReceivedError(android.webkit.WebView,%20android.webkit.WebResourceRequest,%20android.webkit.WebResourceError) - onReceivedHttpError: https://developer.android.com/reference/android/webkit/WebViewClient#onReceivedHttpError(android.webkit.WebView,%20android.webkit.WebResourceRequest,%20android.webkit.WebResourceResponse) - onReceivedSslError: https://developer.android.com/reference/android/webkit/WebViewClient#onReceivedSslError(android.webkit.WebView,%20android.webkit.SslErrorHandler,%20android.net.http.SslError)
Citations:
- 1: https://developer.android.com/reference/android/webkit/WebViewClient
- 2: https://developer.android.com/reference/kotlin/android/webkit/WebViewClient
- 3: https://emanual.github.io/Android-docs/reference/android/webkit/WebViewClient.html
- 4: https://medium.com/@iuliia-stekacheva/android-webview-and-its-pitfalls-7bd7a0a04447
- 5: https://www.aiden.tr/s/sdk_car_arm64-20240303/android-sdk_eng.root_linux-x86/docs/reference/android/webkit/WebViewClient.html
Main-frame failure handling is incomplete for HTTP and SSL error paths.
The current implementation only overrides onReceivedError, which handles DNS/offline failures and non-recoverable SSL errors, but does not handle HTTP 4xx/5xx responses (onReceivedHttpError) or recoverable SSL certificate errors (onReceivedSslError). This leaves users without feedback when the server returns HTTP errors or when certificate validation fails. Both cases are explicitly listed in the PR objective and require separate callbacks to be implemented.
🤖 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
`@app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.kt`
around lines 68 - 89, The WebViewFragment currently only handles onReceivedError
(main-frame DNS/offline and non-recoverable SSL), but missing handling for HTTP
errors and recoverable SSL errors; implement overrides for
onReceivedHttpError(WebView, WebResourceRequest, WebResourceResponse) to detect
main-frame HTTP 4xx/5xx and show the same progress hide + toast logic (use
getString(R.string.webview_load_failed_with_reason) when response reason/status
available), and override onReceivedSslError(WebView, SslErrorHandler, SslError)
to show a user-facing message (hide progress, show appropriate toast using
getString(...) and include sslError.primaryError or message) and call
handler.cancel() to avoid proceeding on unrecoverable cert issues; reuse the
existing progress and message construction approach from onReceivedError so all
three callbacks provide consistent UX.
| } else { | ||
| getString(R.string.webview_load_failed) | ||
| } | ||
| Toast.makeText(context, message, Toast.LENGTH_SHORT).show() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and read the WebViewFragment.kt file
find . -name "WebViewFragment.kt" -type fRepository: bernaferrari/ChangeDetection
Length of output: 152
🏁 Script executed:
#!/bin/bash
# Read the file around line 88 to see the actual context
if [ -f "app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.kt" ]; then
wc -l "app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.kt"
fiRepository: bernaferrari/ChangeDetection
Length of output: 154
🏁 Script executed:
#!/bin/bash
# Read the file with context around line 88
sed -n '60,110p' "app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.kt"Repository: bernaferrari/ChangeDetection
Length of output: 1900
🏁 Script executed:
#!/bin/bash
# Read the entire file to understand the class structure and callback
cat "app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.kt"Repository: bernaferrari/ChangeDetection
Length of output: 4814
Guard nullable Fragment context before showing Toast.
Line 88 uses context directly in an async WebViewClient callback; if the Fragment is detached when the callback arrives, this will crash. Prefer view?.context ?: return (or combine with context ?: return) before Toast.makeText.
Suggested patch
- Toast.makeText(context, message, Toast.LENGTH_SHORT).show()
+ val ctx = view?.context ?: return
+ Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Toast.makeText(context, message, Toast.LENGTH_SHORT).show() | |
| val ctx = view?.context ?: return | |
| Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show() |
🤖 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
`@app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.kt`
at line 88, In WebViewFragment, the Toast call inside the async WebViewClient
callback uses the Fragment's nullable context directly (Toast.makeText(context,
...)); guard against the Fragment being detached by resolving a safe context
first (e.g., val safeCtx = view?.context ?: context ?: return) or check context
?: return before calling Toast.makeText, then use safeCtx when creating/showing
the Toast to avoid crashes when the callback runs after detach.
1 participant
Closes #60.
The in-app browser
WebViewFragment(app/src/main/java/com/bernaferrari/changedetection/addedit/WebViewFragment.kt:48) overridesonPageStartedandonPageFinishedto drive the progress bar but does not overrideonReceivedError. When the user types a URL that fails to load (DNS error, offline, certificate issue, 4xx/5xx) the WebView is left showing a blank area with no indication of what went wrong.Add an
onReceivedErroroverride that hides the progress bar and shows a short Toast with the failure reason on main-frame failures only (so blocked sub-resources do not also pop Toasts):Adds two strings (
webview_load_failed,webview_load_failed_with_reason) so the message stays translatable. The happy path is unchanged.Summary by CodeRabbit
Release Notes