mobile: Fix the internet disconnection error mapping (envoyproxy#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 <[email protected]>
makesoftwaresafe · Jan 23, 2025 · 6f5bdbf · 6f5bdbf
1 parent 6e8fa0a
commit 6f5bdbf
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 5 deletions.
diff --git 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<Long, NetError> 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
@@ -3,13 +3,16 @@
 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;
 import org.chromium.net.impl.CronvoyLogger;
 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
@@ -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: