Skip to content

Commit

Permalink
fixed isRetryHTTP code
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-stakeda committed Jun 3, 2020
1 parent 361bf73 commit 6594bc4
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 8 deletions.
6 changes: 3 additions & 3 deletions src/main/java/net/snowflake/client/jdbc/RestRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -434,13 +434,13 @@ else if (response.getStatusLine().getStatusCode() != 200)
return response;
}

private static boolean isRetryableHTTPCode(CloseableHttpResponse response, boolean retryHTTP403)
static boolean isRetryableHTTPCode(CloseableHttpResponse response, boolean retryHTTP403)
{
return response != null &&
(response.getStatusLine().getStatusCode() < 500 || // service unavailable
response.getStatusLine().getStatusCode() >= 600) && // gateway timeout
response.getStatusLine().getStatusCode() != 408 && // request timeout
(!retryHTTP403 || response.getStatusLine().getStatusCode() != 403);
response.getStatusLine().getStatusCode() != 408 && // request timeout
(retryHTTP403 || response.getStatusLine().getStatusCode() != 403);
}

private static boolean isCertificateRevoked(Exception ex)
Expand Down
159 changes: 158 additions & 1 deletion src/test/java/net/snowflake/client/jdbc/RestRequestIT.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2019 Snowflake Computing Inc. All rights reserved.
* Copyright (c) 2012-2020 Snowflake Computing Inc. All rights reserved.
*/
package net.snowflake.client.jdbc;

Expand All @@ -15,6 +15,8 @@
import org.mockito.stubbing.Answer;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;

import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -146,4 +148,159 @@ public CloseableHttpResponse answer(InvocationOnMock invocation)

execute(client, "fakeurl.com/?requestId=abcd-1234", false);
}

private CloseableHttpResponse anyStatusCodeResponse(int statusCode)
{
StatusLine successStatusLine = mock(StatusLine.class);
when(successStatusLine.getStatusCode()).thenReturn(statusCode);

CloseableHttpResponse response = mock(CloseableHttpResponse.class);
when(response.getStatusLine())
.thenReturn(successStatusLine);

return response;
}

@Test
public void testIsRetryableHTTPCode() throws Exception
{
class TestCase
{
TestCase(int statusCode, boolean retryHTTP403, boolean result)
{
this.statusCode = statusCode;
this.retryHTTP403 = retryHTTP403;
this.result = result;
}

public int statusCode;
public boolean retryHTTP403;
public boolean result;
}
List<TestCase> testCases = new ArrayList<>();
// no retry on HTTP 403 option
testCases.add(new TestCase(100, false, true));
testCases.add(new TestCase(101, false, true));
testCases.add(new TestCase(103, false, true));
testCases.add(new TestCase(200, false, true));
testCases.add(new TestCase(201, false, true));
testCases.add(new TestCase(202, false, true));
testCases.add(new TestCase(203, false, true));
testCases.add(new TestCase(204, false, true));
testCases.add(new TestCase(205, false, true));
testCases.add(new TestCase(206, false, true));
testCases.add(new TestCase(300, false, true));
testCases.add(new TestCase(301, false, true));
testCases.add(new TestCase(302, false, true));
testCases.add(new TestCase(303, false, true));
testCases.add(new TestCase(304, false, true));
testCases.add(new TestCase(307, false, true));
testCases.add(new TestCase(308, false, true));
testCases.add(new TestCase(400, false, true));
testCases.add(new TestCase(401, false, true));
testCases.add(new TestCase(403, false, false)); // no retry on HTTP 403
testCases.add(new TestCase(404, false, true));
testCases.add(new TestCase(405, false, true));
testCases.add(new TestCase(406, false, true));
testCases.add(new TestCase(407, false, true));
testCases.add(new TestCase(408, false, false)); // no retry on HTTP 408
testCases.add(new TestCase(410, false, true));
testCases.add(new TestCase(411, false, true));
testCases.add(new TestCase(412, false, true));
testCases.add(new TestCase(413, false, true));
testCases.add(new TestCase(414, false, true));
testCases.add(new TestCase(415, false, true));
testCases.add(new TestCase(416, false, true));
testCases.add(new TestCase(417, false, true));
testCases.add(new TestCase(418, false, true));
testCases.add(new TestCase(425, false, true));
testCases.add(new TestCase(426, false, true));
testCases.add(new TestCase(428, false, true));
testCases.add(new TestCase(429, false, true));
testCases.add(new TestCase(431, false, true));
testCases.add(new TestCase(451, false, true));
testCases.add(new TestCase(500, false, false));
testCases.add(new TestCase(501, false, false));
testCases.add(new TestCase(502, false, false));
testCases.add(new TestCase(503, false, false));
testCases.add(new TestCase(504, false, false));
testCases.add(new TestCase(505, false, false));
testCases.add(new TestCase(506, false, false));
testCases.add(new TestCase(507, false, false));
testCases.add(new TestCase(508, false, false));
testCases.add(new TestCase(509, false, false));
testCases.add(new TestCase(510, false, false));
testCases.add(new TestCase(511, false, false));
// do retry on HTTP 403 option
testCases.add(new TestCase(100, true, true));
testCases.add(new TestCase(101, true, true));
testCases.add(new TestCase(103, true, true));
testCases.add(new TestCase(200, true, true));
testCases.add(new TestCase(201, true, true));
testCases.add(new TestCase(202, true, true));
testCases.add(new TestCase(203, true, true));
testCases.add(new TestCase(204, true, true));
testCases.add(new TestCase(205, true, true));
testCases.add(new TestCase(206, true, true));
testCases.add(new TestCase(300, true, true));
testCases.add(new TestCase(301, true, true));
testCases.add(new TestCase(302, true, true));
testCases.add(new TestCase(303, true, true));
testCases.add(new TestCase(304, true, true));
testCases.add(new TestCase(307, true, true));
testCases.add(new TestCase(308, true, true));
testCases.add(new TestCase(400, true, true));
testCases.add(new TestCase(401, true, true));
testCases.add(new TestCase(403, true, true)); // do retry on HTTP 403
testCases.add(new TestCase(404, true, true));
testCases.add(new TestCase(405, true, true));
testCases.add(new TestCase(406, true, true));
testCases.add(new TestCase(407, true, true));
testCases.add(new TestCase(408, true, false)); // no retry on HTTP 408
testCases.add(new TestCase(410, true, true));
testCases.add(new TestCase(411, true, true));
testCases.add(new TestCase(412, true, true));
testCases.add(new TestCase(413, true, true));
testCases.add(new TestCase(414, true, true));
testCases.add(new TestCase(415, true, true));
testCases.add(new TestCase(416, true, true));
testCases.add(new TestCase(417, true, true));
testCases.add(new TestCase(418, true, true));
testCases.add(new TestCase(425, true, true));
testCases.add(new TestCase(426, true, true));
testCases.add(new TestCase(428, true, true));
testCases.add(new TestCase(429, true, true));
testCases.add(new TestCase(431, true, true));
testCases.add(new TestCase(451, true, true));
testCases.add(new TestCase(500, true, false));
testCases.add(new TestCase(501, true, false));
testCases.add(new TestCase(502, true, false));
testCases.add(new TestCase(503, true, false));
testCases.add(new TestCase(504, true, false));
testCases.add(new TestCase(505, true, false));
testCases.add(new TestCase(506, true, false));
testCases.add(new TestCase(507, true, false));
testCases.add(new TestCase(508, true, false));
testCases.add(new TestCase(509, true, false));
testCases.add(new TestCase(510, true, false));
testCases.add(new TestCase(511, true, false));

for (TestCase t : testCases)
{
if (t.result)
{
assertTrue(
String.format("Result must be true but false: HTTP Code: %d, RetryHTTP403: %s",
t.statusCode, t.retryHTTP403),
RestRequest.isRetryableHTTPCode(anyStatusCodeResponse(t.statusCode), t.retryHTTP403));
}
else
{
assertFalse(
String.format("Result must be false but true: HTTP Code: %d, RetryHTTP403: %s",
t.statusCode, t.retryHTTP403),
RestRequest.isRetryableHTTPCode(anyStatusCodeResponse(t.statusCode), t.retryHTTP403));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ public static Object[][] data()
@Rule
public TemporaryFolder tmpFolder = new TemporaryFolder();

private static int ERR_RESULT_SET_CLOSED = 200037;

private static boolean developPrint = false;

private static String queryResultFormat;
Expand Down Expand Up @@ -708,8 +706,7 @@ public void testNegativeWithChunkFileNotExist() throws Throwable
}
catch (SQLException ex)
{
assertEquals(ex.getErrorCode(),ERR_RESULT_SET_CLOSED);
// System.out.println("Negative test hits expected error: " + ex.getMessage());
assertEquals((long)ErrorCode.INTERNAL_ERROR.getMessageCode(), ex.getErrorCode());
}

rs.close();
Expand Down

0 comments on commit 6594bc4

Please sign in to comment.