[6.5] Fix "curl error msg for low version"

[guilliamxavier]

My try at #2108 (comment): revert the incorrect version_compare test and append the URI if not already contained in the error message

[gmponos]

Is this related with this also? #2256

[guilliamxavier]

@gmponos: It seems this would fulfill #2256 indeed, as the URI (which includes the host) would be added to the message if not already there ^{(but maybe better would be to append it unconditionally?)}, whereas currently it is added only if the curl version is equal to 7.21.2 (which is a bug, #2108's intention was to add it if the curl version is less than 7.21.2 – but I still haven't seen any justification of how "7.21.2" was chosen).

Now you are also right that the request object (from which you can get the URI and/or the host) is passed alongside the message to the exception constructor (either a specific ConnectException or a generic RequestException) and you can try { /* your code */ } catch (RequestException $e) { /* use $e->getMessage() and $e->getRequest() */ }, but at least two people seemed to wish the URI/host be included in the (logged) error message...

To recap:

1. the current logic is wrong (and to be fixed);
2. open to discussion is when (and if at all) to add the URI ^{(or just the host?)} to the error message?

[gmponos]

Now you are also right that the request object (from which you can get the URI and/or the host) is passed alongside the message to the exception constructor

Well I was wrong about that.. ErrorHandlers do not have the logic in order to get the URI from the request. So if you let an exception bubble up then when it reaches your ErrorHandler you can not get these info and you need to start patching your error handler.

I guess that this is what the author of the linked issue wanted to describe here but somehow we got lost in translation :)

[guilliamxavier]

@gmponos: Okay :) so what's your opinion on when to add the URI to the error message?

[gmponos]

Haven't reviewed this thoroughly.. reason is this, I am not quite sure which branch it should target...

IIRC You have been into some other issues about targeting v6 or v7 so let me explain you my thinking...

if the change is very very small.. (doc, changelog, codestyle, phpstan-baseline fix) I am not going to bother asking ppl to retarget to v6 or v7. it seems like a headache to me.. opening two PR. closing the other etc etc...

Every other change I believe it should target v7 except the fixes for the INTL/ICU.. (Although I sould mention this to another issue I am trying to gather all the info and issues we had with intl the last months and get a full summary with solutions).

Nevertheless this is only MY thinking.. so let's wait for other maintainers as well..

[gmponos]

Hi. Can you target the v7 and we I will move this to 7.1 milestone

[guilliamxavier]

See #2661