react-native-webview/react-native-webview

Android: `onError()` implementation is deficient

birdofpreyru opened this issue ยท 7 comments

Bug description:

The onError() implementation for Android (see below), in its native layer, overrides the version of onReceivedError() method deprecated in Android API 23 (thus, since 2015 โ€” this library is really well-maintained ๐Ÿคฃ )

@Override
public void onReceivedError(
WebView webView,
int errorCode,
String description,
String failingUrl) {

and unlike the new version of the method the old one is fired for the main resource errors only โ€” you get the error only if it failed to load the URI passed into the source itself, if it failed to load assets it depends upon (like JS scripts, CSS sheets, and other stuff required by the main resource) you won't get any error, even if it breaks the page and deserves an attempt to reload the page, or some other error-handling in your host app.

P.S.: As this is one is a sensitive problem for myself, I'll fix it and create PR shortly. If anybody is willing to say thanks for that, I am accepting donations via GitHub.Sponsors ๐Ÿ˜‰

To Reproduce:

Expected behavior:

Screenshots/Videos:

Environment:

  • OS:
  • OS version:
  • react-native version:
  • react-native-webview version:

Hmm... having looked around a bit more, I see that onHttpError implementation actually actively ignores errors for assets loading... which is kind of inconsistent with what I want to do with onError :(

@RequiresApi(api = Build.VERSION_CODES.M)
@Override
public void onReceivedHttpError(
WebView webView,
WebResourceRequest request,
WebResourceResponse errorResponse) {
super.onReceivedHttpError(webView, request, errorResponse);
if (request.isForMainFrame()) {
WritableMap eventData = createWebViewEvent(webView, request.getUrl().toString());
eventData.putInt("statusCode", errorResponse.getStatusCode());
eventData.putString("description", errorResponse.getReasonPhrase());
int reactTag = RNCWebViewWrapper.getReactTagFromWebView(webView);
UIManagerHelper.getEventDispatcherForReactTag((ReactContext) webView.getContext(), reactTag).dispatchEvent(new TopHttpErrorEvent(reactTag, eventData));
}
}

Another inconsistency is with the current logic assuming that onLoadEnd should be fired prior to onError; while if I do fire onError for each asset may fail to load for the main resource, it makes sense to emit onError for each failing asset, and then onLoadEnd after it.

I guess, my suggestion is to:

  • Emit onError for each asset that failed to load during the main page loading.
  • Emit onLoadEnd once entire page finished all loading operations (thus, possibly after multiple previous errors).
  • Emit onLoad only if no errors have been encountered during the page loading for the page itself, nor assets it attempts to load.

Hello ๐Ÿ‘‹, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

The issue is not stale, the PR still waits to be merged.

Hello ๐Ÿ‘‹, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

Yes, bot, the issue is still here, as maintainers still have not merged the PR fixing it.

In the meantime, the issue has been long fixed in my fork of RN WebView, which is a few steps ahead of the upstream library ๐Ÿ˜Ž