alexreisner / geocoder

Complete Ruby geocoding solution.
http://www.rubygeocoder.com
MIT License
6.35k stars 1.19k forks source link

NoMethodError in fetch_coordinates comes up randomly (like once every 50 calls to <model>#update) #21

Closed seanahrens closed 13 years ago

seanahrens commented 13 years ago

Thanks again, Alex, for the gem.

The following errors seems to happen randomly (it's probably happened about five or six times to me now, and it happens just kind of randomly during every 50th or so call to users#update)

--- The Error --- NoMethodError in UsersController#update

You have a nil object when you didn't expect it! You might have expected an instance of Array. The error occurred while evaluating nil.[] Rails.root: /Users/Sean/Sites/c3p

Application Trace | Framework Trace | Full Trace app/models/user.rb:226:in geocode_address' app/controllers/users_controller.rb:73:inupdate' app/controllers/users_controller.rb:72:in `update' Request

--- User.rb ----

after_validation :geocode_address geocoded_by :city

def geocode_address fetch_coordinates if self[:city] end


Any ideas? Thanks, Alex.

Cheers.

alexreisner commented 13 years ago

It's hard to tell whether this is a problem with geocoder or your app, although I think it's your app. To test, try changing your method to:

def geocode_address
  if self[:city]
    fetch_coordinates
  end
end

The next time the error occurs I think it will be on the line with self[:city] and not the one with fetch_coordinates. Still, I don't understand how self could be nil so I'm not sure what's going on here.

Also, is there a reason to use self[:city] instead of just city?

seanahrens commented 13 years ago

I switched the code to

def geocode_address if self[:city] fetch_coordinates end end

so I can try to narrow it down next time.

I switched initially to using self[:city] instead of city (I think) because I was getting this error when it was city. I switched to self[:city] because my initially suspicion was that the error was because geocode_address was using self[:city](the value in the db) and I was checking for city (the value in memory) [I figured this could make sense because before a call to .save, the former could be nil while the latter could be something].

Speaking of which, should geocoder be throwing an exception if you don't have the "if city/self[:city]" condition? I noticed it is for me, and that's one of the primary reasons I have the condition there to begin with.

Related to this, (feel free not to answer because this is out of the scope), is there an easy way to write a conditional to only call geocoder if the city has changed? As my code currently stands, every time I save my user objects, the geocoder runs.

Thanks, Alex.

alexreisner commented 13 years ago

I think it would be simpler to just do something like this:

  after_validation :fetch_coordinates, :if => { |obj| obj.city }

You also don't really need to check for the presence of city as no call to the geocoding API will be made for a blank query. To fetch only if the city has changed I believe you can do this:

after_validation :fetch_coordinates, :if => { |obj| obj.city_changed? }
seanahrens commented 13 years ago

She strikes back! And this time, is shown to be on the "fetch_coordinates" line :/

Error

NoMethodError in UsersController#update

You have a nil object when you didn't expect it! You might have expected an instance of Array. The error occurred while evaluating nil.[] Rails.root: /Users/Sean/Sites/c3p

Application Trace | Framework Trace | Full Trace app/models/user.rb:268:in geocode_address' app/controllers/users_controller.rb:73:inupdate' app/controllers/users_controller.rb:72:in `update'

User.rb
def geocode_address
  if self[:city]
    #for debugging
  end

  if self[:city]
    fetch_coordinates   # line 268
  end
end
seanahrens commented 13 years ago

As an added bit of info, today I changed my local database from sqlite to postgresql, and the same error popped up again randomly during a user save. Just in case that helps eliminate database type as an issue (not sure exactly if this would be suspect in any case).

alexreisner commented 13 years ago

Can the problem be duplicated by trying to save the same object again? If so then please post the object's data. If not this is going to be very tricky to trace.

Also, is it possible it's failing when the city is blank?

seanahrens commented 13 years ago

Yep, when the city is blank (and I remove the if city condition on geocoding), it creates the following exception:

NoMethodError in UsersController#update

You have a nil object when you didn't expect it! You might have expected an instance of Array. The error occurred while evaluating nil.size Rails.root: /Users/Sean/Sites/c3p

Application Trace | Framework Trace | Full Trace app/models/user.rb:271:in geocode_address' app/controllers/users_controller.rb:75:inupdate' app/controllers/users_controller.rb:74:in `update'

....where geocode address looks like this

def geocode_address
  # if self[:city]
  #   #for debugging
  # end
  #   
  # if self[:city]
    fetch_coordinates   #line 271
  # end
end
alexreisner commented 13 years ago

OK, then I think what you want to do is this:

unless city.blank?
  fetch_coordinates
end

I believe what's happening is that the city is "", which does not evaluate to false, so the block is being executed. There is a bug in geocoder (recently fixed but not yet in the gem) which is causing the exception but the above should prevent the buggy code from running.

seanahrens commented 13 years ago

Thanks, my code has been changed to use ".blank?" now. I'll grab the updated gem when it's posted.

Thanks again.

alexreisner commented 13 years ago

Great. I'm closing this issue but let me know if it comes up again.

seanahrens commented 13 years ago

She is a persistant bug :) Hmmf. If I can sometime soon, maybe I'll try opening up the source and seeing if I can narrow the bug down myself. It's a strange bug because it is happening when the city value is a valid, non-blank string.

NoMethodError in UsersController#update

You have a nil object when you didn't expect it! You might have expected an instance of Array. The error occurred while evaluating nil.[] Rails.root: /Users/Sean/Sites/c3p

Application Trace | Framework Trace | Full Trace app/models/user.rb:285:in geocode_address' app/controllers/users_controller.rb:127:inupdate' app/controllers/users_controller.rb:126:in `update'

where.. after_validation :geocode_address geocoded_by :city

def geocode_address
  unless city.blank?
   fetch_coordinates # line 285
  end
end

and where...

User.find(1).city => "Sausalito, CA"

alexreisner commented 13 years ago

Does it happen whenever the city is "Sausalito, CA" or just sometimes?

seanahrens commented 13 years ago

Alex, I think this is the error that may be going hand-in-hand with the exception in this thread:

Google Geocoding API not responding fast enough (see Geocoder::Configuration.timeout to set limit).

And it's not tied to "Sausalito, CA"

alexreisner commented 13 years ago

It seems like something somewhere in your app is depending on coordinates being present and causing an exception when they're not (because the geocoder has taken too long to respond, as it inevitably does).

alexreisner commented 13 years ago

Finally! I was able to reproduce this. This problem only exists with Google and is related to the difficulty in extracting a city from Google's response. There is no field called 'city', so I have to make some guesses about whether what we want is the 'locality', 'sublocality', 'administrative_area_level_3', or something else.

Right now I'm not sure of the correct way to solve this but I'll come up with something before the next gem release. Thanks for being patient.

seanahrens commented 13 years ago

Yayyyyyy! Thanks Alex, great to hear I wasn't just trippin.

On Fri, Mar 18, 2011 at 2:56 PM, alexreisner < reply@reply.github.com>wrote:

Finally! I was able to reproduce this. This problem only exists with Google and is related to the difficulty in extracting a city from Google's response. There is no field called 'city', so I have to make some guesses about whether what we want is the 'locality', 'sublocality', 'administrative_area_level_3', or something else.

Right now I'm not sure of the correct way to solve this but I'll come up with something before the next gem release. Thanks for being patient.

Reply to this email directly or view it on GitHub: https://github.com/alexreisner/geocoder/issues/21#comment_891257

alexreisner commented 13 years ago

Fix is committed and will be included in 0.9.11 gem release.

seanahrens commented 13 years ago

Awesome. Can't wait. Thanks.

On Wednesday, March 23, 2011, alexreisner reply@reply.github.com wrote:

Fix is committed and will be included in 0.9.11 gem release.

Reply to this email directly or view it on GitHub: https://github.com/alexreisner/geocoder/issues/21#comment_910297

Sean Ahrens 415.633.6234