Cronvoy: Map EM Errors to Cronet API Errors II (#1594) (#2633)

Description: As a follow up for PR2568, the PR maps the network_disconnected and quic exception errors. The network_changed error has been removed as this is not relevant to envoymobile. Signed-off-by: Chidera Olibie <[email protected]> Signed-off-by: JP Simard <[email protected]>
envoyproxy · Nov 29, 2022 · 54322e0 · 54322e0
1 parent f5c4c1f
commit 54322e0
Show file tree

Hide file tree

Showing 13 changed files with 336 additions and 167 deletions.
diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java
@@ -16,6 +16,7 @@
 import android.net.NetworkInfo;
 import android.net.NetworkRequest;
 import android.os.Build;
+import androidx.annotation.VisibleForTesting;
 import androidx.core.content.ContextCompat;
 
 import java.util.Collections;
@@ -104,11 +105,20 @@ public void onLost(final Network network) {
     }
   }
 
+  /** @returns The singleton instance of {@link AndroidNetworkMonitor}. */
+  public static AndroidNetworkMonitor getInstance() {
+    assert instance != null;
+    return instance;
+  }
+
   @Override
   public void onReceive(Context context, Intent intent) {
     handleNetworkChange();
   }
 
+  /** @returns True if there is connectivity */
+  public boolean isOnline() { return previousNetworkType != -1; }
+
   private void handleNetworkChange() {
     NetworkInfo networkInfo = connectivityManager.getActiveNetworkInfo();
     int networkType = networkInfo == null ? -1 : networkInfo.getType();
@@ -128,4 +138,10 @@ private void handleNetworkChange() {
       envoyEngine.setPreferredNetwork(EnvoyNetworkType.ENVOY_NETWORK_TYPE_GENERIC);
     }
   }
+
+  /** Expose connectivityManager only for testing */
+  @VisibleForTesting
+  public ConnectivityManager getConnectivityManager() {
+    return connectivityManager;
+  }
 }
diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/BUILD b/mobile/library/java/io/envoyproxy/envoymobile/engine/BUILD
@@ -10,6 +10,7 @@ android_library(
         "AndroidJniLibrary.java",
         "AndroidNetworkMonitor.java",
         "AndroidProxyMonitor.java",
+        "UpstreamHttpProtocol.java",
     ],
     custom_package = "io.envoyproxy.envoymobile.engine",
     manifest = "AndroidEngineManifest.xml",
@@ -18,6 +19,7 @@ android_library(
         "//library/java/io/envoyproxy/envoymobile/engine:envoy_base_engine_lib",
         "//library/java/io/envoyproxy/envoymobile/engine/types:envoy_c_types_lib",
         "//library/java/org/chromium/net",
+        "@maven//:androidx_annotation_annotation",
         "@maven//:androidx_core_core",
     ],
 )

diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/UpstreamHttpProtocol.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/UpstreamHttpProtocol.java
@@ -0,0 +1,20 @@
+package io.envoyproxy.envoymobile.engine;
+
+import androidx.annotation.LongDef;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * The upstream protocol, if an upstream connection was established. Field
+ * entries are based off of Envoy's Http::Protocol
+ * https://github.com/envoyproxy/envoy/blob/main/envoy/http/protocol.h
+ */
+@LongDef({UpstreamHttpProtocol.HTTP10, UpstreamHttpProtocol.HTTP11, UpstreamHttpProtocol.HTTP2,
+          UpstreamHttpProtocol.HTTP3})
+@Retention(RetentionPolicy.SOURCE)
+public @interface UpstreamHttpProtocol {
+  long HTTP10 = 0;
+  long HTTP11 = 1;
+  long HTTP2 = 2;
+  long HTTP3 = 3;
+}
diff --git a/mobile/library/java/org/chromium/net/impl/CronetBidirectionalStream.java b/mobile/library/java/org/chromium/net/impl/CronetBidirectionalStream.java
@@ -653,13 +653,13 @@ private void onErrorReceived(int errorCode, EnvoyFinalStreamIntel finalStreamInt
       mResponseInfo.setReceivedByteCount(finalStreamIntel.getReceivedByteCount());
     }
 
-    NetError netError = mapEnvoyMobileErrorToNetError(finalStreamIntel.getResponseFlags());
+    NetError netError = mapEnvoyMobileErrorToNetError(finalStreamIntel);
     int javaError = mapNetErrorToCronetApiErrorCode(netError);
 
     if (isQuicException(javaError)) {
       mException.set(new QuicExceptionImpl("Exception in BidirectionalStream: " + netError,
                                            javaError, netError.getErrorCode(),
-                                           /*nativeQuicError*/ 0));
+                                           Errors.QUIC_INTERNAL_ERROR));
     } else {
       mException.set(new BidirectionalStreamNetworkException(
           "Exception in BidirectionalStream: " + netError, javaError, netError.getErrorCode()));

diff --git a/mobile/library/java/org/chromium/net/impl/CronetUrlRequest.java b/mobile/library/java/org/chromium/net/impl/CronetUrlRequest.java
@@ -934,13 +934,13 @@ public void onError(int errorCode, String message, int attemptCount,
         return;
       }
 
-      NetError netError = mapEnvoyMobileErrorToNetError(finalStreamIntel.getResponseFlags());
+      NetError netError = mapEnvoyMobileErrorToNetError(finalStreamIntel);
       int javaError = mapNetErrorToCronetApiErrorCode(netError);
 
       if (isQuicException(javaError)) {
         enterErrorState(new QuicExceptionImpl("Exception in CronetUrlRequest: " + netError,
                                               javaError, netError.getErrorCode(),
-                                              /*nativeQuicError*/ 0));
+                                              Errors.QUIC_INTERNAL_ERROR));
         return;
       }
 

