Add faraday and curl's errors in PDC::Error - #54
Conversation
d929ecf to
678ae6d
Compare
| @@ -1,8 +1,38 @@ | |||
| # require 'curb' | |||
sthaha
left a comment
sthaha
left a comment
There was a problem hiding this comment.
I like the patch but some comments that are worth addressing. And please don't forget tests :-)
| raise PDC::ConnectionFailed.new e | ||
| rescue Curl::Err::HostResolutionError => e | ||
| raise PDC::HostResolutionError.new e | ||
| rescue Curl::Err::CurlError => e |
There was a problem hiding this comment.
We don't want a CurlError since it is leaking the details of the implmentation outside, think how the users code will have to change if we replace curl with something else. Curl and Faraday are implementation details of the gem that users shouldn't care about.
How about we return PDC::Error if it is a CurlError
There was a problem hiding this comment.
I have changed this part, and also keep the PDC's errors raise as the original way, just wrap the non pdc errors, user can use a PDC:Error to catch it.
d76ef59 to
792d37e
Compare
|
@sthaha any updates about my new changes :)? |
|
@simonbaird and @simozhan please help to review this patch, after it merged, I will change the et's related codes. |
|
After look at the et's codes, I found there are more changes needed here. |
792d37e to
a05616a
Compare
Add faraday and curl's errors in PDC::Error JIRA: PDC-1845
a05616a to
212330d
Compare
4 participants
Add faraday and curl's errors in PDC::Error
JIRA: PDC-1845