-
Notifications
You must be signed in to change notification settings - Fork 13
Open
Description
In the ruby library I've tried to catch most errors and bubble them up in a meaningful way. For example when I send a fax to a number that I'm not yet allowed to send a fax to I get:
fax = interfax.deliver(faxNumber: "+11111111112", file: 'spec/test.pdf')
InterFAX::Client::BadRequestError: Bad request (400): {"code":-111,"message":"Attempting to fax to a number that is not the designated fax number in a developer account","moreInfo":null}In this library I get
>>> fax = interfax.deliver(fax_number="+442084978650", files=["tests/test.pdf"])
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "interfax/outbound.py", line 20, in deliver
files=self._generate_files(files))
File "interfax/client.py", line 76, in post
return self._request('POST', url, **kwargs)
File "interfax/client.py", line 89, in _request
return self._parse_response(request(method, url, **kwargs))
File "interfax/client.py", line 117, in _parse_response
response.raise_for_status()
File "/Users/cbetta/.pyenv/versions/2.7.12/lib/python2.7/site-packages/requests/models.py", line 862, in raise_for_status
raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: https://rest.interfax.net/outbound/faxes?faxNumber=%2B442084978650
Activity
cbetta commentedon Oct 7, 2016
FYI here is the pretty basic logic I have applied in the Ruby gem https://github.com/interfax/interfax-ruby/blob/ccefaf015320403d9ff5eb37bc259a7686500b3f/lib/interfax/client.rb#L116-L128
danielknell commentedon Oct 10, 2016
I did that initially but that error class has additional info that could be useful when debugging, and seemed more sensible than copying all that data across into a new hierarchy of errors, open to changing it though.
cbetta commentedon Oct 10, 2016
@danielknell oh I agree on that. I'm not too worried about wrapping it in new error objects, that's just a convenience. What I am more worried about is that my error exposes the JSON body:
{"code":-111,"message":"Attempting to fax to a number that is not the designated fax number in a developer account","moreInfo":null}Which is actually useful to the end user. Your code just tells me
Bad Requestwithout context.cbetta commentedon Feb 1, 2017
@danielknell where did we leave this?
danielknell commentedon Feb 2, 2017
I got weighed down with other stuff and then forgot about it, sorry, i will try and find some time to look soon.
cbetta commentedon Feb 2, 2017
Thanks @danielknell, any time you might have available to make this change would be much appreciated.