-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Fix missing webview provider crash on Android #29089
Conversation
Base commit: a50fa55 |
Base commit: a50fa55 |
Any news how to decide this error? |
@jocoders Not sure 😅 @cpojer @thorbenprimke you guys reviewed the previous PR about the same subject, maybe you can help here? 🙏 |
.getClass() | ||
.getCanonicalName() | ||
.equals("android.webkit.WebViewFactory.MissingWebViewPackageException")) { | ||
if (message != null && message.contains("WebView")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the right fix to catch both the original case (from my fix) and the cases by @siddhantsoni, it would be combine both.
if ((message != null && message.contains("No WebView installed")) ||
exception
.getClass()
.getCanonicalName()
.equals("android.webkit.WebViewFactory.MissingWebViewPackageException")
) {
The reason for this is that if you look at the exceptions listed on #26189, the message does not always contain WebView. Looking back at the crashes listed on #24533, they were also AndroidRuntimeExceptions like the ones that you (@Almouro) are seeing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, your fix catch both cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my case the exception message is android.webkit.WebViewFactory$MissingWebViewPackageException: Failed to load WebView provider: No WebView installed
So maybe we can check for the message to contain MissingWebViewPackageException
and that would do it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Almouro, would you be able to take a look at the above comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sota000, I created a new PR with the changes requested.
PR: RodolfoGS#1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey. It seems that on some devices that crash comes out as a runtime crash "android.util.AndroidRuntimeException" and the check fails. I can reproduce it by disabling the webview on the device. Should we also add it to the check? @RodolfoGS @yungsters @sota000
Dear 9Hyeonwoo! I disabled all browsers on the test phone and tried to open some links from my app, but the app did not crashed and no errors. Your case of the error's appearance has not confirmed on my app. Any new ideas how to reproduce this error? |
We're getting this error for a few users of our app as well, so this fix would be very welcome. |
@jocoders I could reproduce the crash disabling To do that I followed this steps:
After of that, my crash when it starts. @Almouro Do you know how I could create a patch (using I can edit the file located in:
But when I navigate with Android Studio that file is located in:
Thanks a lot! |
.getClass() | ||
.getCanonicalName() | ||
.equals("android.webkit.WebViewFactory.MissingWebViewPackageException")) { | ||
if (message != null && message.contains("WebView")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, your fix catch both cases.
Anyone know someone from the react-native team to merge this PR? It's a super simple PR that fix an easy reproducible crash and are opened for about 1 year ago ¯_(ツ)_/¯ |
Hello, same problem on RN 0.64.0; waiting for the merge to be approved 😊 |
Facing the same issue in our production application, is there a way we can handle this from user code? |
@sota000 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
I imported the diff for internal review. Since there is a comment about improving the check, please feel free to create a new PR or update this PR to include the suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for taking the time to review. Please address the comments made inline, and I will reimport the PR.
.getClass() | ||
.getCanonicalName() | ||
.equals("android.webkit.WebViewFactory.MissingWebViewPackageException")) { | ||
if (message != null && message.contains("WebView")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Almouro, would you be able to take a look at the above comments?
@RodolfoGS, can you please create the pull request to the React Native repo instead of your fork? We will import and merge the version as this PR has been stale for a while. Thanks! |
I am going to close this PR and will work on merging #32165. Thanks @RodolfoGS for taking it over as well as @Almouro and others for the initial contribution! |
Summary: I applied the changes requested in this PR: #29089 We upgraded to RN 0.62.2 on our latest release and started to see again the "Failed to load WebView provider: No WebView installed" (see below for Crashlytics screenshot) ![image](https://user-images.githubusercontent.com/870365/131935283-033fbd44-5a3b-49b0-bd25-3d6733f22040.png) This crash had been fixed by #24533 but #26189 (added in 0.62) reverted the fix Indeed the exception raised in Crashlytics is actually a `AndroidRuntimeException` and `MissingWebViewPackageException` is only part of the message. For instance, in the screenshot above, the exception message is `android.webkit.WebViewFactory$MissingWebViewPackageException: Failed to load WebView provider: No WebView installed` Now these crashes are quite tricky to reproduce, so to be on the safe side, I'm filtering out all exceptions containing `WebView` as suggested by thorbenprimke on the original fix. If my reasoning is correct, it should fix siddhantsoni 's issue as well, since `WebView` is included in `MissingWebViewPackageException` But following that reasoning, I am not sure #26189 fixed siddhantsoni 's issue, so siddhantsoni if you could check that this PR also fixes your issue, that would be great! ## Changelog [Android] [Fixed] - Fix missing WebView provider crash in ForwardingCookieHandler <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> Pull Request resolved: #32165 Test Plan: I created a version of react native with this patch applied ``` "react-native": "almouro/react-native#release/062-2-fix-missing-webview-provider" ``` Before the fix ~0.1% of our users were impacted on Android, no new crashes have occurred after the update. This is putting back what was already in place and working for us, but making the check wider to catch more errors. Reviewed By: lunaleaps Differential Revision: D30847404 Pulled By: sota000 fbshipit-source-id: fe3b5fa2c9ebde5bedd17a9d6394a52ccdbdf0d0
Great news that this is fixed! 🎉 Thanks @RodolfoGS for taking over and @sota000 for getting it merged! |
I still can reproduce this issue after update to 0.67.0-rc.3. |
Summary: The check implemented in PR #29089 is flawed as the exception class name and message depends on the OS version as well as the OEM. In OxygenOS running android 11, it comes out as RuntimeException and the check fails and hence the crash occurs again. But on observing closely, its clear that the exception message is consistent across OEMs with similar strings appearing in them that have been included in the if check. Hence there is a simple fix to this issue, by checking the message instead of the exception class. Here is the snippet: ``` private Nullable CookieManager getCookieManager() { if (mCookieManager == null) { possiblyWorkaroundSyncManager(mContext); try { mCookieManager = CookieManager.getInstance(); } catch (IllegalArgumentException ex) { // https://bugs.chromium.org/p/chromium/issues/detail?id=559720 return null; } catch (Exception exception) { String message = exception.getMessage(); // We cannot catch MissingWebViewPackageException as it is in a private / system API // class. This validates the exception's message to ensure we are only handling this // specific exception. // The exception class doesn't always contain the correct name as it depends on the OEM // and OS version. It is better to check the message for clues regarding the exception // as that is somewhat consistent across OEMs. // For instance, the Exception thrown on OxygenOS 11 is a RuntimeException but the message contains the required strings. // https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/webkit/WebViewFactory.java#348 if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || (message!=null && (message.contains("WebView provider") || message.contains("No WebView installed")))){ return null; } else { throw exception; } } } return mCookieManager; } ``` ## Changelog [General] [Added] - A fail proof check to catch any crash involving webview: if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || (message!=null && (message.contains("WebView provider") || message.contains("No WebView installed")))) [General] [Removed] - Flawed check to catch WebViewProvider crash: if (message != null && exception.getClass().getCanonicalName().contains("MissingWebViewPackageException")) Pull Request resolved: #33088 Test Plan: This code has been tested rigorously on OnePlus Nord CE 5G (Oxygen OS 11.0) and Realme X (Realme UI 2.0) both running on Android 11 and reproducing the crash on a hybrid (Native android + ReactNative) app that showed this crash in production being dependent on WebViews. I have implemented an entire patched fork of 0.67.2 to fix this issue in our app. How to test: Launch any app that has a webview. Go to settings->apps->Android System Webview -> disable. Resume/Restart the app. In this fix: Open a webview and notice that the app will not crash. In current version: App crashes on startup as the exception escapes the catch block. Even if it survives startup (did on Realme X), it will crash once you try to open a webview. Reviewed By: rh389 Differential Revision: D34240097 Pulled By: cortinico fbshipit-source-id: 0f1f9a3b078c0ad3074c7841392892cb70b427eb
@HugoGresse seems that was merged on 0.70.1 -> https://github.com/facebook/react-native/releases/tag/v0.70.1
|
…book#33088) Summary: The check implemented in PR facebook#29089 is flawed as the exception class name and message depends on the OS version as well as the OEM. In OxygenOS running android 11, it comes out as RuntimeException and the check fails and hence the crash occurs again. But on observing closely, its clear that the exception message is consistent across OEMs with similar strings appearing in them that have been included in the if check. Hence there is a simple fix to this issue, by checking the message instead of the exception class. Here is the snippet: ``` private Nullable CookieManager getCookieManager() { if (mCookieManager == null) { possiblyWorkaroundSyncManager(mContext); try { mCookieManager = CookieManager.getInstance(); } catch (IllegalArgumentException ex) { // https://bugs.chromium.org/p/chromium/issues/detail?id=559720 return null; } catch (Exception exception) { String message = exception.getMessage(); // We cannot catch MissingWebViewPackageException as it is in a private / system API // class. This validates the exception's message to ensure we are only handling this // specific exception. // The exception class doesn't always contain the correct name as it depends on the OEM // and OS version. It is better to check the message for clues regarding the exception // as that is somewhat consistent across OEMs. // For instance, the Exception thrown on OxygenOS 11 is a RuntimeException but the message contains the required strings. // https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/webkit/WebViewFactory.java#348 if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || (message!=null && (message.contains("WebView provider") || message.contains("No WebView installed")))){ return null; } else { throw exception; } } } return mCookieManager; } ``` ## Changelog [General] [Added] - A fail proof check to catch any crash involving webview: if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || (message!=null && (message.contains("WebView provider") || message.contains("No WebView installed")))) [General] [Removed] - Flawed check to catch WebViewProvider crash: if (message != null && exception.getClass().getCanonicalName().contains("MissingWebViewPackageException")) Pull Request resolved: facebook#33088 Test Plan: This code has been tested rigorously on OnePlus Nord CE 5G (Oxygen OS 11.0) and Realme X (Realme UI 2.0) both running on Android 11 and reproducing the crash on a hybrid (Native android + ReactNative) app that showed this crash in production being dependent on WebViews. I have implemented an entire patched fork of 0.67.2 to fix this issue in our app. How to test: Launch any app that has a webview. Go to settings->apps->Android System Webview -> disable. Resume/Restart the app. In this fix: Open a webview and notice that the app will not crash. In current version: App crashes on startup as the exception escapes the catch block. Even if it survives startup (did on Realme X), it will crash once you try to open a webview. Reviewed By: rh389 Differential Revision: D34240097 Pulled By: cortinico fbshipit-source-id: 0f1f9a3b078c0ad3074c7841392892cb70b427eb
This still giving error, log from Crashlytics using React Native v0.71.11
|
Summary
We upgraded to RN 0.62.2 on our latest release and started to see again the "Failed to load WebView provider: No WebView installed" (see below for Crashlytics screenshot)
This crash had been fixed by #24533 but #26189 (added in 0.62) reverted the fix
Indeed the exception raised in Crashlytics is actually a
AndroidRuntimeException
andMissingWebViewPackageException
is only part of the message.For instance, in the screenshot above, the exception message is
android.webkit.WebViewFactory$MissingWebViewPackageException: Failed to load WebView provider: No WebView installed
Now these crashes are quite tricky to reproduce, so to be on the safe side, I'm filtering out all exceptions containing
WebView
as suggested by @thorbenprimke on the original fix.If my reasoning is correct, it should fix @siddhantsoni 's issue as well, since
WebView
is included inMissingWebViewPackageException
But following that reasoning, I am not sure #26189 fixed @siddhantsoni 's issue, so @siddhantsoni if you could check that this PR also fixes your issue, that would be great!
Changelog
[Android] [Fixed] - Fix missing WebView provider crash in ForwardingCookieHandler
Test Plan
I created a version of react native with this patch applied
Before the fix ~0.1% of our users were impacted on Android, no new crashes have occurred after the update.
This is putting back what was already in place and working for us, but making the check wider to catch more errors.