[DO NOT REVIEW] Use single instance of BTHTTP in BTAPIClient

scannillo commented 2 months ago

Background

Conversations to reduce SDK latency led to investigation if our SDK is re-using TLS connections, when possible. Luckily, URLSession will try to re-use TLS connections whenever possible. As long as we use a single URLSession instance, this re-use logic is automated for us.

⚠️ Note: URLSession cannot re-use a connection across unique URLSession instances.

Why does this matter? Reusing an existing TLS connection eliminates the need to re-do the time consuming handshake process. A TLS handshake negotiates secure connection parameters like encryption algorithms and keys for a safe communication session. By reusing the connection, subsequent requests can leverage the established session, leading to faster data transfer. Setting up a new connection requires compute power from both the mobile device & sever.

Investigation

Apple exposes a boolean property isReusedConnection to see if a network request was able to re-use an existing connection.

The following print statement was added to the BTHTTP:

func urlSession(_ session: URLSession, task: URLSessionTask, didFinishCollecting metrics: URLSessionTaskMetrics) {
    metrics.transactionMetrics.forEach { transaction in
        print("reused: \(transaction.isReusedConnection), sessionInstance: \(session), path: \(transaction.request.url?.path)")
    }
}

This prints the path of the network request, if the connection was re-used, the location ID in memory of the URLSession instance.

For these tests, I launched the PP Web demo, clicked the "Vault" button, canceled, clicked the "Checkout button, canceled, then clicked the "Vault" button again.

Results from `main`:

reused: false, sessionInstance: <__NSURLSessionLocal: 0x102a180c0>, path: Optional("/merchants/dfy45jdj3dxkmz5m/client_api/v1/configuration")
reused: false, sessionInstance: <__NSURLSessionLocal: 0x102f1cfa0>, path: Optional("/merchants/dfy45jdj3dxkmz5m/client_api/v1/paypal_hermes/setup_billing_agreement")
reused: true, sessionInstance: <__NSURLSessionLocal: 0x102f1cfa0>, path: Optional("/merchants/dfy45jdj3dxkmz5m/client_api/v1/paypal_hermes/create_payment_resource")
reused: true, sessionInstance: <__NSURLSessionLocal: 0x102f1cfa0>, path: Optional("/merchants/dfy45jdj3dxkmz5m/client_api/v1/paypal_hermes/setup_billing_agreement")

You can see after the v1/configuration fetch, we do not re-use the connection for the subsequent setup_billing_agreement call. Also note that the ID in memory for the URLSession is different. This is because v1/configuration calls were using a separate instance of URLSession than the rest of our HTTP calls made throughout the SDK.

Results w/ this PR:

reused: false, sessionInstance: <__NSURLSessionLocal: 0x129d1a610>, path: Optional("/merchants/dfy45jdj3dxkmz5m/client_api/v1/configuration")
reused: true, sessionInstance: <__NSURLSessionLocal: 0x129d1a610>, path: Optional("/merchants/dfy45jdj3dxkmz5m/client_api/v1/paypal_hermes/setup_billing_agreement")
reused: true, sessionInstance: <__NSURLSessionLocal: 0x129d1a610>, path: Optional("/merchants/dfy45jdj3dxkmz5m/client_api/v1/paypal_hermes/create_payment_resource")
reused: true, sessionInstance: <__NSURLSessionLocal: 0x129d1a610>, path: Optional("/merchants/dfy45jdj3dxkmz5m/client_api/v1/paypal_hermes/setup_billing_agreement")

With these changes, we do re-use the connection created by the v1/configuration call, and use the same URLSession instance for subsequent GW API calls.

Changes

Remove BTAPIClient.configurationHTTP property and only use BTAPIClient.http
- This duplicate instance of BTHTTP was not doing anything differently (we previously discovered that it wasn't doing anything for caching in PR #1287)

Impact

A few local tests show the connection setup taking ~110ms on a strong WiFi connection in production. This could reduce latency by that much, since we are potentially omitting 1 handshake. (Potentially b/c the determination is up to URLSession).

Checklist

[X] Added a changelog entry

Authors

@scannillo

scannillo commented 2 months ago

Added [DO NOT REVIEW] label. There is a problem revealed by the UI tests, where configurationHTTP was indrectly accounting for edge case Client Tokens that didn't contain a clientApiURL param. Will update this PR, or open a new one with future updates.

scannillo commented 1 month ago

Replaced by https://github.com/braintree/braintree_ios/pull/1314

braintree / braintree_ios