RFC / WIP (do not merge): surface cURL errors

[Stelminator]

This is a work in progress, but I wanted to get comments sooner rather than later, since this is mostly complete.

I have a couple use cases involved here:

1. I have a need to detect different kinds of cURL errors from multi-requests, specifically CURLE_OPERATION_TIMEDOUT (28), since I have subrequests using timeouts. These currently manifest as 'requests.no_crlf_separator' due to nothing being returned from curl_multi_getcontent in request_multiple resulting in a parse error. These are detectable from the code in $done['code'], but none of this is exposed anywhere I can get to.
2. I want to be able to capture the time when an error has finished, and have the code available there. I've added this as a separate hook to not break existing parse_response implementations, but I'm open to suggestions here.

Please take a look and tell me what you think.

[mishan]

need to handle apostrophes

[mishan]

Pretty cool otherwise 👍

[mishan]

Class 'Requests_Exception_Transport_cURL_Easy' not found in /home/travis/build/rmccue/Requests/library/Requests/Transport/cURL.php on line 234

[Stelminator]

yup, only "mostly" complete. Still looking at what I want that inheritance chain to look like.

[rmccue]

I'd think these should all be in a single exception class, Requests_Exception_Transport_cURL. Unlike HTTP errors, these are mostly an internal thing. If we can even get the same behaviour across both cURL and sockets, I'd be in favour of a single class there. (We can pass the raw error data including the cURL code itself through in the exception data.)

Generally though, I think the approach of using a separate hook is good here, and we should definitely be detecting the errors better than we do currently. Thanks for your work on the PR so far 🌴

[rmccue]

If we can even get the same behaviour across both cURL and sockets, I'd be in favour of a single class there.

Forgot to mention also: if we can get these consistent across both, I wouldn't be against generic Requests_Exception_Timeout/etc exceptions. It's just the internal cURL behaviour I don't want to expose.

[Stelminator]

Ah, ok, yeah, as long as I can make a decision on how to handle the error code, I'd be in favor of fewer classes.

[Stelminator]

I know this needs some whitespace cleanup, but I can't seem to get the tests to pass locally on master, so I'm not what is and what is not a legitimate failure.

[Stelminator]

a bit cleaner now.

[Stelminator]

working on fixing up the tests.

[Stelminator]

ok, not sure what's up with those last two errors.

[Stelminator]

ok, please take a look at this version. I'm not sure why the tests are failing, but I don't see how the ones remaining could be due to my change. Earlier, I had messed up a merge/rebase, and so there were legitimate errors. seems like there's a network issue with the tests.

[rmccue]

Sorry for the delay here, had to work out why the tests were failing! Can you merge master into here please? :)

[rmccue]

Indentation needs to be tabs, not spaces.

[Stelminator]

Should be a bit cleaner now.

[rmccue]

Fantastic, thanks so much!