Skip to content

Commit

Permalink
add quic exception and delete tests
Browse files Browse the repository at this point in the history
The mockurlrequestmock tests are not useful because the different
FailurPhases being tested are not used anywhere in the code.
Also deleted FailurePhase as it's also unneeded

Signed-off-by: Chidera Olibie <colibie@google.com>
  • Loading branch information
colibie committed Oct 24, 2022
1 parent 560265e commit 7bf274c
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 96 deletions.
1 change: 1 addition & 0 deletions library/java/io/envoyproxy/envoymobile/engine/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ android_library(
"AndroidJniLibrary.java",
"AndroidNetworkMonitor.java",
"AndroidProxyMonitor.java",
"UpstreamHttpProtocol.java",
],
custom_package = "io.envoyproxy.envoymobile.engine",
manifest = "AndroidEngineManifest.xml",
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
/*nativeQuicError*/ 1));
} else {
mException.set(new BidirectionalStreamNetworkException(
"Exception in BidirectionalStream: " + netError, javaError, netError.getErrorCode()));
Expand Down
4 changes: 2 additions & 2 deletions library/java/org/chromium/net/impl/CronetUrlRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
/*nativeQuicError*/ 1));
return;
}

Expand Down
15 changes: 10 additions & 5 deletions library/java/org/chromium/net/impl/Errors.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
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;
Expand Down Expand Up @@ -68,17 +70,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 negotiated_protocol is quic return QUIC_PROTOCOL_FAILED
*/
public static NetError mapEnvoyMobileErrorToNetError(EnvoyFinalStreamIntel finalStreamIntel) {
// Check if negotiated_protocol is quic
if (finalStreamIntel.getUpstreamProtocol() == UpstreamHttpProtocol.HTTP3) {
return NetError.ERR_QUIC_PROTOCOL_ERROR;
}

// 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) &&
responseFlag == EnvoyMobileError.UPSTREAM_CONNECTION_FAILURE) &&
!AndroidNetworkMonitor.getInstance().isOnline()) {
return NetError.ERR_INTERNET_DISCONNECTED;
}

return ENVOYMOBILE_ERROR_TO_NET_ERROR.getOrDefault(responseFlag, NetError.ERR_OTHER);
}

Expand Down
25 changes: 19 additions & 6 deletions test/integration/filters/http/test_read/filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,29 @@ 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");
// no error was specified, so move on
if (error == query_parameters.end()) {
return Http::FilterHeadersStatus::Continue;
}

StreamInfo::StreamInfo& stream_info = decoder_callbacks_->streamInfo();
uint64_t response_flag;
if (absl::SimpleAtoi(query_parameters.at("start"), &response_flag)) {
decoder_callbacks_->streamInfo().setResponseFlag(
TestReadFilter::mapErrorToResponseFlag(response_flag));
decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, "test_read filter threw: ", nullptr,
absl::nullopt, "");
// set response error code
if (absl::SimpleAtoi(error->second, &response_flag)) {
stream_info.setResponseFlag(TestReadFilter::mapErrorToResponseFlag(response_flag));
// check if we want a quic server error
if (query_parameters.find("quic") != query_parameters.end()) {
stream_info.setUpstreamInfo(std::make_shared<StreamInfo::UpstreamInfoImpl>());
stream_info.upstreamInfo()->setUpstreamProtocol(Http::Protocol::Http3);
}
}

decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, "test_read filter threw: ", nullptr,
absl::nullopt, "");
return Http::FilterHeadersStatus::StopIteration;
}

Expand Down
2 changes: 1 addition & 1 deletion test/java/io/envoyproxy/envoymobile/engine/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,4 @@ envoy_mobile_android_test(
"//library/java/io/envoyproxy/envoymobile/engine:envoy_engine_lib",
"//library/kotlin/io/envoyproxy/envoymobile:envoy_interfaces_lib",
],
)
)
72 changes: 17 additions & 55 deletions test/java/org/chromium/net/CronetUrlRequestTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.chromium.net.testing.CronetTestRule.CronetTestFramework;
import org.chromium.net.testing.CronetTestRule.OnlyRunNativeCronet;
import org.chromium.net.testing.CronetTestRule.RequiresMinApi;
import org.chromium.net.testing.FailurePhase;
import org.chromium.net.testing.Feature;
import org.chromium.net.testing.MockUrlRequestJobFactory;
import org.chromium.net.testing.NativeTestServer;
Expand All @@ -65,6 +64,9 @@

/**
* Test functionality of CronetUrlRequest.
* This test makes use of 2 cronetEngines.
* One with request filters to throw custom errors and the other without.
* TODO(colibie): switch to one engine, with the filter ignoring urls with no custom errors.
*/
@RunWith(RobolectricTestRunner.class)
public class CronetUrlRequestTest {
Expand Down Expand Up @@ -723,45 +725,6 @@ public void testMockNotFound() throws Exception {
assertEquals(callback.mResponseStep, ResponseStep.ON_SUCCEEDED);
}

@Test
@SmallTest
@Feature({"Cronet"})
@OnlyRunNativeCronet // Java impl doesn't support MockUrlRequestJobFactory
@Ignore("https://github.com/envoyproxy/envoy-mobile/issues/1549")
public void testMockStartAsyncError() throws Exception {
final int arbitraryNetError = -3;
TestUrlRequestCallback callback = startAndWaitForComplete(
mMockUrlRequestJobFactory.getCronetEngine(),
MockUrlRequestJobFactory.getMockUrlWithFailure(FailurePhase.START, arbitraryNetError));
assertNull(callback.mResponseInfo);
assertNotNull(callback.mError);
assertEquals(arbitraryNetError,
((NetworkException)callback.mError).getCronetInternalErrorCode());
assertEquals(0, callback.mRedirectCount);
assertTrue(callback.mOnErrorCalled);
assertEquals(ResponseStep.ON_FAILED, callback.mResponseStep);
}

@Test
@SmallTest
@Feature({"Cronet"})
@OnlyRunNativeCronet // Java impl doesn't support MockUrlRequestJobFactory
@Ignore("https://github.com/envoyproxy/envoy-mobile/issues/1549")
public void testMockReadDataSyncError() throws Exception {
final int arbitraryNetError = -4;
TestUrlRequestCallback callback = startAndWaitForComplete(
mMockUrlRequestJobFactory.getCronetEngine(),
MockUrlRequestJobFactory.getMockUrlWithFailure(FailurePhase.READ_SYNC, arbitraryNetError));
assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
assertEquals(15, callback.mResponseInfo.getReceivedByteCount());
assertNotNull(callback.mError);
assertEquals(arbitraryNetError,
((NetworkException)callback.mError).getCronetInternalErrorCode());
assertEquals(0, callback.mRedirectCount);
assertTrue(callback.mOnErrorCalled);
assertEquals(ResponseStep.ON_FAILED, callback.mResponseStep);
}

/**
* Tests that request continues when client certificate is requested.
*/
Expand Down Expand Up @@ -2139,12 +2102,11 @@ public void testCookiesArentSavedOrSent() throws Exception {
@SmallTest
@Feature({"Cronet"})
@OnlyRunNativeCronet
@Ignore("https://github.com/envoyproxy/envoy-mobile/issues/1549")
public void testQuicErrorCode() throws Exception {
long envoyMobileError = 0x2000;
TestUrlRequestCallback callback = startAndWaitForComplete(
mMockUrlRequestJobFactory.getCronetEngine(),
MockUrlRequestJobFactory.getMockUrlWithFailure(
FailurePhase.START, NetError.ERR_QUIC_PROTOCOL_ERROR.getErrorCode()));
MockUrlRequestJobFactory.getMockQuicUrlWithFailure(envoyMobileError));
assertNull(callback.mResponseInfo);
assertNotNull(callback.mError);
assertEquals(NetworkException.ERROR_QUIC_PROTOCOL_FAILED,
Expand All @@ -2159,12 +2121,12 @@ public void testQuicErrorCode() throws Exception {
@SmallTest
@Feature({"Cronet"})
@OnlyRunNativeCronet
@Ignore("https://github.com/envoyproxy/envoy-mobile/issues/1549")
@Ignore("https://github.com/envoyproxy/envoy-mobile/issues/1594: error removed in envoymobile")
public void testQuicErrorCodeForNetworkChanged() throws Exception {
TestUrlRequestCallback callback = startAndWaitForComplete(
mMockUrlRequestJobFactory.getCronetEngine(),
MockUrlRequestJobFactory.getMockUrlWithFailure(
FailurePhase.START, NetError.ERR_NETWORK_CHANGED.getErrorCode()));
TestUrlRequestCallback callback =
startAndWaitForComplete(mMockUrlRequestJobFactory.getCronetEngine(),
MockUrlRequestJobFactory.getMockUrlWithFailure(
NetError.ERR_NETWORK_CHANGED.getErrorCode()));
assertNull(callback.mResponseInfo);
assertNotNull(callback.mError);
assertEquals(NetworkException.ERROR_NETWORK_CHANGED,
Expand All @@ -2185,9 +2147,9 @@ public void testQuicErrorCodeForNetworkChanged() throws Exception {
@SmallTest
@Feature({"Cronet"})
@OnlyRunNativeCronet
@Ignore("https://github.com/envoyproxy/envoy-mobile/issues/1549")
public void testLegacyOnFailedCallback() throws Exception {
final int netError = -123;
final long envoyMobileError = 0x2000;
final int netError = NetError.ERR_OTHER.getErrorCode();
final AtomicBoolean failedExpectation = new AtomicBoolean();
final ConditionVariable done = new ConditionVariable();
UrlRequest.Callback callback = new UrlRequest.Callback() {
Expand Down Expand Up @@ -2231,8 +2193,8 @@ public void onCanceled(UrlRequest request, UrlResponseInfo info) {
}
};

UrlRequest.Builder builder = mTestFramework.mCronetEngine.newUrlRequestBuilder(
MockUrlRequestJobFactory.getMockUrlWithFailure(FailurePhase.START, netError), callback,
UrlRequest.Builder builder = mMockUrlRequestJobFactory.getCronetEngine().newUrlRequestBuilder(
MockUrlRequestJobFactory.getMockUrlWithFailure(envoyMobileError), callback,
Executors.newSingleThreadExecutor());
final UrlRequest urlRequest = builder.build();
urlRequest.start();
Expand All @@ -2244,9 +2206,9 @@ public void onCanceled(UrlRequest request, UrlResponseInfo info) {
private void checkSpecificErrorCode(@EnvoyMobileError long envoyMobileError, NetError netError,
int errorCode, String name, boolean immediatelyRetryable)
throws Exception {
TestUrlRequestCallback callback = startAndWaitForComplete(
mMockUrlRequestJobFactory.getCronetEngine(),
MockUrlRequestJobFactory.getMockUrlWithFailure(FailurePhase.START, envoyMobileError));
TestUrlRequestCallback callback =
startAndWaitForComplete(mMockUrlRequestJobFactory.getCronetEngine(),
MockUrlRequestJobFactory.getMockUrlWithFailure(envoyMobileError));
assertNull(callback.mResponseInfo);
assertNotNull(callback.mError);
assertEquals(netError.getErrorCode(),
Expand Down
1 change: 0 additions & 1 deletion test/java/org/chromium/net/testing/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ android_library(
"ContextUtils.java",
"CronetTestRule.java",
"CronetTestUtil.java",
"FailurePhase.java",
"Feature.java",
"FileUtils.java",
"Http2TestHandler.java",
Expand Down
12 changes: 0 additions & 12 deletions test/java/org/chromium/net/testing/FailurePhase.java

This file was deleted.

16 changes: 4 additions & 12 deletions test/java/org/chromium/net/testing/MockUrlRequestJobFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,12 @@ public void shutdown() {
* org.chromium.net.test.FailurePhase.
* @param @param envoyMobileError reported by the engine.
*/
public static String getMockUrlWithFailure(FailurePhase phase, long envoyMobileError) {
switch (phase) {
case START:
case READ_SYNC:
case READ_ASYNC:
break;
default:
throw new IllegalArgumentException("phase not in org.chromium.net.test.FailurePhase");
}
return TEST_URL + "/failed?" + phase + "=" + envoyMobileError;
public static String getMockUrlWithFailure(long envoyMobileError) {
return TEST_URL + "/failed?error=" + envoyMobileError;
}

public static String getMockUrlWithFailure(FailurePhase phase, int netError) {
throw new UnsupportedOperationException("To be implemented");
public static String getMockQuicUrlWithFailure(long envoyMobileError) {
return TEST_URL + "/failed?quic=1&error=" + envoyMobileError;
}

/**
Expand Down

0 comments on commit 7bf274c

Please sign in to comment.