Skip to content

Commit f550fcf

Browse files
hive-botclaude
andcommitted
fix: retry connection resets, drop hidden PATCH retries, and wrap download read failures
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 20d3167 commit f550fcf

8 files changed

Lines changed: 149 additions & 17 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
227227
They will be removed in the next major version.
228228

229229
### Fixed
230+
- A connection failure while reading a downloaded document
231+
(`translateDocumentDownload()`) or translation memory export
232+
(`downloadTranslationMemoryExport()`) now throws `ConnectionException`
233+
instead of a raw `IOException`; `IOException` is left for failures writing
234+
the output.
230235
- A `User-Agent` header passed with `setHeaders()` now replaces the built
231236
`User-Agent` in any casing, e.g. `user-agent`.
237+
- `PATCH` requests no longer make up to 3 hidden extra attempts per try on top
238+
of `maxRetries`.
239+
- Connection resets and broken pipes are now retried, and
240+
`ConnectionException.getShouldRetry()` returns `true` for them.
232241
- The language query parameters of `getMultilingualGlossaryDictionaryEntries()`
233242
and `deleteMultilingualGlossaryDictionary()` now encode a space as `%20`
234243
instead of `+`.

‎README.md‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1808,13 +1808,15 @@ invalid arguments are provided, they may raise the standard exceptions
18081808
- `TooManyRequestsException`: HTTP 429 after all retries.
18091809
- `QuotaExceededException`: the quota is exceeded (HTTP 456).
18101810
- `ConnectionException`: no HTTP response was received, for example after a
1811-
connection, DNS or timeout failure.
1811+
connection, DNS or timeout failure or a connection reset, or the connection
1812+
failed while a downloaded document or translation memory export was being
1813+
read.
18121814
- A malformed API response raises a `DeepLException`.
18131815

18141816
`DeepLException.getHttpStatusCode()` returns the HTTP status code of the failed
18151817
response, or `null` if there was none. `DeepLException.getShouldRetry()`
18161818
returns `true` for transient errors (HTTP 429, HTTP 5xx, connection, DNS and
1817-
timeout failures); the client has already retried these up to the configured
1819+
timeout failures, connection resets and broken pipes); the client has already retried these up to the configured
18181820
maximum number of retries.
18191821

18201822
### Writing a Plugin
@@ -1857,8 +1859,10 @@ The available options setters are:
18571859

18581860
- `setMaxRetries()`: maximum number of failed HTTP requests to retry, the
18591861
default is 5. Note: only failures due to transient conditions are retried:
1860-
HTTP 429, HTTP 5xx, and connection, DNS and timeout failures, including a
1861-
timeout while reading the response.
1862+
HTTP 429, HTTP 5xx, and connection, DNS and timeout failures, connection
1863+
resets and broken pipes, including a timeout while reading the response.
1864+
Every request, including `PATCH` requests, is sent at most `maxRetries + 1`
1865+
times.
18621866
- `setTimeout()`: connection timeout for each HTTP request.
18631867
- `setProxy()`: provide details about a proxy to use for all HTTP requests to
18641868
DeepL, as a `java.net.Proxy`.

‎deepl-java/src/main/java/com/deepl/api/DeepLClient.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2553,7 +2553,7 @@ public void downloadTranslationMemoryExport(TranslationMemoryJob job, File outpu
25532553
*
25542554
* @param job Completed export job carrying the download URL.
25552555
* @param outputStream Stream to download the exported translation memory to.
2556-
* @throws IOException If an I/O error occurs.
2556+
* @throws IOException If an I/O error occurs while writing to the output stream.
25572557
* @throws InterruptedException If the thread is interrupted during execution of this function.
25582558
* @throws DeepLException If the job has no download URL, or any error occurs while downloading.
25592559
* @see DeepLClient#downloadTranslationMemoryExport(TranslationMemoryJob, File)
@@ -2579,7 +2579,7 @@ public void downloadTranslationMemoryExport(TranslationMemoryJob job, OutputStre
25792579
.setHttpStatusCode(response.getCode());
25802580
}
25812581
assert response.getBody() != null;
2582-
StreamUtil.transferTo(response.getBody(), outputStream);
2582+
HttpClientWrapper.copyBody(response.getBody(), outputStream);
25832583
}
25842584
}
25852585

‎deepl-java/src/main/java/com/deepl/api/HttpClientWrapper.java‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,9 @@ private HttpResponse sendPatchRequestWithBackoff(String relativeUrl, HttpContent
147147
private HttpResponse sendPatchRequest(
148148
String relativeUrl, HttpContent content, BackoffTimer backoffTimer) throws DeepLException {
149149
// Like HttpURLConnection, honor the JVM's proxy settings unless a proxy was configured.
150-
HttpClientBuilder builder = HttpClients.custom().useSystemProperties();
150+
// Automatic retries are disabled so that only the backoff loop retries, up to maxRetries.
151+
HttpClientBuilder builder =
152+
HttpClients.custom().useSystemProperties().disableAutomaticRetries();
151153
if (proxy != null) {
152154
InetSocketAddress addr = (InetSocketAddress) proxy.address();
153155
builder.setProxy(new HttpHost(addr.getHostName(), addr.getPort()));
@@ -328,7 +330,7 @@ private HttpResponseStream sendRequest(
328330
} catch (IOException e) {
329331
ConnectionException failure = toConnectionException(e);
330332
throw new ConnectionException(
331-
failure.getMessage(), failure.getShouldRetry(), e, failure.getShouldRetry());
333+
failure.getMessage(), failure.getShouldRetry(), e, isUnsentFailure(e));
332334
}
333335

334336
if (content != null) {
@@ -356,14 +358,50 @@ private HttpResponseStream sendRequest(
356358
}
357359
}
358360

359-
// Connect, DNS and timeout failures are transient: the request is retried.
361+
/**
362+
* Copies a streamed response body to the output stream. A failure while reading the body is a
363+
* {@link ConnectionException}; a failure while writing the output stays an {@link IOException}.
364+
*/
365+
static void copyBody(InputStream body, OutputStream outputStream)
366+
throws ConnectionException, IOException {
367+
final byte[] buffer = new byte[8192];
368+
while (true) {
369+
int read;
370+
try {
371+
read = body.read(buffer);
372+
} catch (RuntimeException | IOException e) {
373+
throw toConnectionException(e);
374+
}
375+
if (read < 0) {
376+
return;
377+
}
378+
outputStream.write(buffer, 0, read);
379+
}
380+
}
381+
382+
// Connect, DNS, timeout, connection reset and broken pipe failures are transient: the request is
383+
// retried.
360384
private static ConnectionException toConnectionException(Exception e) {
361-
boolean shouldRetry =
362-
e instanceof SocketTimeoutException
363-
|| e instanceof ConnectException
364-
|| e instanceof NoRouteToHostException
365-
|| e instanceof UnknownHostException
366-
|| e instanceof ConnectTimeoutException;
385+
boolean shouldRetry = isUnsentFailure(e) || isConnectionResetOrBrokenPipe(e);
367386
return new ConnectionException(e.getMessage(), shouldRetry, e);
368387
}
388+
389+
// Failures of the connect step after which the server cannot have received the request.
390+
private static boolean isUnsentFailure(Exception e) {
391+
return e instanceof SocketTimeoutException
392+
|| e instanceof ConnectException
393+
|| e instanceof NoRouteToHostException
394+
|| e instanceof UnknownHostException
395+
|| e instanceof ConnectTimeoutException;
396+
}
397+
398+
// Java has no exception types for these, only SocketException messages such as
399+
// "Connection reset", "Connection reset by peer" and "Broken pipe (Write failed)".
400+
private static boolean isConnectionResetOrBrokenPipe(Exception e) {
401+
if (!(e instanceof SocketException) || e.getMessage() == null) {
402+
return false;
403+
}
404+
String message = e.getMessage().toLowerCase(Locale.ROOT);
405+
return message.contains("connection reset") || message.contains("broken pipe");
406+
}
369407
}

‎deepl-java/src/main/java/com/deepl/api/Translator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ public void translateDocumentDownload(DocumentHandle handle, File outputFile)
652652
*
653653
* @param handle Handle associated with document translation to download.
654654
* @param outputStream Stream to download translated document to.
655-
* @throws IOException If an I/O error occurs.
655+
* @throws IOException If an I/O error occurs while writing to the output stream.
656656
* @throws InterruptedException If the thread is interrupted during execution of this function.
657657
* @throws DeepLException If any error occurs while communicating with the DeepL API.
658658
*/
@@ -666,7 +666,7 @@ public void translateDocumentDownload(DocumentHandle handle, OutputStream output
666666
try (HttpResponseStream response = httpClientWrapper.downloadWithBackoff(relativeUrl, params)) {
667667
checkResponse(response);
668668
assert response.getBody() != null;
669-
StreamUtil.transferTo(response.getBody(), outputStream);
669+
HttpClientWrapper.copyBody(response.getBody(), outputStream);
670670
}
671671
}
672672

‎deepl-java/src/offlineTest/java/com/deepl/api/HttpClientTest.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@
1818
import com.github.tomakehurst.wiremock.http.Fault;
1919
import com.github.tomakehurst.wiremock.stubbing.Scenario;
2020
import java.net.Proxy;
21+
import java.net.ServerSocket;
22+
import java.net.Socket;
23+
import java.net.SocketException;
2124
import java.net.UnknownHostException;
2225
import java.time.Duration;
26+
import java.util.concurrent.atomic.AtomicInteger;
2327
import org.junit.jupiter.api.Test;
2428
import org.junit.jupiter.api.extension.RegisterExtension;
2529

@@ -118,6 +122,61 @@ void throwsConnectionExceptionWhenTheConnectionIsReset() {
118122
assertThrows(ConnectionException.class, () -> client(0).getUsage());
119123
}
120124

125+
// A PATCH request, because HttpURLConnection silently resends a GET once after a reset.
126+
@Test
127+
void retriesAConnectionReset() throws Exception {
128+
wm.stubFor(
129+
patch(urlEqualTo("/v3/glossaries/g1"))
130+
.inScenario("reset")
131+
.whenScenarioStateIs(Scenario.STARTED)
132+
.willSetStateTo("recovered")
133+
.willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER)));
134+
wm.stubFor(
135+
patch(urlEqualTo("/v3/glossaries/g1"))
136+
.inScenario("reset")
137+
.whenScenarioStateIs("recovered")
138+
.willReturn(okJson(GLOSSARY)));
139+
client(1).updateMultilingualGlossaryName("g1", "n");
140+
wm.verify(2, patchRequestedFor(urlEqualTo("/v3/glossaries/g1")));
141+
}
142+
143+
// WireMock reads the whole request before failing, so a socket that resets every connection
144+
// stands in for a server that drops the connection while the request body is still being sent.
145+
@Test
146+
void sendsAPatchRequestOnlyOnceWhenMaxRetriesIsZero() throws Exception {
147+
AtomicInteger connections = new AtomicInteger();
148+
try (ServerSocket server = new ServerSocket(0)) {
149+
Thread acceptor =
150+
new Thread(
151+
() -> {
152+
try {
153+
while (true) {
154+
Socket socket = server.accept();
155+
connections.incrementAndGet();
156+
socket.setSoLinger(true, 0);
157+
socket.close();
158+
}
159+
} catch (Exception closed) {
160+
// the server socket was closed at the end of the test
161+
}
162+
});
163+
acceptor.start();
164+
DeepLClientOptions options =
165+
new DeepLClientOptions().setServerUrl("http://127.0.0.1:" + server.getLocalPort());
166+
DeepLClient resetClient = new DeepLClient("k", options.setMaxRetries(0));
167+
StringBuilder longName = new StringBuilder();
168+
for (int i = 0; i < 1 << 23; i++) {
169+
longName.append('n');
170+
}
171+
ConnectionException thrown =
172+
assertThrows(
173+
ConnectionException.class,
174+
() -> resetClient.updateMultilingualGlossaryName("g1", longName.toString()));
175+
assertTrue(thrown.getCause() instanceof SocketException, thrown.toString());
176+
}
177+
assertEquals(1, connections.get());
178+
}
179+
121180
// A request to any host but the local server goes to the refusing proxy that the offline
122181
// suite configures through the JVM settings. The .invalid host never resolves, so a request
123182
// that ignored the proxy would fail with another message.

‎deepl-java/src/offlineTest/java/com/deepl/api/TranslateDocumentTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.junit.jupiter.api.Assertions.assertTrue;
2121

2222
import com.github.tomakehurst.wiremock.client.MappingBuilder;
23+
import com.github.tomakehurst.wiremock.http.Fault;
2324
import com.github.tomakehurst.wiremock.http.Request;
2425
import com.github.tomakehurst.wiremock.stubbing.Scenario;
2526
import com.github.tomakehurst.wiremock.stubbing.ServeEvent;
@@ -429,6 +430,16 @@ void sendsTheDocumentKeyWhenDownloadingTheResultToAFile() throws Exception {
429430
assertEquals("Protonenstrahl", Fixtures.readFile(output));
430431
}
431432

433+
@Test
434+
void throwsConnectionExceptionWhenTheDownloadedDocumentIsCutOff() {
435+
wm.stubFor(
436+
post("/v2/document/D1/result")
437+
.willReturn(aResponse().withFault(Fault.MALFORMED_RESPONSE_CHUNK)));
438+
assertThrows(
439+
ConnectionException.class,
440+
() -> client.translateDocumentDownload(HANDLE_OBJECT, new ByteArrayOutputStream()));
441+
}
442+
432443
@Test
433444
void throwsDocumentNotReadyWhenTheResultIsNotAvailableYet() {
434445
wm.stubFor(post("/v2/document/D1/result").willReturn(aResponse().withStatus(503)));

‎deepl-java/src/offlineTest/java/com/deepl/api/TranslationMemoriesTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static org.junit.jupiter.api.Assertions.assertThrows;
2222
import static org.junit.jupiter.api.Assertions.assertTrue;
2323

24+
import com.github.tomakehurst.wiremock.http.Fault;
2425
import com.github.tomakehurst.wiremock.stubbing.Scenario;
2526
import java.io.ByteArrayOutputStream;
2627
import java.io.File;
@@ -455,6 +456,16 @@ void downloadsTheExportToAStream() throws Exception {
455456
assertEquals((byte) 0xff, output.toByteArray()[1]);
456457
}
457458

459+
@Test
460+
void throwsConnectionExceptionWhenTheExportDownloadIsCutOff() {
461+
wm.stubFor(
462+
get("/storage/export").willReturn(aResponse().withFault(Fault.MALFORMED_RESPONSE_CHUNK)));
463+
assertThrows(
464+
ConnectionException.class,
465+
() ->
466+
client.downloadTranslationMemoryExport(completedExport(), new ByteArrayOutputStream()));
467+
}
468+
458469
@Test
459470
void leavesNoFileBehindWhenTheDownloadFails() throws Exception {
460471
wm.stubFor(get("/storage/export").willReturn(aResponse().withStatus(500)));

0 commit comments

Comments
 (0)