diff --git a/mobile/library/java/org/chromium/net/impl/Errors.java b/mobile/library/java/org/chromium/net/impl/Errors.java
@@ -2,6 +2,9 @@
 
 import android.util.Log;
 import androidx.annotation.LongDef;
+import io.envoyproxy.envoymobile.engine.AndroidNetworkMonitor;
+import io.envoyproxy.envoymobile.engine.UpstreamHttpProtocol;
+import io.envoyproxy.envoymobile.engine.types.EnvoyFinalStreamIntel;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.Collections;
@@ -14,6 +17,8 @@
  * from Envoymobile error to Chromium neterror and finally to the public Network Exception.
  */
 public class Errors {
+  // This represents a nativeQuicError since we don't expose individual quic errors yet.
+  public static final int QUIC_INTERNAL_ERROR = 1;
   private static final Map<Long, NetError> ENVOYMOBILE_ERROR_TO_NET_ERROR = buildErrorMap();
 
   /**Subset of errors defined in
@@ -67,13 +72,20 @@ public String toString() {
    * @param responseFlag envoymobile's finalStreamIntel responseFlag
    * @return the NetError that the EnvoyMobileError maps to
    */
-  public static NetError mapEnvoyMobileErrorToNetError(long responseFlag) {
-    /* Todo(https://github.com/envoyproxy/envoy-mobile/issues/1594):
-     * if (EnvoyMobileError.DNS_RESOLUTION_FAILED || EnvoyMobileError.UPSTREAM_CONNECTION_FAILURE)
-     * && NetworkChangeNotifier.isOffline return NetError.ERR_INTERNET_DISCONNECTED
-     *
-     * if negotiated_protocol is quic return QUIC_PROTOCOL_FAILED
-     */
+  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 ||
+         responseFlag == EnvoyMobileError.UPSTREAM_CONNECTION_FAILURE) &&
+        !AndroidNetworkMonitor.getInstance().isOnline()) {
+      return NetError.ERR_INTERNET_DISCONNECTED;
+    }
+
+    // Check if negotiated_protocol is quic
+    if (finalStreamIntel.getUpstreamProtocol() == UpstreamHttpProtocol.HTTP3) {
+      return NetError.ERR_QUIC_PROTOCOL_ERROR;
+    }
+
     return ENVOYMOBILE_ERROR_TO_NET_ERROR.getOrDefault(responseFlag, NetError.ERR_OTHER);
   }
 

diff --git a/mobile/test/integration/filters/http/test_read/filter.cc b/mobile/test/integration/filters/http/test_read/filter.cc
@@ -8,17 +8,30 @@ namespace TestRead {
 
 Http::FilterHeadersStatus TestReadFilter::decodeHeaders(Http::RequestHeaderMap& request_headers,
                                                         bool) {
-  // sample path is /failed?start=0x10000
+  // sample path is /failed?error=0x10000
   Http::Utility::QueryParams query_parameters =
       Http::Utility::parseQueryString(request_headers.Path()->value().getStringView());
+  auto error = query_parameters.find("error");
   uint64_t response_flag;
-  if (absl::SimpleAtoi(query_parameters.at("start"), &response_flag)) {
-    decoder_callbacks_->streamInfo().setResponseFlag(
-        TestReadFilter::mapErrorToResponseFlag(response_flag));
+  if (error != query_parameters.end() && absl::SimpleAtoi(error->second, &response_flag)) {
+    // set response error code
+    StreamInfo::StreamInfo& stream_info = decoder_callbacks_->streamInfo();
+    stream_info.setResponseFlag(TestReadFilter::mapErrorToResponseFlag(response_flag));
+
+    // check if we want a quic server error: sample path is /failed?quic=1&error=0x10000
+    if (query_parameters.find("quic") != query_parameters.end()) {
+      stream_info.setUpstreamInfo(std::make_shared<StreamInfo::UpstreamInfoImpl>());
+      stream_info.upstreamInfo()->setUpstreamProtocol(Http::Protocol::Http3);
+    }
+
+    // trigger the error and stop iteration to other filters
     decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, "test_read filter threw: ", nullptr,
                                        absl::nullopt, "");
+    return Http::FilterHeadersStatus::StopIteration;
   }
-  return Http::FilterHeadersStatus::StopIteration;
+
+  // continue to other filters since the provided query string is invalid for error simulation
+  return Http::FilterHeadersStatus::Continue;
 }
 
 StreamInfo::ResponseFlag TestReadFilter::mapErrorToResponseFlag(uint64_t errorCode) {

diff --git a/mobile/test/integration/filters/http/test_read/filter.h b/mobile/test/integration/filters/http/test_read/filter.h
@@ -14,7 +14,9 @@ namespace HttpFilters {
 namespace TestRead {
 
 /**
- * This is a test-only filter to return specified error code based on a request header.
+ * This is a test-only filter to return specified error code based on a request url query string.
+ * It either simulates the requested error if the url query matches the error patterns
+ * or does nothing
  */
 class TestReadFilter final : public Http::PassThroughFilter,
                              public Logger::Loggable<Logger::Id::filter> {

diff --git a/mobile/test/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitorTest.java b/mobile/test/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitorTest.java
@@ -0,0 +1,68 @@
+package io.envoyproxy.envoymobile.engine;
+
+import static org.robolectric.Shadows.shadowOf;
+
+import android.content.Context;
+import android.content.Intent;
+import android.Manifest;
+import android.net.ConnectivityManager;
+import android.net.NetworkInfo;
+import androidx.test.filters.MediumTest;
+import androidx.test.platform.app.InstrumentationRegistry;
+import androidx.test.rule.GrantPermissionRule;
+import androidx.test.annotation.UiThreadTest;
+import io.envoyproxy.envoymobile.MockEnvoyEngine;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.shadows.ShadowConnectivityManager;
+
+/**
+ * Tests functionality of AndroidNetworkMonitor
+ */
+@RunWith(RobolectricTestRunner.class)
+public class AndroidNetworkMonitorTest {
+
+  @Rule
+  public GrantPermissionRule mRuntimePermissionRule =
+      GrantPermissionRule.grant(Manifest.permission.ACCESS_NETWORK_STATE);
+
+  private AndroidNetworkMonitor androidNetworkMonitor;
+  private ShadowConnectivityManager connectivityManager;
+  private Context context;
+
+  @Before
+  public void setUp() {
+    context = InstrumentationRegistry.getInstrumentation().getTargetContext();
+    AndroidNetworkMonitor.load(context, new MockEnvoyEngine());
+    androidNetworkMonitor = AndroidNetworkMonitor.getInstance();
+    connectivityManager = shadowOf(androidNetworkMonitor.getConnectivityManager());
+  }
+
+  /**
+   * Tests that isOnline() returns the correct result.
+   */
+  @Test
+  @MediumTest
+  @UiThreadTest
+  public void testAndroidNetworkMonitorIsOnline() {
+    Intent intent = new Intent(ConnectivityManager.CONNECTIVITY_ACTION);
+    // Set up network change
+    androidNetworkMonitor.onReceive(context, intent);
+    Assert.assertTrue(androidNetworkMonitor.isOnline());
+
+    // Save old networkInfo and simulate a no network scenerio
+    NetworkInfo networkInfo = androidNetworkMonitor.getConnectivityManager().getActiveNetworkInfo();
+    connectivityManager.setActiveNetworkInfo(null);
+    androidNetworkMonitor.onReceive(context, intent);
+    Assert.assertFalse(androidNetworkMonitor.isOnline());
+
+    // Bring back online since the AndroidNetworkMonitor class is a singleton
+    connectivityManager.setActiveNetworkInfo(networkInfo);
+    androidNetworkMonitor.onReceive(context, intent);
+  }
+}
diff --git a/mobile/test/java/io/envoyproxy/envoymobile/engine/BUILD b/mobile/test/java/io/envoyproxy/envoymobile/engine/BUILD
@@ -1,4 +1,4 @@
-load("@envoy_mobile//bazel:kotlin_test.bzl", "envoy_mobile_kt_test")
+load("@envoy_mobile//bazel:kotlin_test.bzl", "envoy_mobile_android_test", "envoy_mobile_kt_test")
 
 licenses(["notice"])  # Apache 2
 
@@ -31,3 +31,19 @@ envoy_mobile_kt_test(
         "//library/java/io/envoyproxy/envoymobile/engine:envoy_base_engine_lib",
     ],
 )
+
+envoy_mobile_android_test(
+    name = "android_network_monitor_tests",
+    srcs = [
+        "AndroidNetworkMonitorTest.java",
+    ],
+    native_deps = [
+        "//library/common/jni:libndk_envoy_jni.so",
+        "//library/common/jni:libndk_envoy_jni.jnilib",
+    ],
+    deps = [
+        "//library/java/io/envoyproxy/envoymobile/engine:envoy_base_engine_lib",
+        "//library/java/io/envoyproxy/envoymobile/engine:envoy_engine_lib",
+        "//library/kotlin/io/envoyproxy/envoymobile:envoy_interfaces_lib",
+    ],
+)