From b6be31166f4f8dc6ed141b79614aaeb52a7b3df5 Mon Sep 17 00:00:00 2001 From: Rachit Mishra Date: Fri, 19 Aug 2022 16:32:16 +0530 Subject: [PATCH 1/4] fix: handle webview provider missing exception --- .../network/ForwardingCookieHandler.java | 22 +++---------------- 1 file changed, 3 insertions(+), 19 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 def2f4e3957e0d..f9cabee05ee501 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,25 +138,9 @@ 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")))) { - return null; - } else { - throw exception; - } + // fatal exception is no good for the user experience, + // return null in any case when a webview provider is not found. + return null; } } From 237bd91314e04d0cce0dfd21cabe733a2e2d5fcd Mon Sep 17 00:00:00 2001 From: Rachit Mishra Date: Fri, 23 Sep 2022 00:25:55 +0530 Subject: [PATCH 2/4] fix: update comment for graceful error handling --- .../modules/network/ForwardingCookieHandler.java | 14 ++------------ 1 file changed, 2 insertions(+), 12 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 c10b0de18d77f4..2303cdcb0fc451 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,18 +138,8 @@ protected void doInBackgroundGuarded(Void... params) { // https://bugs.chromium.org/p/chromium/issues/detail?id=559720 return null; } catch (Exception exception) { - // fatal exception is no good for the user experience, - // return null in any case when a webview provider is not found. - // 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 + // Fatal exceptions are not good for the user experience, we will return null + // for all the other unhandled conditions when a webview provider is not found. return null; } } From 8959daeacf402c3e49a1c566af1b428394214004 Mon Sep 17 00:00:00 2001 From: Rachit Mishra Date: Fri, 23 Sep 2022 00:37:20 +0530 Subject: [PATCH 3/4] docs: add more info to getCookieManager return --- .../react/modules/network/ForwardingCookieHandler.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 2303cdcb0fc451..d48ac8789edc07 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,8 +138,10 @@ protected void doInBackgroundGuarded(Void... params) { // https://bugs.chromium.org/p/chromium/issues/detail?id=559720 return null; } catch (Exception exception) { - // Fatal exceptions are not good for the user experience, we will return null - // for all the other unhandled conditions when a webview provider is not found. + // Fatal exceptions are not good for the user experience, + // 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; } } From eb3bedcb51f5a575c504dc0b94addb02bd77f8e2 Mon Sep 17 00:00:00 2001 From: Rachit Mishra Date: Sat, 24 Sep 2022 00:42:43 +0530 Subject: [PATCH 4/4] Update ReactAndroid/src/main/java/com/facebook/react/modules/network/ForwardingCookieHandler.java Co-authored-by: Nicola Corti --- .../react/modules/network/ForwardingCookieHandler.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 d48ac8789edc07..d0dbccdb4db484 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,7 +138,13 @@ protected void doInBackgroundGuarded(Void... params) { // https://bugs.chromium.org/p/chromium/issues/detail?id=559720 return null; } catch (Exception exception) { - // Fatal exceptions are not good for the user experience, + // 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.