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 ๐คฃ )
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
:(
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 ๐