From 9640c4295e0f23eff100eb89d27503db8944e849 Mon Sep 17 00:00:00 2001 From: Fredy Wijaya Date: Thu, 23 Jan 2025 14:43:22 -0600 Subject: [PATCH] mobile: Fix the internet disconnection error mapping (#38169) This PR fixes the error mapping for the internet disconnection in Cronvoy to only check the network status instead of looking at the other errors because when the network is offline, the only logical error is `ERR_INTERNET_DISCONNECTED` irrespective of the other errors. Risk Level: low Testing: unit test Docs Changes: n/a Release Notes: n/a Platform Specific Features: android --------- Signed-off-by: Fredy Wijaya --- .../java/org/chromium/net/impl/Errors.java | 7 +-- .../org/chromium/net/CronetHttp3Test.java | 7 +++ .../org/chromium/net/impl/ErrorsTest.java | 48 +++++++++++++++++++ 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/mobile/library/java/org/chromium/net/impl/Errors.java b/mobile/library/java/org/chromium/net/impl/Errors.java index 7cdd8641ac2b..a5ce8c3b4e69 100644 --- a/mobile/library/java/org/chromium/net/impl/Errors.java +++ b/mobile/library/java/org/chromium/net/impl/Errors.java @@ -75,14 +75,11 @@ public String toString() { * @return the NetError that the EnvoyMobileError maps to */ public static NetError mapEnvoyMobileErrorToNetError(EnvoyFinalStreamIntel finalStreamIntel) { - // if connection fails to be established, check if user is offline - long responseFlag = finalStreamIntel.getResponseFlags(); - if (((responseFlag & EnvoyMobileError.DNS_RESOLUTION_FAILED) != 0 || - (responseFlag & EnvoyMobileError.UPSTREAM_CONNECTION_FAILURE) != 0) && - !AndroidNetworkMonitor.getInstance().isOnline()) { + if (!AndroidNetworkMonitor.getInstance().isOnline()) { return NetError.ERR_INTERNET_DISCONNECTED; } + long responseFlag = finalStreamIntel.getResponseFlags(); // This will only map the first matched error to a NetError code. for (Map.Entry entry : ENVOYMOBILE_ERROR_TO_NET_ERROR.entrySet()) { if ((responseFlag & entry.getKey()) != 0) { diff --git a/mobile/test/java/org/chromium/net/CronetHttp3Test.java b/mobile/test/java/org/chromium/net/CronetHttp3Test.java index b6f2aa59f3d8..941105e4f427 100644 --- a/mobile/test/java/org/chromium/net/CronetHttp3Test.java +++ b/mobile/test/java/org/chromium/net/CronetHttp3Test.java @@ -3,6 +3,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import android.Manifest; + import io.envoyproxy.envoymobile.engine.types.EnvoyNetworkType; import org.chromium.net.impl.CronvoyUrlRequestContext; import io.envoyproxy.envoymobile.engine.EnvoyEngine; @@ -10,6 +12,7 @@ import androidx.test.core.app.ApplicationProvider; import org.chromium.net.testing.TestUploadDataProvider; import androidx.test.filters.SmallTest; +import androidx.test.rule.GrantPermissionRule; import org.chromium.net.impl.NativeCronvoyEngineBuilderImpl; import org.chromium.net.testing.CronetTestRule; @@ -32,6 +35,10 @@ */ @RunWith(RobolectricTestRunner.class) public class CronetHttp3Test { + @Rule + public GrantPermissionRule grantPermissionRule = + GrantPermissionRule.grant(Manifest.permission.ACCESS_NETWORK_STATE); + @Rule public final CronetTestRule mTestRule = new CronetTestRule(); private static final String TAG = CronetHttp3Test.class.getSimpleName(); diff --git a/mobile/test/java/org/chromium/net/impl/ErrorsTest.java b/mobile/test/java/org/chromium/net/impl/ErrorsTest.java index dd1db3b8ee0d..3603e0dfaab2 100644 --- a/mobile/test/java/org/chromium/net/impl/ErrorsTest.java +++ b/mobile/test/java/org/chromium/net/impl/ErrorsTest.java @@ -2,17 +2,65 @@ import static org.chromium.net.impl.Errors.mapEnvoyMobileErrorToNetError; import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.robolectric.Shadows.shadowOf; +import android.Manifest; +import android.content.Context; +import android.net.ConnectivityManager; +import android.net.NetworkCapabilities; + +import androidx.test.platform.app.InstrumentationRegistry; +import androidx.test.rule.GrantPermissionRule; + +import io.envoyproxy.envoymobile.engine.AndroidNetworkMonitor; +import io.envoyproxy.envoymobile.engine.EnvoyEngine; import io.envoyproxy.envoymobile.engine.UpstreamHttpProtocol; import io.envoyproxy.envoymobile.engine.types.EnvoyFinalStreamIntel; import org.chromium.net.impl.Errors.NetError; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; import org.junit.runner.RunWith; import org.junit.Test; import org.robolectric.RobolectricTestRunner; @RunWith(RobolectricTestRunner.class) public class ErrorsTest { + @Rule + public GrantPermissionRule grantPermissionRule = + GrantPermissionRule.grant(Manifest.permission.ACCESS_NETWORK_STATE); + + private NetworkCapabilities networkCapabilities; + + @Before + public void setUp() { + Context context = InstrumentationRegistry.getInstrumentation().getTargetContext(); + AndroidNetworkMonitor.load(context, mock(EnvoyEngine.class)); + ConnectivityManager connectivityManager = + AndroidNetworkMonitor.getInstance().getConnectivityManager(); + networkCapabilities = + connectivityManager.getNetworkCapabilities(connectivityManager.getActiveNetwork()); + + shadowOf(networkCapabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + } + + @After + public void tearDown() { + AndroidNetworkMonitor.shutdown(); + } + + @Test + public void testMapEnvoyMobileErrorToInternetDisconnected() { + shadowOf(networkCapabilities).removeCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + + long responseFlags = 1L << 4; + EnvoyFinalStreamIntel intel = constructStreamIntel(responseFlags, UpstreamHttpProtocol.HTTP3); + NetError error = mapEnvoyMobileErrorToNetError(intel); + assertEquals(NetError.ERR_INTERNET_DISCONNECTED, error); + } + @Test public void testMapEnvoyMobileErrorToNetErrorHttp3() throws Exception { // 8 corresponds to NoRouteFound in StreamInfo::CoreResponseFlag: