Skip to content

Socket errors on GET requests with large headers will not trigger a retry on Android #8712

Open
@gbak

Description

@gbak

I am seeing an issue when a GET request that uses a pooled connection may throw a SocketException in the Android socketWrite0 method. This exception is handled and retried correctly when the call to socketWrite is made through OkHttp's finishRequest() method.

Exchange.kt

fun finishRequest() {

try {
    codec.finishRequest()
} catch (e: IOException) {
    eventListener.requestFailed(call, e)
    trackFailure(e)
    throw e
   }
}

The Problem

When the GET request has a large set of headers, about 8K this SocketException exception is thrown while looping over the request headers in writeReqeusts(). The exception thrown in writeRequest() is caught in the CallServerInteceptor.kt: intercept() try/catch block which does not have any retry logic.

Http1ExchangeCodec.kt

fun writeRequest(headers: Headers, requestLine: String) {
    check(state == STATE_IDLE) { "state: $state" }
    sink.writeUtf8(requestLine).writeUtf8("\r\n")
    for (i in 0 until headers.size) {
      sink.writeUtf8(headers.name(i))
          .writeUtf8(": ")
          .writeUtf8(headers.value(i))
          .writeUtf8("\r\n")
    }
    sink.writeUtf8("\r\n")
    state = STATE_OPEN_REQUEST_BODY
  }

  override fun readResponseHeaders(expectContinue: Boolean): Response.Builder? {
    check(state == STATE_OPEN_REQUEST_BODY ||
        state == STATE_WRITING_REQUEST_BODY ||
        state == STATE_READ_RESPONSE_HEADERS) {
      "state: $state"
    }

The Socket exception is thrown in writeRequests and the state is never set to STATE_OPEN_REQEUST_BODY. The exception is caught by OkHttp and then readResponseHeaders is called and the call will eventually fail on the check() for the state. I eventually see a state:0 in the exception that's thrown back to me.

Expected outcome
It would be expected that if I use the .retryOnConnectionFailure(true) option the request would be retried on an expected socket error.

I've Tried

  • Reducing the Connection Pool keep-alive to a small value and that helps, but that has drawbacks with not utilizing the pool as much.

  • Forcing the pool to do the more exhaustive check on the socket in the debugger also works, that successfully checks the socket before it is used and expires it. It would be nice to have a knob to allow adding GET requests to the socket checks.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug in existing code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions