ConnectionFailed vs TimeoutError with Patron

[stayhero]

Hi,

I recently discovered that we receive a TimeoutError when Patron is not able to connect to the host and the open_timeout limit is reached.

From reading the source, we should receive a ConnectionFailed error?

rescue ::Patron::TimeoutError => err
  if err.message == "Connection time-out"
     raise Faraday::Error::ConnectionFailed, err
  else
     raise Faraday::Error::TimeoutError, err
   end

I believe Patron returns another error message ("Connection timed out after ..."). I created a branch with a failing test. Unfortunately most other adapters also seem to return TimeoutError in such cases. Therefore, before I create a pull request, I'd like to know the desired behavior?

Forked branch with fixed error for Patron but new errors for other adapters:
master...stayhero:fix-patron-connection-timeout

[iMacTia]

Hi @stayhero and thanks for pointing this out.
I believe this issue is due to the fact that Patron changed the error message into something else since the check was put in place.
I understand your PR would solve the issue, but honestly this will just temporarily work, until they'll change the message again into something else.
Therefor, unless Patron has a way to differentiate the two cases from a class point of view, I would prefer to go for the removal of the ConnectionFailed raise and keep only the TimeoutError one.

[stayhero]

@iMacTia Ok, thanks. After digging a bit deeper, it seems the error message comes directly from curl (https://github.com/curl/curl/blob/e60fe20fdf94e829ba5fce33f7a9d6c281149f7d/lib/multi.c#L1375) and they never changed it. It's rather that curl raises different error messages when doing a simple http connect vs. a "multi-connect" on timeout.

Hence to me it seems you could basically trust curl to not change their message in the future. If it's not causing to much headaches to you I'd create a pull-request simply checking for both error messages?

Or do you think about deprecating Patron support in near future and its generally better to use Typhoeus for the average Faraday user who wants a curl-backend? 😬

[iMacTia]

@stayhero Typhoeus is actually deprecated already 😅 so Patron is the only curl-backend alternative at this point.
Indeed being this a curl thing rather than Patron makes it more "stable" but then I assume the error message depends on the version of curl installed on the server.
At this point it makes sense to have both error messages tested, so we'll support both versions

[iMacTia]

Fixed in #687