Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define object in case of Net::HTTPBadRequest #149

Merged
merged 1 commit into from Apr 29, 2015
Merged

Define object in case of Net::HTTPBadRequest #149

merged 1 commit into from Apr 29, 2015

Conversation

radar
Copy link
Contributor

@radar radar commented Apr 29, 2015

First, some background.

I have a Rails app which has a config/initializers/twilio.rb:

require 'twilio-ruby'

Twilio.configure do |config|
  config.account_sid = "[sid]"
  config.auth_token = "[auth token]"
end

I can successfully use this code to send an SMS:

sms_number = "[redacted]"
dialing_code = "+61"
phone_number = "[redacted]"

client = Twilio::REST::Client.new
message = client.account.messages.create({
  :from => "#{sms_number}",
  :to => "+#{dialing_code} #{phone_number}",
  :body => "Your verification code for Bike Exchange is #{verification_code}",
})

That works nicely.

Today, I wanted to validate phone numbers and I saw that Twilio now has a lookups API for that. There's no documentation in the README for this gem wrt to lookups, so I had to piece together how to use this. I guessed this (running in a rails console):

lookups = Twilio::REST::LookupsClient.new
lookups.get("+61[my phone number, but redacted]")

When this request is made, Twilio returns a "400 Bad Request" error. Unfortunately for the Twilio gem, the response body is empty, and so object isn't set at all. Net::HTTPBadRequest is indeed a kind of Net::HTTPClientError, which you can check with:

 Net::HTTPBadRequest < Net::HTTPClientError

Alternatively, you could look through the docs for a couple of minutes. Back on track: because object isn't set, the code inside this final if isn't able to raise the exception because object is nil. Instead what happens, is this:

[2] pry(main)> lookups.get("+[my redacted phone number]")
NoMethodError: undefined method `[]' for nil:NilClass
from /Users/ryanbigg/.gem/ruby/2.2.2/gems/twilio-ruby-4.0.0/lib/twilio-ruby/rest/base_client.rb:121:in `connect_and_send'

So either:

  1. I am using this library wrong and it's a PEBKAC, so there should be documentation written in the README informing people how to use it properly; or
  2. I am using it correctly and it's the library's fault some how.

Please advise.

@philnash
Copy link
Contributor

Hey @radar,

Looks to me like both of your final points are somewhat true. It's definitely the case that the library should not be throwing a NoMethodError undefined method[]' for nil:NilClass` on a lookup. However, there's also a documentation issue in that you have not been told how it's supposed to work.

The call you want to make is:

lookups = Twilio::REST::LookupsClient.new
lookups.phone_numbers.get("+61[my phone number, but redacted]")

Which should return you information about your phone number.

I do think that this change is correct though and should save some hassle in the case of others falling into the same trap.

So, I'll merge this and get some documentation going too.

Thanks for the in depth report!

philnash added a commit that referenced this pull request Apr 29, 2015
Define object in case of Net::HTTPBadRequest
@philnash philnash merged commit 3766d18 into twilio:master Apr 29, 2015
ajtack pushed a commit to ajtack/twilio-ruby that referenced this pull request Apr 28, 2017
…nextgen

Add guides missing next-gen java snippets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants