diff --git a/.github/workflows/pull-request-test.yml b/.github/workflows/pull-request-test.yml index 1afdc6a..3990923 100644 --- a/.github/workflows/pull-request-test.yml +++ b/.github/workflows/pull-request-test.yml @@ -13,5 +13,5 @@ jobs: distribution: 'zulu' - uses: actions/checkout@v2 - name: Test with Gradle - run: gradle test:test --stacktrace + run: ./gradlew test:test --stacktrace diff --git a/.github/workflows/semantic-pr.yml b/.github/workflows/semantic-pr.yml index fe910b1..54475a4 100644 --- a/.github/workflows/semantic-pr.yml +++ b/.github/workflows/semantic-pr.yml @@ -7,7 +7,7 @@ on: jobs: pr-title-check: name: Check PR for semantic title - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest steps: - name: PR title is valid if: > diff --git a/.gitignore b/.gitignore index ddd2895..a6a810d 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,9 @@ *.tar.gz *.rar +# Keep gradle wrapper +!gradle/wrapper/gradle-wrapper.jar + # virtual machine crash logs, see http://www.java.com/en/download/help/error_hotspot.xml hs_err_pid* diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar new file mode 100644 index 0000000..e708b1c Binary files /dev/null and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 442d913..8cf6eb5 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-6.8.3-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-6.8.3-all.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/src/main/java/com/amplitude/HttpCall.java b/src/main/java/com/amplitude/HttpCall.java index d049f48..c52c6d2 100644 --- a/src/main/java/com/amplitude/HttpCall.java +++ b/src/main/java/com/amplitude/HttpCall.java @@ -2,6 +2,7 @@ import com.amplitude.exception.AmplitudeInvalidAPIKeyException; import org.json.JSONArray; +import org.json.JSONException; import org.json.JSONObject; import javax.net.ssl.HttpsURLConnection; @@ -89,6 +90,12 @@ protected Response makeRequest(List events) throws AmplitudeInvalidAPIKey timesOutResponse.put("status", Status.TIMEOUT); timesOutResponse.put("code", 408); responseBody = Response.populateResponse(timesOutResponse); + } catch (JSONException e) { + // Some error responses from load balancers and reverse proxies may have + // response bodies that are not JSON (e.g. HTML, XML). + JSONObject decodeFailureResponse = new JSONObject(); + decodeFailureResponse.put("code", responseCode); + responseBody = Response.populateResponse(decodeFailureResponse); } finally { if (inputStream != null) { try { diff --git a/src/main/java/com/amplitude/HttpTransport.java b/src/main/java/com/amplitude/HttpTransport.java index d49f649..754ec95 100644 --- a/src/main/java/com/amplitude/HttpTransport.java +++ b/src/main/java/com/amplitude/HttpTransport.java @@ -199,8 +199,7 @@ private EventsRetryResult retryEventsOnce(String userId, String deviceId, List events, int status, String message) { diff --git a/src/test/java/com/amplitude/HttpCallTest.java b/src/test/java/com/amplitude/HttpCallTest.java index b243b56..c78e49d 100644 --- a/src/test/java/com/amplitude/HttpCallTest.java +++ b/src/test/java/com/amplitude/HttpCallTest.java @@ -268,6 +268,22 @@ public void testHttpCallWithCustomProxy(HttpCallMode httpCallMode, URL url) assertTrue(connection.usingProxy()); } + @ParameterizedTest + @MethodSource("httpCallArguments") + public void testHttpCallWithNonJsonResponseBody(HttpCallMode httpCallMode, URL url) + throws IOException, AmplitudeInvalidAPIKeyException { + HttpsURLConnection connection = + MockHttpsURLConnectionHelper.getMockHttpsURLConnection(502, ""); + mockURLStreamHandler.setConnection(url, connection); + HttpCall httpCall = getHttpCallFromCallMode(httpCallMode, null, new Proxy(Proxy.Type.HTTP, new InetSocketAddress("0.0.0.0", 443))); + List events = EventsGenerator.generateEvents(1); + Response response = httpCall.makeRequest(events); + + assertEquals(502, response.code); + assertEquals(Status.FAILED, response.status); + verifyConnectionOption(connection); + } + static Stream httpCallArguments() { return Stream.of( arguments(HttpCallMode.REGULAR, Constants.API_URL), diff --git a/src/test/java/com/amplitude/HttpTransportTest.java b/src/test/java/com/amplitude/HttpTransportTest.java index cd1b188..e4c6743 100644 --- a/src/test/java/com/amplitude/HttpTransportTest.java +++ b/src/test/java/com/amplitude/HttpTransportTest.java @@ -42,7 +42,7 @@ public void setUp() { "RATELIMIT, true", "PAYLOAD_TOO_LARGE, true", "TIMEOUT, true", - "FAILED, false", + "FAILED, true", "UNKNOWN, false" }) public void testShouldRetryForStatus(Status status, boolean expected) { @@ -55,9 +55,10 @@ public void testRetryEvents() throws AmplitudeInvalidAPIKeyException, Interrupte Response payloadTooLargeResponse = ResponseUtil.getPayloadTooLargeResponse(); Response invalidResponse = ResponseUtil.getInvalidResponse(false); Response rateLimitResponse = ResponseUtil.getRateLimitResponse(false); + Response failedResponse = ResponseUtil.getFailedResponse(); HttpCall httpCall = mock(HttpCall.class); - CountDownLatch latch = new CountDownLatch(4); + CountDownLatch latch = new CountDownLatch(5); CountDownLatch latch2 = new CountDownLatch(10); when(httpCall.makeRequest(anyList())) .thenAnswer( @@ -75,6 +76,11 @@ public void testRetryEvents() throws AmplitudeInvalidAPIKeyException, Interrupte latch.countDown(); return payloadTooLargeResponse; }) + .thenAnswer( + invocation -> { + latch.countDown(); + return failedResponse; + }) .thenAnswer( invocation -> { latch.countDown(); @@ -94,9 +100,9 @@ public void onLogEventServerResponse(Event event, int status, String message) { httpTransport.setHttpCall(httpCall); httpTransport.setCallbacks(callbacks); httpTransport.retryEvents(events, invalidResponse); - assertTrue(latch.await(1L, TimeUnit.SECONDS)); + assertTrue(latch.await(2L, TimeUnit.SECONDS)); assertTrue(latch2.await(1L, TimeUnit.SECONDS)); - verify(httpCall, times(4)).makeRequest(anyList()); + verify(httpCall, times(5)).makeRequest(anyList()); for (int i = 0; i < events.size(); i++) { if (i < (events.size() / 4)) { assertEquals(200, resultMap.get(events.get(i))); @@ -299,14 +305,20 @@ public void onLogEventServerResponse(Event event, int status, String message) { @Test public void testFailedResponse() throws AmplitudeInvalidAPIKeyException, InterruptedException { Response failedResponse = ResponseUtil.getFailedResponse(); + Response successResponse = ResponseUtil.getSuccessResponse(); HttpCall httpCall = mock(HttpCall.class); - CountDownLatch latch = new CountDownLatch(1); + CountDownLatch latch = new CountDownLatch(2); CountDownLatch latch2 = new CountDownLatch(10); when(httpCall.makeRequest(anyList())) .thenAnswer( invocation -> { latch.countDown(); return failedResponse; + }) + .thenAnswer( + invocation -> { + latch.countDown(); + return successResponse; }); List events = EventsGenerator.generateEvents(10); @@ -324,9 +336,9 @@ public void onLogEventServerResponse(Event event, int status, String message) { httpTransport.sendEventsWithRetry(events); assertTrue(latch.await(1L, TimeUnit.SECONDS)); assertTrue(latch2.await(1L, TimeUnit.SECONDS)); - verify(httpCall, times(1)).makeRequest(anyList()); + verify(httpCall, times(2)).makeRequest(anyList()); for (Event event : events) { - assertEquals(500, resultMap.get(event)); + assertEquals(200, resultMap.get(event)); } }