diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 4ad3fef33..d661066e8 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "0.18.0" + ".": "0.18.1" } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index b9e0bbb54..e909c9986 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 0.18.1 (2025-01-29) + +Full Changelog: [v0.18.0...v0.18.1](https://github.com/openai/openai-java/compare/v0.18.0...v0.18.1) + +### Bug Fixes + +* **client:** don't leak responses when retrying ([#185](https://github.com/openai/openai-java/issues/185)) ([a13f967](https://github.com/openai/openai-java/commit/a13f967d4406cb447b14260c5435f77d91b6b1be)) + ## 0.18.0 (2025-01-29) Full Changelog: [v0.17.0...v0.18.0](https://github.com/openai/openai-java/compare/v0.17.0...v0.18.0) diff --git a/README.md b/README.md index f2446b5aa..0048284f5 100644 --- a/README.md +++ b/README.md @@ -9,8 +9,8 @@ -[![Maven Central](https://img.shields.io/maven-central/v/com.openai/openai-java)](https://central.sonatype.com/artifact/com.openai/openai-java/0.18.0) -[![javadoc](https://javadoc.io/badge2/com.openai/openai-java/0.18.0/javadoc.svg)](https://javadoc.io/doc/com.openai/openai-java/0.18.0) +[![Maven Central](https://img.shields.io/maven-central/v/com.openai/openai-java)](https://central.sonatype.com/artifact/com.openai/openai-java/0.18.1) +[![javadoc](https://javadoc.io/badge2/com.openai/openai-java/0.18.1/javadoc.svg)](https://javadoc.io/doc/com.openai/openai-java/0.18.1) @@ -25,7 +25,7 @@ The REST API documentation can be found on [platform.openai.com](https://platfor ### Gradle ```kotlin -implementation("com.openai:openai-java:0.18.0") +implementation("com.openai:openai-java:0.18.1") ``` ### Maven @@ -34,7 +34,7 @@ implementation("com.openai:openai-java:0.18.0") com.openai openai-java - 0.18.0 + 0.18.1 ``` diff --git a/build.gradle.kts b/build.gradle.kts index e734683f2..aa608f927 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -8,7 +8,7 @@ repositories { allprojects { group = "com.openai" - version = "0.18.0" // x-release-please-version + version = "0.18.1" // x-release-please-version } subprojects { diff --git a/openai-java-core/src/main/kotlin/com/openai/core/http/RetryingHttpClient.kt b/openai-java-core/src/main/kotlin/com/openai/core/http/RetryingHttpClient.kt index 34a77e732..f74d19a93 100644 --- a/openai-java-core/src/main/kotlin/com/openai/core/http/RetryingHttpClient.kt +++ b/openai-java-core/src/main/kotlin/com/openai/core/http/RetryingHttpClient.kt @@ -57,15 +57,17 @@ private constructor( } response - } catch (t: Throwable) { - if (++retries > maxRetries || !shouldRetry(t)) { - throw t + } catch (throwable: Throwable) { + if (++retries > maxRetries || !shouldRetry(throwable)) { + throw throwable } null } val backoffMillis = getRetryBackoffMillis(retries, response) + // All responses must be closed, so close the failed one before retrying. + response?.close() Thread.sleep(backoffMillis.toMillis()) } } @@ -113,6 +115,8 @@ private constructor( } val backoffMillis = getRetryBackoffMillis(retries, response) + // All responses must be closed, so close the failed one before retrying. + response?.close() return sleepAsync(backoffMillis.toMillis()).thenCompose { executeWithRetries(requestWithRetryCount, requestOptions) } @@ -223,23 +227,23 @@ private constructor( return Duration.ofNanos((TimeUnit.SECONDS.toNanos(1) * backoffSeconds * jitter).toLong()) } - private fun sleepAsync(millis: Long): CompletableFuture { - val future = CompletableFuture() - TIMER.schedule( - object : TimerTask() { - override fun run() { - future.complete(null) - } - }, - millis - ) - return future - } - companion object { private val TIMER = Timer("RetryingHttpClient", true) + private fun sleepAsync(millis: Long): CompletableFuture { + val future = CompletableFuture() + TIMER.schedule( + object : TimerTask() { + override fun run() { + future.complete(null) + } + }, + millis + ) + return future + } + @JvmStatic fun builder() = Builder() } diff --git a/openai-java-core/src/test/kotlin/com/openai/core/http/RetryingHttpClientTest.kt b/openai-java-core/src/test/kotlin/com/openai/core/http/RetryingHttpClientTest.kt index 36c5cc91b..b1c6dd752 100644 --- a/openai-java-core/src/test/kotlin/com/openai/core/http/RetryingHttpClientTest.kt +++ b/openai-java-core/src/test/kotlin/com/openai/core/http/RetryingHttpClientTest.kt @@ -5,6 +5,9 @@ import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo import com.github.tomakehurst.wiremock.junit5.WireMockTest import com.github.tomakehurst.wiremock.stubbing.Scenario import com.openai.client.okhttp.OkHttpClient +import com.openai.core.RequestOptions +import java.io.InputStream +import java.util.concurrent.CompletableFuture import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.params.ParameterizedTest @@ -13,11 +16,49 @@ import org.junit.jupiter.params.provider.ValueSource @WireMockTest internal class RetryingHttpClientTest { + private var openResponseCount = 0 private lateinit var httpClient: HttpClient @BeforeEach fun beforeEach(wmRuntimeInfo: WireMockRuntimeInfo) { - httpClient = OkHttpClient.builder().baseUrl(wmRuntimeInfo.httpBaseUrl).build() + val okHttpClient = OkHttpClient.builder().baseUrl(wmRuntimeInfo.httpBaseUrl).build() + httpClient = + object : HttpClient { + override fun execute( + request: HttpRequest, + requestOptions: RequestOptions + ): HttpResponse = trackClose(okHttpClient.execute(request, requestOptions)) + + override fun executeAsync( + request: HttpRequest, + requestOptions: RequestOptions + ): CompletableFuture = + okHttpClient.executeAsync(request, requestOptions).thenApply { trackClose(it) } + + override fun close() = okHttpClient.close() + + private fun trackClose(response: HttpResponse): HttpResponse { + openResponseCount++ + return object : HttpResponse { + private var isClosed = false + + override fun statusCode(): Int = response.statusCode() + + override fun headers(): Headers = response.headers() + + override fun body(): InputStream = response.body() + + override fun close() { + response.close() + if (isClosed) { + return + } + openResponseCount-- + isClosed = true + } + } + } + } resetAllScenarios() } @@ -35,6 +76,7 @@ internal class RetryingHttpClientTest { assertThat(response.statusCode()).isEqualTo(200) verify(1, postRequestedFor(urlPathEqualTo("/something"))) + assertNoResponseLeaks() } @ParameterizedTest @@ -60,6 +102,7 @@ internal class RetryingHttpClientTest { assertThat(response.statusCode()).isEqualTo(200) verify(1, postRequestedFor(urlPathEqualTo("/something"))) + assertNoResponseLeaks() } @ParameterizedTest @@ -116,6 +159,7 @@ internal class RetryingHttpClientTest { postRequestedFor(urlPathEqualTo("/something")) .withHeader("x-stainless-retry-count", equalTo("2")) ) + assertNoResponseLeaks() } @ParameterizedTest @@ -156,6 +200,7 @@ internal class RetryingHttpClientTest { postRequestedFor(urlPathEqualTo("/something")) .withHeader("x-stainless-retry-count", equalTo("42")) ) + assertNoResponseLeaks() } @ParameterizedTest @@ -186,8 +231,13 @@ internal class RetryingHttpClientTest { assertThat(response.statusCode()).isEqualTo(200) verify(2, postRequestedFor(urlPathEqualTo("/something"))) + assertNoResponseLeaks() } private fun HttpClient.execute(request: HttpRequest, async: Boolean): HttpResponse = if (async) executeAsync(request).get() else execute(request) + + // When retrying, all failed responses should be closed. Only the final returned response should + // be open. + private fun assertNoResponseLeaks() = assertThat(openResponseCount).isEqualTo(1) }