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

bazel/envoy_mobile_test_extensions.bzl

@@ -6,4 +6,5 @@ TEST_EXTENSIONS = [
     "//library/common/extensions/filters/http/test_accessor:config",
     "//library/common/extensions/filters/http/test_event_tracker:config",
     "//library/common/extensions/filters/http/test_kv_store:config",
+    "//test/integration/filters/http/test_read:config",
 ]

library/java/org/chromium/net/impl/Annotations.java

@@ -19,16 +19,6 @@ public final class Annotations {
     int HIGHEST = 5;
   }
 
-  /** Subset of errors defined in chromium/src/net/base/net_error_list.h */
-  @IntDef({NetError.ERR_HTTP2_PING_FAILED, NetError.ERR_QUIC_HANDSHAKE_FAILED})
-  @Retention(RetentionPolicy.SOURCE)
-  public @interface NetError {
-    int ERR_NETWORK_CHANGED = -21;
-    int ERR_HTTP2_PING_FAILED = -352;
-    int ERR_QUIC_PROTOCOL_ERROR = -356;
-    int ERR_QUIC_HANDSHAKE_FAILED = -358;
-  }
-
   /** Enum defined here: chromium/src/components/cronet/url_request_context_config.h, line 37 */
   @IntDef({HttpCacheType.DISABLED, HttpCacheType.DISK, HttpCacheType.MEMORY})
   @Retention(RetentionPolicy.SOURCE)

library/java/org/chromium/net/impl/BUILD

@@ -24,6 +24,7 @@ android_library(
         "CronetUploadDataStream.java",
         "CronetUrlRequest.java",
         "CronetUrlRequestContext.java",
+        "Errors.java",
         "Executors.java",
         "HttpReason.java",
         "ImplVersion.java",

library/java/org/chromium/net/impl/BidirectionalStreamNetworkException.java

@@ -1,6 +1,6 @@
 package org.chromium.net.impl;
 
-import org.chromium.net.impl.Annotations.NetError;
+import org.chromium.net.impl.Errors.NetError;
 
 /**
  * Used in {@link CronetBidirectionalStream}. Implements {@link NetworkExceptionImpl}.
@@ -13,13 +13,11 @@ public BidirectionalStreamNetworkException(String message, int errorCode,
 
   @Override
   public boolean immediatelyRetryable() {
-    switch (mCronetInternalErrorCode) {
-    case NetError.ERR_HTTP2_PING_FAILED:
-    case NetError.ERR_QUIC_HANDSHAKE_FAILED:
+    if (mCronetInternalErrorCode == NetError.ERR_HTTP2_PING_FAILED.getErrorCode() ||
+        mCronetInternalErrorCode == NetError.ERR_QUIC_HANDSHAKE_FAILED.getErrorCode()) {
       assert mErrorCode == ERROR_OTHER;
       return true;
-    default:
-      return super.immediatelyRetryable();
     }
+    return super.immediatelyRetryable();
   }
 }

library/java/org/chromium/net/impl/CronetBidirectionalStream.java

@@ -1,5 +1,9 @@
 package org.chromium.net.impl;
 
+import static org.chromium.net.impl.Errors.isQuicException;
+import static org.chromium.net.impl.Errors.mapEnvoyMobileErrorToNetError;
+import static org.chromium.net.impl.Errors.mapNetErrorToCronetApiErrorCode;
+
 import android.util.Log;
 
 import androidx.annotation.Nullable;
@@ -15,6 +19,7 @@
 import org.chromium.net.impl.Annotations.RequestPriority;
 import org.chromium.net.impl.CronetBidirectionalState.Event;
 import org.chromium.net.impl.CronetBidirectionalState.NextAction;
+import org.chromium.net.impl.Errors.NetError;
 import org.chromium.net.impl.UrlResponseInfoImpl.HeaderBlockImpl;
 
 import java.net.MalformedURLException;
@@ -643,21 +648,23 @@ public void run() {
     });
   }
 
-  private void onErrorReceived(int errorCode, int nativeError, int nativeQuicError,
-                               String errorString, long receivedByteCount) {
+  private void onErrorReceived(int errorCode, EnvoyFinalStreamIntel finalStreamIntel) {
     if (mResponseInfo != null) {
-      mResponseInfo.setReceivedByteCount(receivedByteCount);
+      mResponseInfo.setReceivedByteCount(finalStreamIntel.getReceivedByteCount());
     }
-    CronetException exception;
-    if (errorCode == NetworkException.ERROR_QUIC_PROTOCOL_FAILED ||
-        errorCode == NetworkException.ERROR_NETWORK_CHANGED) {
-      exception = new QuicExceptionImpl("Exception in BidirectionalStream: " + errorString,
-                                        errorCode, nativeError, nativeQuicError);
+
+    NetError netError = mapEnvoyMobileErrorToNetError(finalStreamIntel.getResponseFlags());
+    int javaError = mapNetErrorToCronetApiErrorCode(netError);
+
+    if (isQuicException(javaError)) {
+      mException.set(new QuicExceptionImpl("Exception in BidirectionalStream: " + netError,
+                                           javaError, netError.getErrorCode(),
+                                           /*nativeQuicError*/ 0));
     } else {
-      exception = new BidirectionalStreamNetworkException(
-          "Exception in BidirectionalStream: " + errorString, errorCode, nativeError);
+      mException.set(new BidirectionalStreamNetworkException(
+          "Exception in BidirectionalStream: " + netError, javaError, netError.getErrorCode()));
     }
