Closed
Description
I was reviewing some callers upgrading from OkHttp 4 to OkHttp 5 and I was surprised to see this:
val okhttp3.Response.grpcStatus: GrpcStatus
get() =
runCatching { trailers() }.getOrNull()?.get("grpc-status")?.toIntOrNull()?.let { GrpcStatus.get(it) }
?: header("grpc-status")?.toIntOrNull()?.let { GrpcStatus.get(it) }
?: GrpcStatus.OK
The runCatching()
here is calling trailers()
and ignoring its IllegalStateException
if trailers aren’t available.
The OkHttp 5 behaviour change is there – now instead of throwing it blocks. I assumed changing code from throwing to blocking was safe, and I was wrong!
Anyway, if you were okay catching exceptions we used to have an API with semantics like getTrailersIfTheyAreReady()
. OkHttp 5 replaces that with a function with semantics like awaitTrailers()
.
I’m tempted to add a new function that has semantics similar to what we had in OkHttp 4.x:
class Response {
...
/**
* Returns the trailers on this response if they're available
* immediately. This will be empty if the trailers are available,
* but have no data.
*
* This returns null if the trailers are not available yet.
*
* Typically this will be null for HTTP/1 calls until the response
* is read up to the last chunk. It will be null for HTTP/2 calls
* until the thread that services the network has processed the
* inbound frame that includes the trailers.
*
* @throws IOException if the response is unreadable
*/
@Throws(IOException::class)
fun peekTrailers(): Headers? {
...
}
}