Skip to content

Commit

Permalink
Removed generic IO exception condition in network failure. Replaced w…
Browse files Browse the repository at this point in the history
…ith specific exceptions. Included in unit testing (#16469)
  • Loading branch information
kushagraThapar committed Oct 17, 2020
1 parent ce5ec0a commit 7cce8ba
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,19 @@
import io.netty.channel.ChannelException;
import io.netty.handler.timeout.ReadTimeoutException;

import javax.net.ssl.SSLException;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLPeerUnverifiedException;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.net.ConnectException;
import java.net.HttpRetryException;
import java.net.NoRouteToHostException;
import java.net.SocketException;
import java.net.UnknownHostException;
import java.net.UnknownServiceException;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.InterruptedByTimeoutException;

public class WebExceptionUtility {
public static boolean isWebExceptionRetriable(Exception ex) {
Expand Down Expand Up @@ -65,7 +72,19 @@ public static boolean isNetworkFailure(Exception ex) {
}

private static boolean isNetworkFailureInternal(Exception ex) {
if (ex instanceof IOException) {
// We have seen these cases in CRIs
if (ex instanceof ClosedChannelException
|| ex instanceof SocketException
|| ex instanceof SSLException
|| ex instanceof UnknownHostException) {
return true;
}

// These cases might be related, but we haven't seen them ever
if (ex instanceof UnknownServiceException
|| ex instanceof HttpRetryException
|| ex instanceof InterruptedByTimeoutException
|| ex instanceof InterruptedIOException) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,27 @@

package com.azure.cosmos.implementation.directconnectivity;

import com.fasterxml.jackson.databind.JsonMappingException;
import io.netty.channel.ChannelException;
import io.netty.channel.ConnectTimeoutException;
import io.netty.handler.timeout.ReadTimeoutException;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import javax.net.ssl.SSLException;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLPeerUnverifiedException;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.net.ConnectException;
import java.net.HttpRetryException;
import java.net.NoRouteToHostException;
import java.net.SocketException;
import java.net.SocketTimeoutException;
import java.net.UnknownHostException;
import java.net.UnknownServiceException;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.InterruptedByTimeoutException;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;

Expand Down Expand Up @@ -100,6 +109,33 @@ public Object[][] networkFailure() {
},
{
new ChannelException(), true
},
{
new IOException(), false
},
{
new ClosedChannelException(), true
},
{
new SocketException(), true
},
{
new SSLException("dummy"), true
},
{
new UnknownServiceException(), true
},
{
new HttpRetryException("dummy", 500), true
},
{
new InterruptedByTimeoutException(), true
},
{
new InterruptedIOException(), true
},
{
new JsonMappingException(null, "dummy"), false
}
};
}
Expand Down

0 comments on commit 7cce8ba

Please sign in to comment.