Error handling missing geocode data

[mfenner]

NoMethodError (undefined method `[]' for nil:NilClass):
  app/models/geocode.rb:60:in `block in load_from_addresses_impl'
  app/models/geocode.rb:59:in `each'
  app/models/geocode.rb:59:in `load_from_addresses_impl'
  app/models/geocode.rb:82:in `load_from_addresses'
  lib/chart_data.rb:100:in `generate_data_for_articles_by_location_chart'
  app/controllers/reports_controller.rb:214:in `multiple_documents_visualizations'
  app/controllers/reports_controller.rb:99:in `visualizations'
  app/controllers/application_controller.rb:42:in `save_session_dois'

[jure]

Interesting, haven't seen this one myself. Taking a look.

[jure]

I don't understand this one, how can results be nil at all, while it's initialized just a few lines above this exception:

    results = {}
    orig_addresses = []
    geos.each do |geo|
      results[geo.address] = geo

https://github.com/articlemetrics/alm-report/blob/master/app/models/geocode.rb#L57-L60

[mfenner]

Hm, good question. Maybe still worth rewriting. I personally don't like each, and prefer reduce, or maybe each_with_object to save some memory.

[jure]

Can you, just for sanity's sake, check that these lines of code are identical in the deployed repo? I still can't parse how this error would occur.

[mfenner]

The deployed code is exactly the same on lines 57-60.

[jure]

Even if this was a case of the wrong line being reported for some reason (I've seen that before, but can't recall what exactly the reason was), I just can't see how this could raise an exception.

  def self.load_from_addresses_impl(addresses)
    geos, not_in_cache = check_cache(addresses.keys.clone)
    if geos.length < addresses.length
      db_geos = Geocode.where(:address => not_in_cache)
      add_to_cache(db_geos)
      geos += db_geos
    end
    results = {}
    orig_addresses = []
    geos.each do |geo|
      results[geo.address] = geo
      orig_addresses << addresses[geo.address.downcase]
    end
    [results, orig_addresses]
  end

geos isn't nil, because otherwise it would fail on geos.length. results isn't nil because we initialize it with an empty hash. Even if geo.address is nil, results[nil] is simply nil. addresses isn't nil either, otherwise it would fail at addresses.length. It's worth refactoring this, sure, but I'd hate to not understand how this can possibly raise an exception.

[jure]

I'd be interested in knowing if this is still occurring, as some of the Geocode implementation changed. Do we have any other information about the original error? I.e. what report it was or what DOI caused it, etc?

[mfenner]

We can deploy the code as it is and watch the error log, as this is indeed a bit obscure.