Don't overwrite CURLOPT_HTTP_VERSION option

[ob-stripe]

r? @remi-stripe
cc @stripe/api-libraries

Don't force CURLOPT_HTTP_VERSION to CURL_HTTP_VERSION_2TLS if a value is already set. This allows users to set their own value for CURLOPT_HTTP_VERSION when using custom clients, e.g.:

$client = new \Stripe\HttpClient\CurlClient([CURLOPT_HTTP_VERSION => CURL_HTTP_VERSION_1_1]);
\Stripe\ApiRequestor::setHttpClient($client);

[remi-stripe]

That looks good to me but I have some feels:

• we should update the readme
• we should likely add the version to the request headers (where we track curl and such) because we should be able to know
• not entirely sure I follow whether this could be a breaking change
• we likely need some kind of test? Not fully convinced

Can I ask that you re-assign to brandur or someone else than me? I'm always wary of approving core libs changes

[ob-stripe]

we should update the readme

I think this is already being covered by the "Custom cURL options" section and we don't need to cover every single option. Overriding the HTTP version should rarely be necessary.

we should likely add the version to the request headers (where we track curl and such) because we should be able to know

I don't think this is necessary, we already know which HTTP version is being used. If we want to make it clearer in our own internal logs we can do that without relying on a custom header.

not entirely sure I follow whether this could be a breaking change

It's not. Even if someone is already passing a custom CURLOPT_HTTP_VERSION today and it's being ignored, this wouldn't break anything. (Unless the user is passing an invalid value, but I wouldn't call that a breaking change.)

we likely need some kind of test? Not fully convinced

I meant to write one, but it's actually difficult to test this without some refactoring. I'll give it another try though.

[ob-stripe]

Okay, so the only way we can test this right now is by switching the visibility of executeRequestWithRetries from private to protected so that we can mock the method in tests in order to check the contents of $opts. I'd rather not do that though.

[remi-stripe]

r? @brandur-stripe

[brandur-stripe]

LGTM.

[ob-stripe]

Thank you both!

[ob-stripe]

Released as 6.29.1.