-    mException.set(exception);
+
     failWithException();
   }
 
@@ -1031,9 +1038,7 @@ public void onError(int errorCode, String message, int attemptCount, EnvoyStream
     mEnvoyFinalStreamIntel = finalStreamIntel;
     switch (mState.nextAction(Event.ON_ERROR)) {
     case NextAction.NOTIFY_USER_NETWORK_ERROR:
-      // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1594): fix error scheme.
-      onErrorReceived(errorCode, /* nativeError= */ -1,
-                      /* nativeQuicError */ 0, message, finalStreamIntel.getReceivedByteCount());
+      onErrorReceived(errorCode, finalStreamIntel);
       break;
     case NextAction.NOTIFY_USER_FAILED:
       // There was already an error in-progress - the network error came too late and is ignored.

library/java/org/chromium/net/impl/CronetUrlRequest.java

@@ -1,5 +1,9 @@
 package org.chromium.net.impl;
 
+import static org.chromium.net.impl.Errors.isQuicException;
+import static org.chromium.net.impl.Errors.mapEnvoyMobileErrorToNetError;
+import static org.chromium.net.impl.Errors.mapNetErrorToCronetApiErrorCode;
+
 import android.os.ConditionVariable;
 import android.util.Log;
 import androidx.annotation.IntDef;
@@ -30,9 +34,11 @@
 import org.chromium.net.CallbackException;
 import org.chromium.net.CronetException;
 import org.chromium.net.InlineExecutionProhibitedException;
+import org.chromium.net.NetworkException;
 import org.chromium.net.RequestFinishedInfo;
 import org.chromium.net.RequestFinishedInfo.Metrics;
 import org.chromium.net.UploadDataProvider;
+import org.chromium.net.impl.Errors.NetError;
 
 /** UrlRequest, backed by Envoy-Mobile. */
 public final class CronetUrlRequest extends UrlRequestBase {
@@ -448,6 +454,7 @@ private static int determineNextErrorState(boolean streamEnded, @State int origi
     }
   }
 
+  // No-op if already in a terminal state.
   private void enterErrorState(CronetException error) {
     @State int originalState;
     @State int updatedState;
@@ -927,10 +934,18 @@ public void onError(int errorCode, String message, int attemptCount,
         return;
       }
 
-      String errorMessage = "failed with error after " + attemptCount + " attempts. Message=[" +
-                            message + "] Code=[" + errorCode + "]";
-      CronetException exception = new CronetExceptionImpl(errorMessage, /* cause= */ null);
-      enterErrorState(exception); // No-op if already in a terminal state.
+      NetError netError = mapEnvoyMobileErrorToNetError(finalStreamIntel.getResponseFlags());
+      int javaError = mapNetErrorToCronetApiErrorCode(netError);
+
+      if (isQuicException(javaError)) {
+        enterErrorState(new QuicExceptionImpl("Exception in CronetUrlRequest: " + netError,
+                                              javaError, netError.getErrorCode(),
+                                              /*nativeQuicError*/ 0));
+        return;
+      }
+
+      enterErrorState(new NetworkExceptionImpl("Exception in CronetUrlRequest: " + netError,
+                                               javaError, netError.getErrorCode()));
     }
 
     @Override

library/java/org/chromium/net/impl/Errors.java

@@ -0,0 +1,129 @@
+package org.chromium.net.impl;
+
+import android.util.Log;
+import androidx.annotation.LongDef;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import org.chromium.net.NetworkException;
+
+/**
+ * Handles mapping of the error codes that exist in the Cronvoy space. That is,
+ * from Envoymobile error to Chromium neterror and finally to the public Network Exception.
+ */
+public class Errors {
+  private static final Map<Long, NetError> ENVOYMOBILE_ERROR_TO_NET_ERROR = buildErrorMap();
+
+  /**Subset of errors defined in
+   * https://github.com/envoyproxy/envoy/blob/main/envoy/stream_info/stream_info.h */
+  @LongDef(flag = true,
+           value = {EnvoyMobileError.DNS_RESOLUTION_FAILED, EnvoyMobileError.DURATION_TIMEOUT,
+                    EnvoyMobileError.STREAM_IDLE_TIMEOUT,
+                    EnvoyMobileError.UPSTREAM_CONNECTION_FAILURE,
+                    EnvoyMobileError.UPSTREAM_CONNECTION_TERMINATION,
+                    EnvoyMobileError.UPSTREAM_REMOTE_RESET})
+  @Retention(RetentionPolicy.SOURCE)
+  public @interface EnvoyMobileError {
+    long DNS_RESOLUTION_FAILED = 0x4000000;
+    long DURATION_TIMEOUT = 0x400000;
+    long STREAM_IDLE_TIMEOUT = 0x10000;
+    long UPSTREAM_CONNECTION_FAILURE = 0x20;
+    long UPSTREAM_CONNECTION_TERMINATION = 0x40;
+    long UPSTREAM_REMOTE_RESET = 0x10;
+  }
+
+  /** Subset of errors defined in chromium/src/net/base/net_error_list.h */
+  public enum NetError {
+    ERR_NETWORK_CHANGED(-21),
+    ERR_HTTP2_PING_FAILED(-352),
+    ERR_QUIC_PROTOCOL_ERROR(-356),
+    ERR_QUIC_HANDSHAKE_FAILED(-358),
+    ERR_NAME_NOT_RESOLVED(-105),
+    ERR_INTERNET_DISCONNECTED(-106),
+    ERR_TIMED_OUT(-7),
+    ERR_CONNECTION_CLOSED(-100),
+    ERR_CONNECTION_TIMED_OUT(-118),
+    ERR_CONNECTION_REFUSED(-102),
+    ERR_CONNECTION_RESET(-101),
+    ERR_ADDRESS_UNREACHABLE(-109),
+    ERR_OTHER(-1000);
+
+    private final int errorCode;
+
+    NetError(int errorCode) { this.errorCode = errorCode; }
+
+    public int getErrorCode() { return errorCode; }
+
+    @Override
+    public String toString() {
+      return "net::" + name();
+    }
+  }
+
+  /**
+   * Maps Envoymobile's errorcode to chromium's net errorcode
+   * @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
+     */
+    return ENVOYMOBILE_ERROR_TO_NET_ERROR.getOrDefault(responseFlag, NetError.ERR_OTHER);
+  }
+
+  /**
+   * Maps chromium's net errorcode to Cronet API errorcode
+   * @return the corresponding NetworkException errorcode
+   */
+  public static int mapNetErrorToCronetApiErrorCode(NetError netError) {
+    switch (netError) {
+    case ERR_NAME_NOT_RESOLVED:
+      return NetworkException.ERROR_HOSTNAME_NOT_RESOLVED;
+    case ERR_TIMED_OUT:
+      return NetworkException.ERROR_TIMED_OUT;
+    case ERR_CONNECTION_CLOSED:
+      return NetworkException.ERROR_CONNECTION_CLOSED;
+    case ERR_CONNECTION_RESET:
+      return NetworkException.ERROR_CONNECTION_RESET;
+    case ERR_CONNECTION_REFUSED:
+      return NetworkException.ERROR_CONNECTION_REFUSED;
+    case ERR_OTHER:
+      return NetworkException.ERROR_OTHER;
+    case ERR_INTERNET_DISCONNECTED:
+      return NetworkException.ERROR_INTERNET_DISCONNECTED;
+    case ERR_NETWORK_CHANGED:
+      return NetworkException.ERROR_NETWORK_CHANGED;
+    case ERR_QUIC_PROTOCOL_ERROR:
+      return NetworkException.ERROR_QUIC_PROTOCOL_FAILED;
+    }
+    Log.e(CronetUrlRequestContext.LOG_TAG, "Unknown error code: " + netError);
+    return NetworkException.ERROR_OTHER;
+  }
+
+  /**
+   * Returns {@code true} if the error may contain QUIC specific errorcode
+   */
+  public static boolean isQuicException(int javaError) {
+    return javaError == NetworkException.ERROR_QUIC_PROTOCOL_FAILED ||
+        javaError == NetworkException.ERROR_NETWORK_CHANGED;
+  }
+
+  private static Map<Long, NetError> buildErrorMap() {
+    Map<Long, NetError> errorMap = new HashMap<>();
+    errorMap.put(EnvoyMobileError.DNS_RESOLUTION_FAILED, NetError.ERR_NAME_NOT_RESOLVED);
+    errorMap.put(EnvoyMobileError.DURATION_TIMEOUT, NetError.ERR_TIMED_OUT);
+    errorMap.put(EnvoyMobileError.STREAM_IDLE_TIMEOUT, NetError.ERR_TIMED_OUT);
+    errorMap.put(EnvoyMobileError.UPSTREAM_CONNECTION_TERMINATION, NetError.ERR_CONNECTION_CLOSED);
+    errorMap.put(EnvoyMobileError.UPSTREAM_REMOTE_RESET, NetError.ERR_CONNECTION_RESET);
+    errorMap.put(EnvoyMobileError.UPSTREAM_CONNECTION_FAILURE, NetError.ERR_CONNECTION_REFUSED);
+    return Collections.unmodifiableMap(errorMap);
+  }
+
+  private Errors() {}
+}

library/java/org/chromium/net/impl/NativeCronetEngineBuilderImpl.java

@@ -29,6 +29,8 @@
  */
 public class NativeCronetEngineBuilderImpl extends CronetEngineBuilderImpl {
 
+  // TODO(refactor) move unshared variables into their specific methods.
+  private final List<EnvoyNativeFilterConfig> nativeFilterChain = new ArrayList<>();
   private final EnvoyLogger mEnvoyLogger = null;
   private final EnvoyEventTracker mEnvoyEventTracker = null;
   private boolean mAdminInterfaceEnabled = false;
@@ -81,6 +83,19 @@ public CronetEngineBuilderImpl setMockCertVerifierForTesting() {
     return this;
   }
 
+  /**
+   * Adds url interceptors to the cronetEngine
+   *
+   * @return the builder to facilitate chaining.
+   */
+  @VisibleForTesting
+  public CronetEngineBuilderImpl addUrlInterceptorsForTesting() {
+    nativeFilterChain.add(new EnvoyNativeFilterConfig(
+        "envoy.filters.http.test_read",
+        "{\"@type\": type.googleapis.com/envoymobile.test.integration.filters.http.test_read.TestRead}"));
+    return this;
+  }
+
   @Override
   public ExperimentalCronetEngine build() {
     if (getUserAgent() == null) {
@@ -100,7 +115,6 @@ EnvoyEngine createEngine(EnvoyOnEngineRunning onEngineRunning) {
 
   private EnvoyConfiguration createEnvoyConfiguration() {
     List<EnvoyHTTPFilterFactory> platformFilterChain = Collections.emptyList();
-    List<EnvoyNativeFilterConfig> nativeFilterChain = Collections.emptyList();
     Map<String, EnvoyStringAccessor> stringAccessors = Collections.emptyMap();
     Map<String, EnvoyKeyValueStore> keyValueStores = Collections.emptyMap();
     List<String> statSinks = Collections.emptyList();

test/integration/filters/http/test_read/BUILD

@@ -0,0 +1,42 @@
+load(
+    "@envoy//bazel:envoy_build_system.bzl",
+    "envoy_cc_extension",
+    "envoy_package",
+    "envoy_proto_library",
+)
+
+licenses(["notice"])  # Apache 2
+
+envoy_package()
+
+envoy_proto_library(
+    name = "filter",
+    srcs = ["filter.proto"],
+    deps = [
+        "@envoy_api//envoy/config/common/matcher/v3:pkg",
+    ],
+)
+
+envoy_cc_extension(
+    name = "test_read_filter_lib",
+    srcs = ["filter.cc"],
+    hdrs = ["filter.h"],
+    repository = "@envoy",
+    deps = [
+        "filter_cc_proto",
+        "@envoy//source/common/http:utility_lib",
+        "@envoy//source/common/stream_info:stream_info_lib",
+        "@envoy//source/extensions/filters/http/common:pass_through_filter_lib",
+    ],
+)
+
+envoy_cc_extension(
+    name = "config",
+    srcs = ["config.cc"],
+    hdrs = ["config.h"],
+    repository = "@envoy",
+    deps = [
+        ":test_read_filter_lib",
+        "@envoy//source/extensions/filters/http/common:factory_base_lib",
+    ],
+)