diff --git a/src/main/java/io/simplelocalize/cli/client/SimpleLocalizeClient.java b/src/main/java/io/simplelocalize/cli/client/SimpleLocalizeClient.java index fabab1e..8187cda 100644 --- a/src/main/java/io/simplelocalize/cli/client/SimpleLocalizeClient.java +++ b/src/main/java/io/simplelocalize/cli/client/SimpleLocalizeClient.java @@ -35,6 +35,8 @@ public class SimpleLocalizeClient { private static final String ERROR_MESSAGE_PATH = "$.msg"; + private static final int MAX_DOWNLOAD_ATTEMPTS = 3; + private static final long RETRY_DELAY_MILLIS = 1000; private final HttpClient httpClient; private final SimpleLocalizeHttpRequestFactory httpRequestFactory; private final SimpleLocalizeUriFactory uriFactory; @@ -129,8 +131,58 @@ public void downloadFile(String url, Path savePath) throws IOException, Interrup Files.createDirectories(parentDirectory); } log.info("Downloading {}", savePath); - HttpResponse response = httpClient.send(httpRequest, HttpResponse.BodyHandlers.ofFile(savePath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)); - throwOnError(response); + for (int attempt = 1; ; attempt++) + { + HttpResponse response; + try + { + response = httpClient.send(httpRequest, HttpResponse.BodyHandlers.ofFile(savePath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)); + } catch (IOException e) + { + if (attempt >= MAX_DOWNLOAD_ATTEMPTS) + { + throw e; + } + log.warn("Downloading {} failed: {}, retrying ({}/{})", savePath, e.getMessage(), attempt + 1, MAX_DOWNLOAD_ATTEMPTS); + Thread.sleep(RETRY_DELAY_MILLIS * attempt); + continue; + } + if (response.statusCode() == 200) + { + return; + } + // the body handler wrote the error response to savePath; don't leave it there as a translation file + try + { + if (isRetryableStatusCode(response.statusCode()) && attempt < MAX_DOWNLOAD_ATTEMPTS) + { + log.warn("Downloading {} failed with HTTP {}, retrying ({}/{})", savePath, response.statusCode(), attempt + 1, MAX_DOWNLOAD_ATTEMPTS); + } else + { + throwOnError(response); + } + } finally + { + deleteQuietly(savePath); + } + Thread.sleep(RETRY_DELAY_MILLIS * attempt); + } + } + + private boolean isRetryableStatusCode(int statusCode) + { + return statusCode == 429 || statusCode >= 500; + } + + private void deleteQuietly(Path path) + { + try + { + Files.deleteIfExists(path); + } catch (IOException e) + { + log.warn("Unable to remove incomplete file: {}", path); + } } public String fetchProject() throws IOException, InterruptedException @@ -192,14 +244,8 @@ private void throwOnError(HttpResponse httpResponse) { if (httpResponse.statusCode() != 200) { - com.jayway.jsonpath.Configuration parseContext = com.jayway.jsonpath.Configuration - .defaultConfiguration() - .addOptions(Option.SUPPRESS_EXCEPTIONS); - - Object responseBody = httpResponse.body(); - String stringBody = safeCastHttpBodyToString(responseBody); - String message = JsonPath.using(parseContext).parse(stringBody).read(ERROR_MESSAGE_PATH); - if (message == null) + String message = tryReadErrorMessage(httpResponse); + if (StringUtils.isBlank(message)) { message = "Unknown error, HTTP Status: " + httpResponse.statusCode(); } @@ -208,6 +254,27 @@ private void throwOnError(HttpResponse httpResponse) } } + private String tryReadErrorMessage(HttpResponse httpResponse) + { + String stringBody = safeCastHttpBodyToString(httpResponse.body()); + if (StringUtils.isBlank(stringBody)) + { + return null; + } + try + { + com.jayway.jsonpath.Configuration parseContext = com.jayway.jsonpath.Configuration + .defaultConfiguration() + .addOptions(Option.SUPPRESS_EXCEPTIONS); + Object message = JsonPath.using(parseContext).parse(stringBody).read(ERROR_MESSAGE_PATH); + return message != null ? String.valueOf(message) : null; + } catch (Exception e) + { + log.debug("Unable to extract error message from response body", e); + return null; + } + } + private String safeCastHttpBodyToString(Object responseBody) { if (responseBody instanceof byte[] responseBodyBytes) @@ -216,6 +283,15 @@ private String safeCastHttpBodyToString(Object responseBody) } else if (responseBody instanceof String responseBodyString) { return responseBodyString; + } else if (responseBody instanceof Path responseBodyPath) + { + try + { + return Files.readString(responseBodyPath); + } catch (IOException e) + { + return ""; + } } return ""; } diff --git a/src/test/java/io/simplelocalize/cli/client/SimpleLocalizeClientTest.java b/src/test/java/io/simplelocalize/cli/client/SimpleLocalizeClientTest.java index 4efa13a..e261c08 100644 --- a/src/test/java/io/simplelocalize/cli/client/SimpleLocalizeClientTest.java +++ b/src/test/java/io/simplelocalize/cli/client/SimpleLocalizeClientTest.java @@ -272,4 +272,120 @@ void shouldDownloadAndTruncateBeforeWriting() throws Exception //then assertThat(Path.of(downloadPath)).hasContent("sample").isRegularFile(); } + + @Test + void shouldThrowApiRequestExceptionWhenDownloadFailsWithEmptyBody() + { + //given + SimpleLocalizeClient client = new SimpleLocalizeClient(MOCK_SERVER_BASE_URL, "96a7b6ca75c79d4af4dfd5db2946fdd4"); + mockServer.when(request() + .withMethod("GET") + .withPath("/s3/missing-file"), + Times.exactly(1)) + .respond( + response() + .withStatusCode(403) + .withDelay(TimeUnit.MILLISECONDS, 200) + ); + + DownloadableFile downloadableFile = new DownloadableFile(MOCK_SERVER_BASE_URL + "/s3/missing-file", "common", null, null, null, null); + String downloadPath = "./junit/download-error/file.json"; + + //when & then + Assertions + .assertThatThrownBy(() -> client.downloadFile(downloadableFile, downloadPath)) + .isInstanceOf(ApiRequestException.class) + .hasMessage("Unknown error, HTTP Status: 403"); + + assertThat(Path.of(downloadPath)).doesNotExist(); + } + + @Test + void shouldThrowApiRequestExceptionWithMessageWhenDownloadFailsWithJsonBody() + { + //given + SimpleLocalizeClient client = new SimpleLocalizeClient(MOCK_SERVER_BASE_URL, "96a7b6ca75c79d4af4dfd5db2946fdd4"); + mockServer.when(request() + .withMethod("GET") + .withPath("/s3/expired-file"), + Times.exactly(1)) + .respond( + response() + .withStatusCode(410) + .withContentType(MediaType.APPLICATION_JSON_UTF_8) + .withBody("{ \"msg\": \"link expired\" }") + .withDelay(TimeUnit.MILLISECONDS, 200) + ); + + DownloadableFile downloadableFile = new DownloadableFile(MOCK_SERVER_BASE_URL + "/s3/expired-file", "common", null, null, null, null); + String downloadPath = "./junit/download-error/expired.json"; + + //when & then + Assertions + .assertThatThrownBy(() -> client.downloadFile(downloadableFile, downloadPath)) + .isInstanceOf(ApiRequestException.class) + .hasMessage("link expired"); + + assertThat(Path.of(downloadPath)).doesNotExist(); + } + + @Test + void shouldRetryDownloadOnServerError() throws Exception + { + //given + SimpleLocalizeClient client = new SimpleLocalizeClient(MOCK_SERVER_BASE_URL, "96a7b6ca75c79d4af4dfd5db2946fdd4"); + mockServer.when(request() + .withMethod("GET") + .withPath("/s3/flaky-file"), + Times.exactly(2)) + .respond( + response() + .withStatusCode(500) + ); + mockServer.when(request() + .withMethod("GET") + .withPath("/s3/flaky-file"), + Times.exactly(1)) + .respond( + response() + .withStatusCode(200) + .withContentType(MediaType.APPLICATION_JSON_UTF_8) + .withBody("{ \"key\": \"value\" }") + ); + + DownloadableFile downloadableFile = new DownloadableFile(MOCK_SERVER_BASE_URL + "/s3/flaky-file", "common", null, null, null, null); + String downloadPath = "./junit/download-retry/file.json"; + + //when + client.downloadFile(downloadableFile, downloadPath); + + //then + assertThat(Path.of(downloadPath)).hasContent("{ \"key\": \"value\" }").isRegularFile(); + } + + @Test + void shouldGiveUpDownloadAfterMaxRetryAttempts() + { + //given + SimpleLocalizeClient client = new SimpleLocalizeClient(MOCK_SERVER_BASE_URL, "96a7b6ca75c79d4af4dfd5db2946fdd4"); + mockServer.when(request() + .withMethod("GET") + .withPath("/s3/broken-file"), + Times.exactly(3)) + .respond( + response() + .withStatusCode(503) + ); + + DownloadableFile downloadableFile = new DownloadableFile(MOCK_SERVER_BASE_URL + "/s3/broken-file", "common", null, null, null, null); + String downloadPath = "./junit/download-retry/broken.json"; + + //when & then + Assertions + .assertThatThrownBy(() -> client.downloadFile(downloadableFile, downloadPath)) + .isInstanceOf(ApiRequestException.class) + .hasMessage("Unknown error, HTTP Status: 503"); + + assertThat(Path.of(downloadPath)).doesNotExist(); + } }