From b8add1ace7b7a279b417361576fad0c1cda3efdc Mon Sep 17 00:00:00 2001 From: Colin Smith Date: Wed, 5 Aug 2026 11:28:44 +0100 Subject: [PATCH] feat: allow request timeouts to be configured via ClientOptions BaseClient builds its OkHttpClient with no timeout overrides, so every caller gets the OkHttp defaults of 10s connect, 10s read and 10s write, with no way to change them: Client exposes only (apiKey) and (apiKey, ClientOptions), ClientOptions carries the region, newHttpClient is private static, and the BaseClient constructors that accept an OkHttpClient are protected and unreachable from a Client subclass. For an integration whose calls are mostly writes, a 10s read timeout turns a slow-but-successful Recurly response into a failure whose outcome the caller cannot determine. A subscription create that times out may well have been applied, so the caller has to reconcile by reading back rather than retrying, and the safest version of that is a lot of machinery to carry for something a timeout setting would avoid. ClientOptions now takes optional connect, read, write and call timeouts, and newHttpClient applies each one that is set. Anything left unset keeps the current behaviour exactly, so this is source and behaviour compatible; the existing tests are unchanged and still pass. newHttpClient becomes package-private so the tests can assert on the client it builds. Four cases: defaults unchanged when nothing is configured, all four timeouts applied when set, timeouts applied individually without disturbing the others, and a configured timeout surviving construction through the public Client constructor. --- src/main/java/com/recurly/v3/BaseClient.java | 21 +++++-- .../java/com/recurly/v3/ClientOptions.java | 47 +++++++++++++++ .../java/com/recurly/v3/BaseClientTest.java | 59 +++++++++++++++++++ 3 files changed, 123 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/recurly/v3/BaseClient.java b/src/main/java/com/recurly/v3/BaseClient.java index 17b0a8dc..fcd18ef3 100644 --- a/src/main/java/com/recurly/v3/BaseClient.java +++ b/src/main/java/com/recurly/v3/BaseClient.java @@ -35,11 +35,11 @@ public abstract class BaseClient { private String apiUrl; protected BaseClient(final String apiKey) { - this(apiKey, newHttpClient(validateApiKey(apiKey)), new ClientOptions()); + this(apiKey, new ClientOptions()); } protected BaseClient(final String apiKey, final ClientOptions clientOptions) { - this(apiKey, newHttpClient(validateApiKey(apiKey)), clientOptions); + this(apiKey, newHttpClient(validateApiKey(apiKey), clientOptions), clientOptions); } protected BaseClient(final String apiKey, final OkHttpClient client) { @@ -59,9 +59,22 @@ private static String validateApiKey(final String apiKey) { return apiKey; } - private static OkHttpClient newHttpClient(final String apiKey) { + static OkHttpClient newHttpClient(final String apiKey, final ClientOptions clientOptions) { final OkHttpClient.Builder httpClientBuilder = new OkHttpClient.Builder(); - + + if (clientOptions.getConnectTimeout() != null) { + httpClientBuilder.connectTimeout(clientOptions.getConnectTimeout()); + } + if (clientOptions.getReadTimeout() != null) { + httpClientBuilder.readTimeout(clientOptions.getReadTimeout()); + } + if (clientOptions.getWriteTimeout() != null) { + httpClientBuilder.writeTimeout(clientOptions.getWriteTimeout()); + } + if (clientOptions.getCallTimeout() != null) { + httpClientBuilder.callTimeout(clientOptions.getCallTimeout()); + } + final String authToken = Credentials.basic(apiKey, ""); final HeaderInterceptor headerInterceptor = new HeaderInterceptor(authToken, Client.API_VERSION); diff --git a/src/main/java/com/recurly/v3/ClientOptions.java b/src/main/java/com/recurly/v3/ClientOptions.java index eacf7f7f..2416f09d 100644 --- a/src/main/java/com/recurly/v3/ClientOptions.java +++ b/src/main/java/com/recurly/v3/ClientOptions.java @@ -1,4 +1,5 @@ package com.recurly.v3; +import java.time.Duration; import java.util.HashMap; public class ClientOptions { @@ -15,6 +16,10 @@ public enum Regions { } private Regions region; + private Duration connectTimeout; + private Duration readTimeout; + private Duration writeTimeout; + private Duration callTimeout; public ClientOptions() { this.region = Regions.US; @@ -28,4 +33,46 @@ public void setRegion(Regions r) { public String getBaseUrl() { return regionsMap.get(this.region); } + + /** Connect timeout for the underlying HTTP client. Unset leaves the OkHttp default in place. */ + public void setConnectTimeout(Duration connectTimeout) { + this.connectTimeout = connectTimeout; + } + + public Duration getConnectTimeout() { + return this.connectTimeout; + } + + /** + * Read timeout for the underlying HTTP client. Unset leaves the OkHttp default in place. Raising + * this is how a caller tolerates a slow response instead of failing the request. + */ + public void setReadTimeout(Duration readTimeout) { + this.readTimeout = readTimeout; + } + + public Duration getReadTimeout() { + return this.readTimeout; + } + + /** Write timeout for the underlying HTTP client. Unset leaves the OkHttp default in place. */ + public void setWriteTimeout(Duration writeTimeout) { + this.writeTimeout = writeTimeout; + } + + public Duration getWriteTimeout() { + return this.writeTimeout; + } + + /** + * Timeout spanning the complete call, including redirects and retries. Unset leaves OkHttp's + * behaviour of applying no overall limit. + */ + public void setCallTimeout(Duration callTimeout) { + this.callTimeout = callTimeout; + } + + public Duration getCallTimeout() { + return this.callTimeout; + } } \ No newline at end of file diff --git a/src/test/java/com/recurly/v3/BaseClientTest.java b/src/test/java/com/recurly/v3/BaseClientTest.java index bd15991e..a38e8041 100644 --- a/src/test/java/com/recurly/v3/BaseClientTest.java +++ b/src/test/java/com/recurly/v3/BaseClientTest.java @@ -22,6 +22,7 @@ import okhttp3.Response; import org.apache.commons.io.IOUtils; +import java.time.Duration; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import org.junit.Assert; @@ -503,6 +504,64 @@ public void testInterpolatePathMatching() { assertEquals("/url_path/replacement", interpolatedPath); } + @Test + public void testDefaultTimeoutsAreUnchangedWhenNoneAreConfigured() { + final OkHttpClient httpClient = BaseClient.newHttpClient("apiKey", new ClientOptions()); + + assertEquals(10_000, httpClient.connectTimeoutMillis()); + assertEquals(10_000, httpClient.readTimeoutMillis()); + assertEquals(10_000, httpClient.writeTimeoutMillis()); + assertEquals(0, httpClient.callTimeoutMillis()); + } + + @Test + public void testConfiguredTimeoutsAreAppliedToTheHttpClient() { + final ClientOptions clientOptions = new ClientOptions(); + clientOptions.setConnectTimeout(Duration.ofSeconds(5)); + clientOptions.setReadTimeout(Duration.ofSeconds(30)); + clientOptions.setWriteTimeout(Duration.ofSeconds(20)); + clientOptions.setCallTimeout(Duration.ofSeconds(60)); + + final OkHttpClient httpClient = BaseClient.newHttpClient("apiKey", clientOptions); + + assertEquals(5_000, httpClient.connectTimeoutMillis()); + assertEquals(30_000, httpClient.readTimeoutMillis()); + assertEquals(20_000, httpClient.writeTimeoutMillis()); + assertEquals(60_000, httpClient.callTimeoutMillis()); + } + + @Test + public void testTimeoutsAreAppliedIndividually() { + final ClientOptions clientOptions = new ClientOptions(); + clientOptions.setReadTimeout(Duration.ofSeconds(45)); + + final OkHttpClient httpClient = BaseClient.newHttpClient("apiKey", clientOptions); + + assertEquals(45_000, httpClient.readTimeoutMillis()); + assertEquals(10_000, httpClient.connectTimeoutMillis()); + assertEquals(10_000, httpClient.writeTimeoutMillis()); + } + + @Test + public void testConfiguredTimeoutsSurviveClientConstruction() { + final ClientOptions clientOptions = new ClientOptions(); + clientOptions.setReadTimeout(Duration.ofSeconds(30)); + + final Client client = new Client("apiKey", clientOptions); + + assertEquals(30_000, httpClientOf(client).readTimeoutMillis()); + } + + private static OkHttpClient httpClientOf(final BaseClient client) { + try { + final Field field = BaseClient.class.getDeclaredField("client"); + field.setAccessible(true); + return (OkHttpClient) field.get(client); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(e.getMessage(), e); + } + } + private static String getResponseJson() { return "{ \"my_string\": \"aaron\" }"; }