Mapbox error handling

server/lib/python/cartodb_services/cartodb_services/geocoder.py

@@ -9,9 +9,6 @@ import json
 PRECISION_PRECISE = 'precise'
 PRECISION_INTERPOLATED = 'interpolated'
 
-EMPTY_RESPONSE = [[], {}]
-
-
 def geocoder_metadata(relevance, precision, match_types):
     return {
         'relevance': round(relevance, 2),
@@ -20,6 +17,18 @@ def geocoder_metadata(relevance, precision, match_types):
     }
 
 
+def geocoder_error_response(message):
+    return [[], {'error': message}]
+
+
+# Single empty result
+EMPTY_RESPONSE = [[], {}]
+# HTTP 429 and related
+TOO_MANY_REQUESTS_ERROR_RESPONSE = geocoder_error_response('Rate limit exceeded')
+# Full empty _batch_geocode response
+EMPTY_BATCH_RESPONSE = []
+
+
 def compose_address(street, city=None, state=None, country=None):
     return ', '.join(filter(None, [street, city, state, country]))
 
@@ -42,7 +51,7 @@ def run_street_point_geocoder(plpy, GD, geocoder, service_manager, username, org
         service_manager.assert_within_limits(quota=False)
         geocode_results = geocoder.bulk_geocode(searches)
         results = []
-        if geocode_results:
+        if not geocode_results == EMPTY_BATCH_RESPONSE:
             for result in geocode_results:
                 if len(result) > 2:
                     metadata = result[2]
@@ -65,6 +74,7 @@ def run_street_point_geocoder(plpy, GD, geocoder, service_manager, username, org
                     results.append([result[0], None, json.dumps(metadata)])
 
         empty_count = len(searches) - success_count - failed_count
+        logger.debug("--> Success: {}; empty: {}; failed: {}".format(success_count, empty_count, failed_count))
         service_manager.quota_service.increment_success_service_use(success_count)
         service_manager.quota_service.increment_empty_service_use(empty_count)
         service_manager.quota_service.increment_failed_service_use(failed_count)

server/lib/python/cartodb_services/cartodb_services/mapbox/bulk_geocoder.py

@@ -1,8 +1,6 @@
-import json, requests, time
+import requests
-from requests.adapters import HTTPAdapter
 from cartodb_services import StreetPointBulkGeocoder
 from cartodb_services.mapbox import MapboxGeocoder
-from cartodb_services.tools.exceptions import ServiceException
 from iso3166 import countries
 from cartodb_services.tools.country import country_to_iso3
 

server/lib/python/cartodb_services/cartodb_services/mapbox/geocoder.py

@@ -5,7 +5,7 @@ Python client for the Mapbox Geocoder service.
 import json
 import requests
 from mapbox import Geocoder
-from cartodb_services.geocoder import PRECISION_PRECISE, PRECISION_INTERPOLATED, geocoder_metadata, EMPTY_RESPONSE
+from cartodb_services.geocoder import PRECISION_PRECISE, PRECISION_INTERPOLATED, geocoder_metadata, EMPTY_RESPONSE, EMPTY_BATCH_RESPONSE, TOO_MANY_REQUESTS_ERROR_RESPONSE, geocoder_error_response
 from cartodb_services.metrics import Traceable
 from cartodb_services.tools.exceptions import ServiceException
 from cartodb_services.tools.qps import qps_retry
@@ -69,7 +69,7 @@ class MapboxGeocoder(Traceable):
                     result.append(EMPTY_RESPONSE)
             return result
         else:
-            return EMPTY_RESPONSE
+            return EMPTY_BATCH_RESPONSE
 
     def _extract_lng_lat_from_feature(self, feature):
         geometry = feature[ENTRY_GEOMETRY]
@@ -118,9 +118,17 @@ class MapboxGeocoder(Traceable):
         :param city:
         :param state_province:
         :param country: Country ISO 3166 code
-        :return: [x, y] on success, [] on error
+        :return: [x, y] on success, raises ServiceException on error
         """
-        return self.geocode_meta(searchtext, city, state_province, country)[0]
+        response = self.geocode_meta(searchtext, city, state_province, country)
+        if response:
+            error_message = response[1].get('error', None)
+            if error_message:
+                raise ServiceException(error_message, None)
+            else:
+                return response[0]
+        else:
+            return EMPTY_RESPONSE
 
     @qps_retry(qps=10)
     def geocode_meta(self, searchtext, city=None, state_province=None,
@@ -138,7 +146,8 @@ class MapboxGeocoder(Traceable):
 
         free_search = ', '.join(address)
 
-        return self.geocode_free_text_meta([free_search], country)[0]
+        response = self.geocode_free_text_meta([free_search], country)
+        return response[0] if response else EMPTY_RESPONSE
 
     @qps_retry(qps=10)
     def geocode_free_text_meta(self, free_searches, country=None):
@@ -157,24 +166,26 @@ class MapboxGeocoder(Traceable):
 
             if response.status_code == requests.codes.ok:
                 return self._parse_geocoder_response(response.text)
+            elif response.status_code == requests.codes.too_many_requests:
+                return [TOO_MANY_REQUESTS_ERROR_RESPONSE] * len(free_searches)
             elif response.status_code == requests.codes.bad_request:
-                return EMPTY_RESPONSE
+                return EMPTY_BATCH_RESPONSE
             elif response.status_code == requests.codes.unprocessable_entity:
-                return EMPTY_RESPONSE
+                return EMPTY_BATCH_RESPONSE
             else:
-                raise ServiceException(response.status_code, response)
+                msg = "Unkown status: {}".format(response.status_code)
+                return [geocoder_error_response(msg)] * len(free_searches)
         except requests.Timeout as te:
             # In case of timeout we want to stop the job because the server
             # could be down
-            self._logger.error('Timeout connecting to Mapbox geocoding server',
+            msg = 'Timeout connecting to Mapbox geocoding server'
-                               te)
+            self._logger.error(msg, te)
-            raise ServiceException('Error geocoding {0} using Mapbox'.format(
+            return [geocoder_error_response(msg)] * len(free_searches)
-                free_search), None)
         except requests.ConnectionError as ce:
             # Don't raise the exception to continue with the geocoding job
             self._logger.error('Error connecting to Mapbox geocoding server',
                                exception=ce)
-            return EMPTY_RESPONSE
+            return EMPTY_BATCH_RESPONSE
 
     def _escape(self, free_search):
         # Semicolon is used to separate batch geocoding; there's no documented