From 3f3394a5668d515e3476933aec67f07794c6e765 Mon Sep 17 00:00:00 2001 From: Rachit Mishra Date: Mon, 26 Sep 2022 13:30:14 -0700 Subject: [PATCH] fix: handle webview provider missing exception (#34456) Summary: The [existing fix](https://github.com/facebook/react-native/blob/fb936dfffb3ca2d9bc0969dfe36a70bdf66783e0/ReactAndroid/src/main/java/com/facebook/react/modules/network/ForwardingCookieHandler.java#L151) to handle missing WebView provider uses string comparison based checks to handle the exception gracefully, or otherwise simply throw the exception. This ends up crashing the app for the end user. Screenshot 2022-08-19 at 4 33 31 PM Fatal exceptions are bad in any case and not good user experience, we can gracefully handle this by [returning](https://github.com/facebook/react-native/compare/main...rachitmishra:react-native-1:patch-2#diff-f7ca1976002c4612051e4949395e64511b6f769e347c488e9a0d15cb5331fe76R141) `null` for all cases when WebView provider is not found. ## Changelog [Android] [Fixed] - Gracefully handle crash if no WebView provider is found on the device Pull Request resolved: https://github.com/facebook/react-native/pull/34456 Test Plan: IMO no testing is required as we were already returning null in certain cases after handling the exception message, also `ForwardingCookieManager::getCookieManager` is already marked `Nullable` so we are safe there. Reviewed By: lunaleaps Differential Revision: D39809020 Pulled By: cortinico fbshipit-source-id: 54b290ad7740859bdc84401904236c32761a4631 --- .../network/ForwardingCookieHandler.java | 33 +++++++------------ 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/network/ForwardingCookieHandler.java b/ReactAndroid/src/main/java/com/facebook/react/modules/network/ForwardingCookieHandler.java index 6b228d09aa7bc6..304c3a18915a8e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/network/ForwardingCookieHandler.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/network/ForwardingCookieHandler.java @@ -138,27 +138,18 @@ protected void doInBackgroundGuarded(Void... params) { // 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") - || message.contains("Cannot load WebView") - || message.contains("disableWebView") - || message.contains("WebView is disabled")))) { - return null; - } else { - throw exception; - } + // Ideally we would like to catch a `MissingWebViewPackageException` here. + // That API is private so we can't access it. + // Historically we used string matching on the error message to understand + // if the exception was a Missing Webview One. + // OEMs have been customizing that message making really hard to catch it. + // Therefore we result to returning null as a default instead of rethrowing + // the exception as it will result in a app crash at runtime. + // a) We will return null for all the other unhandled conditions when a webview provider is + // not found. + // b) We already have null checks in place for `getCookieManager()` calls. + // c) We have annotated the method as @Nullable to notify future devs about our return type. + return null; } }