Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android: onError() implementation is deficient #3289

Closed
birdofpreyru opened this issue Jan 15, 2024 · 7 comments
Closed

Android: onError() implementation is deficient #3289

birdofpreyru opened this issue Jan 15, 2024 · 7 comments

Comments

@birdofpreyru
Copy link

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:
@birdofpreyru
Copy link
Author

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.

@birdofpreyru
Copy link
Author

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.

birdofpreyru added a commit to birdofpreyru/react-native-webview that referenced this issue Jan 16, 2024
Copy link

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

@github-actions github-actions bot added the Stale label Mar 16, 2024
@birdofpreyru
Copy link
Author

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

@github-actions github-actions bot removed the Stale label Mar 17, 2024
Copy link

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

@github-actions github-actions bot added the Stale label May 17, 2024
@birdofpreyru
Copy link
Author

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 😎

@github-actions github-actions bot removed the Stale label May 18, 2024
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant