Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ internal inline fun <reified T> jsonHandler(jsonMapper: JsonMapper): Handler<T>
try {
jsonMapper.readValue(response.body(), jacksonTypeRef())
} catch (e: Exception) {
throw OpenAIInvalidDataException("Error reading response", e)
throw OpenAIInvalidDataException("Error reading response", e, response.headers())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

package com.openai.core.handlers

import com.openai.core.http.Headers
import com.openai.core.http.HttpResponse
import com.openai.core.http.HttpResponse.Handler
import com.openai.core.http.PhantomReachableClosingStreamResponse
Expand Down Expand Up @@ -30,7 +31,7 @@ internal fun <T> streamHandler(
// We wrap the `lines` instead of the top-level sequence because
// we only want to catch `IOException` from the reader; not from
// the user's own code.
IOExceptionWrappingSequence(lines),
IOExceptionWrappingSequence(lines, response.headers()),
)
}
}
Expand All @@ -53,7 +54,10 @@ internal fun <T> streamHandler(
}

/** A sequence that catches, wraps, and rethrows [IOException] as [OpenAIIoException]. */
private class IOExceptionWrappingSequence<T>(private val sequence: Sequence<T>) : Sequence<T> {
private class IOExceptionWrappingSequence<T>(
private val sequence: Sequence<T>,
private val headers: Headers,
) : Sequence<T> {

override fun iterator(): Iterator<T> {
val iterator = sequence.iterator()
Expand All @@ -63,14 +67,14 @@ private class IOExceptionWrappingSequence<T>(private val sequence: Sequence<T>)
try {
iterator.next()
} catch (e: IOException) {
throw OpenAIIoException("Stream failed", e)
throw OpenAIIoException("Stream failed", e, headers)
}

override fun hasNext(): Boolean =
try {
iterator.hasNext()
} catch (e: IOException) {
throw OpenAIIoException("Stream failed", e)
throw OpenAIIoException("Stream failed", e, headers)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
package com.openai.errors

import com.openai.core.http.Headers
import java.util.Optional

class OpenAIInvalidDataException
@JvmOverloads
constructor(message: String? = null, cause: Throwable? = null) : OpenAIException(message, cause)
constructor(
message: String? = null,
cause: Throwable? = null,
private val headers: Headers? = null,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the original Kotlin constructor ABI

Adding headers to the primary constructor changes its synthetic default-argument descriptor; @JvmOverloads preserves the Java overloads but not that Kotlin ABI. A Kotlin consumer compiled against the previous release using OpenAIInvalidDataException("message") invokes the old (String, Throwable, int, DefaultConstructorMarker) constructor and will get NoSuchMethodError after upgrading without recompilation. Keep the original two-parameter primary constructor and introduce header support without replacing its default constructor; the identical change in OpenAIIoException.kt needs the same treatment.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the latest commits. Both exception classes now keep the original two-parameter primary constructor unchanged and use an internal three-argument secondary constructor only when response headers are available. This preserves the existing Kotlin default-argument constructor ABI while keeping the new optional headers() accessor.

) : OpenAIException(message, cause) {

fun headers(): Optional<Headers> = Optional.ofNullable(headers)
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
package com.openai.errors

import com.openai.core.http.Headers
import java.util.Optional

class OpenAIIoException
@JvmOverloads
constructor(message: String? = null, cause: Throwable? = null) : OpenAIException(message, cause)
constructor(
message: String? = null,
cause: Throwable? = null,
private val headers: Headers? = null,
) : OpenAIException(message, cause) {

fun headers(): Optional<Headers> = Optional.ofNullable(headers)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.openai.core.handlers

import com.fasterxml.jackson.databind.json.JsonMapper
import com.openai.core.http.Headers
import com.openai.core.http.HttpResponse
import com.openai.errors.OpenAIInvalidDataException
import java.io.InputStream
import kotlin.test.Test
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.assertThrows

internal class JsonHandlerTest {

@Test
fun jsonHandler_whenBodyCannotBeRead_exposesResponseHeaders() {
val headers = Headers.builder().put("x-request-id", "req_123").build()
val handler = jsonHandler<Map<String, Any>>(JsonMapper.builder().build())

val error =
assertThrows<OpenAIInvalidDataException> {
handler.handle(httpResponse("{".byteInputStream(), headers))
}

assertThat(error).hasMessage("Error reading response")
assertThat(error.headers()).contains(headers)
}

private fun httpResponse(body: InputStream, headers: Headers): HttpResponse =
object : HttpResponse {

override fun statusCode(): Int = 200

override fun headers(): Headers = headers

override fun body(): InputStream = body

override fun close() {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,15 @@ internal class StreamHandlerTest {

@Test
fun streamHandler_whenReaderThrowsIOException_wrapsException() {
val headers = Headers.builder().put("x-request-id", "req_123").build()
val handler = streamHandler<String> { _, lines -> lines.forEach {} }
val streamResponse = handler.handle(httpResponse("a\nb\nc\n".byteInputStream().throwing()))
val streamResponse =
handler.handle(httpResponse("a\nb\nc\n".byteInputStream().throwing(), headers))

val e = assertThrows<OpenAIIoException> { streamResponse.stream().forEach {} }
assertThat(e).hasMessage("Stream failed")
assertThat(e).hasCauseInstanceOf(IOException::class.java)
assertThat(e.headers()).contains(headers)
}

@Test
Expand All @@ -68,12 +71,15 @@ internal class StreamHandlerTest {
assertThat(e).isSameAs(ioException)
}

private fun httpResponse(body: InputStream): HttpResponse =
private fun httpResponse(
body: InputStream,
headers: Headers = Headers.builder().build(),
): HttpResponse =
object : HttpResponse {

override fun statusCode(): Int = 0

override fun headers(): Headers = Headers.builder().build()
override fun headers(): Headers = headers

override fun body(): InputStream = body

Expand Down