Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add more granurality to errors. #21

Closed
wants to merge 2 commits into from

3 participants

@rafael

Greetings,

I was thinking that it would be nice to have more granularity in the exceptions raised by the twilio-ruby gem. For instance, you could add specific errors for invalid number or international calls not available.

I made a small commit to show you this idea. I could add others as well :)

What do you think?

@rafael rafael Add more granurality to client errors.
* Add specific exception for invalid number and international sms unavailable errors.
* Raise these exceptions in the case the error code is 21401 or 21408 respectively.
68c3bd7
@kevinburke

Hi Rafael,
Thanks for the pull request, and for using the library. One problem with adding special cases for each error message is that we have hundreds of different types of errors, and they often change :) It might be tough to code all of them into the client library, as we would have to update the client library each time we change our server code.

Maybe we can give more informative error messages, though, when things go wrong. What could we do to make the current error message better? What does it look like when you get an error??

@rafael

Hi Kevin,

Thanks a lot for your answer. Yeah I see your point that is going to be more code to maintain. I suggested this because in my code I want to handle different types of errors. So for instance I have something like this:

begin
self.account.sms.messages.create(from: FROM_NUMBER,
                                 to: to,
                                 body: message)
rescue Twilio::REST::RequestError => exp
  if ["The 'To' number  is not a valid phone number",
      "International SMS delivery not available"].include?(exp.message)
    Rails.logger.error exp.message
  else
    raise
  end
end

However, I think is not the right solution. Because the message can change in the server for the reasons you mentioned, and also right now, for the same error I get slightly variations of the message . For invalid number sometimes I get:

"The 'To' number is not a valid phone number"

and others

(number eg. 1+55555) is not a valid phone number.

So I didn't want to start coding regular expressions that match the message that I should receive.

What I wanted to do is something like this:

begin                                                                        
  @client.account.sms.messages.create(from: FROM_NUMBER, to: to, body: message)                                   
rescue Twilio::REST::InvalidNumber, Twilio:REST:xxxx => exp                                  
  Rails.logger.error exp.message                                                                                                    
end                                                                       

What do you think?

Thanks a lot again for you support.

@andrewmbenton

Hi Rafael,

We have error codes which are more stable than the strings. These error codes get passed back in the error response along with the message. For instance, the invalid 'To' number message is error 21211: http://www.twilio.com/docs/errors/21211. I think building more specific exceptions around the error codes is a great idea since those almost never change.

@rafael

Hi Andrew,

Thanks for your answer. I agree with you and that's what I was trying to do with my pull request ( I don't know if you read the code I attached). Because I don't see a way to retrieve the error code from the response in ruby. If you go to line 261 (https://github.com/twilio/twilio-ruby/blob/master/lib/twilio-ruby/rest/client.rb#L261) of client.rb it seems that you are only putting the message in the exception not the error code.

Should we store the entire object in the exception, so there is a way to retrieve the error code?

Thanks in advanced for your support.

@rafael

Hello Guys,

I updated the pull request to use a different approach. I don't what you think about this. I'm using something similar like the solutions proposed here: http://avdi.org/talks/exceptional-ruby-2011-02-04/.

I think in this way, you don't have to maintain every possible kind of exception that the client could raise, but the error code is stored in the exception, enabling the third party developers to retrieve it easily.

What do you think of this approach?

Cheers.

@kevinburke

Note that the 2008 API reports errors in a different format... here is an example

{u'TwilioResponse': {u'RestException': {u'Message': u'The requested resource was not found',
                                    u'Status': 404}}}
@andrewmbenton

2008 api is not relevant since the twilio-ruby library is exclusively a 2010 client.

@rafael

Cool. So do you think is possible to merge the changes?

@andrewmbenton

sorry it took me awhile, but i have merged your changes into the develop branch (with some slight changes related to style). i am planning a 3.6.0 release this weekend and your changes will be in that. thanks for the good idea and implementation!

@rafael

Oh no problem! Thanks for merging these changes. I'll be waiting for release 3.6.0 then :)

@rafael rafael closed this
@andrewmbenton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 14, 2012
  1. @rafael

    Add more granurality to client errors.

    rafael authored
    * Add specific exception for invalid number and international sms unavailable errors.
    * Raise these exceptions in the case the error code is 21401 or 21408 respectively.
Commits on Mar 15, 2012
  1. @rafael
This page is out of date. Refresh to see the latest.
View
2  lib/twilio-ruby/rest/client.rb
@@ -258,7 +258,7 @@ def connect_and_send(request) # :doc:
end
end
if response.kind_of? Net::HTTPClientError
- raise Twilio::REST::RequestError, object['message']
+ raise Twilio::REST::RequestError.exception(object['message'], object['code'])
end
object
end
View
9 lib/twilio-ruby/rest/errors.rb
@@ -1,6 +1,13 @@
module Twilio
module REST
- class RequestError < StandardError; end
class ServerError < StandardError; end
+
+ class RequestError< StandardError
+ attr_reader :code
+ def initialize(msg, code=nil);
+ super(msg);
+ @code = code;
+ end
+ end
end
end
Something went wrong with that request. Please try again.