[Discussion] Add more connection failure tracking and tests.

[yschimke]

Unlikely to be correct, but if the ConnectPlan is the right place, then a few more questions.

• Are there more places for routeDatabase.failed?
• Where should routeDatabase.connected be called?
• Which exceptions indicate serious problems?

RealConnection.trackFailure doesn't look right for HTTP/2.

[yschimke]

This is the only failing test when routeDatabase.failed is not called. Seems like we are missing coverage.

[swankjesse]

Or perhaps, the coverage we do have (in RouteFailureTest) is overly pessimistic??

[yschimke]

Only failure from disabling the RouteDatabase tracking prior to this

CallTest[jvm] > readTimeoutFails()[jvm] FAILED
    java.lang.AssertionError: 
    Expecting actual not to be null
        at okhttp3.RecordedResponse.assertCode(RecordedResponse.java:61)
        at okhttp3.CallTest.readTimeoutFails(CallTest.kt:1023)

[swankjesse]

This will also capture cases where the call was canceled. Hmm.

[swankjesse]

And looking at the other call to routeDatabase.failed(), I think we should notify the proxy selector.

This is from RealConnection.kt:

    // Tell the proxy selector when we fail to connect on a fresh connection.
    if (failedRoute.proxy.type() != Proxy.Type.DIRECT) {
      val address = failedRoute.address
      address.proxySelector.connectFailed(
        address.url.toUri(), failedRoute.proxy.address(), failure
      )
    }

Perhaps the nicest structure is to move that snippet out of RealConnection and into RouteDatabase, so the two are always in sync.

[yschimke]

Yep, I wanted this to prompt discussion. I'm not clear that the current exception filtering is obviously right.

I'll split it out and use the opportunity to make a test.

[swankjesse]

ooooh does your fix cause this to connect to a good host? That is PRETTY RAD

[swankjesse]

Or perhaps, the coverage we do have (in RouteFailureTest) is overly pessimistic??