Make sure errors messages are correctly encoded

Merge pull request #595 from openfisca/msg-encoding-587

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+# 21.0.3 [#595](https://github.com/openfisca/openfisca-core/pull/595)
+
+#### Bug fix
+
+- Fix API response encoding from ascii to utf-8
+  * Improve user message by displaying `UnicodeDecodeError` information
+
 # 21.0.2 [#589](https://github.com/openfisca/openfisca-core/pull/589) [#600](https://github.com/openfisca/openfisca-core/pull/600) [#605](https://github.com/openfisca/openfisca-core/pull/605)
 
 _Note: the 21.0.1 and 21.0.0 versions have been unpublished due to performance issues_
@@ -94,7 +101,6 @@ holder.set_input(period, np.asarray([0])) # Highly not recommanded
 
 - When calculating an Enum variable, the output will be an [EnumArray](http://openfisca.readthedocs.io/en/latest/enum_array.html#module-openfisca_core.indexed_enums).
 
-
 # 20.0.0 [#590](https://github.com/openfisca/openfisca-core/pull/583)
 
 #### Breaking changes

openfisca_core/entities.py

@@ -71,7 +71,7 @@ def init_variable_values(self, entity_object, entity_id):
 
             if not isinstance(variable_values, dict):
                 raise SituationParsingError(path_in_json,
-                    'Invalid type: must be of type object. Input variables must be set for specific periods. For instance: {"salary": {"2017-01": 2000, "2017-02": 2500}}')
+                    u'Invalid type: must be of type object. Input variables must be set for specific periods. For instance: {"salary": {"2017-01": 2000, "2017-02": 2500}}')
 
             holder = self.get_holder(variable_name)
             for date, value in variable_values.iteritems():
@@ -90,15 +90,14 @@ def init_variable_values(self, entity_object, entity_id):
                         except KeyError:
                             possible_values = [item.name for item in holder.variable.possible_values]
                             raise SituationParsingError(path_in_json,
-                                "'{}' is not a valid value for '{}'. Possible values are ['{}'].".format(
-                                    value, variable_name, "', '".join(possible_values)
-                                    ).encode('utf-8')
+                                u"'{}' is not a valid value for '{}'. Possible values are ['{}'].".format(
+                                    value, variable_name, "', '".join(possible_values))
                                 )
                     try:
                         array[entity_index] = value
                     except (ValueError, TypeError) as e:
                         raise SituationParsingError(path_in_json,
-                    'Invalid type: must be of type {}.'.format(holder.variable.json_type))
+                            u'Invalid type: must be of type {}.'.format(holder.variable.json_type))
 
                     holder.buffer[period] = array
 
@@ -116,7 +115,7 @@ def finalize_variables_init(self):
                     # It is only raised when we consume the buffer. We thus don't know which exact key caused the error.
                     # We do a basic research to find the culprit path
                     culprit_path = next(
-                        dpath.search(self.entities_json, "*/{}/{}".format(e.variable_name, str(e.period)), yielded = True),
+                        dpath.search(self.entities_json, u"*/{}/{}".format(e.variable_name, str(e.period)), yielded = True),
                         None)
                     if culprit_path:
                         path = [self.plural] + culprit_path[0].split('/')
@@ -146,7 +145,7 @@ def clone(self, new_simulation):
     def __getattr__(self, attribute):
         projector = get_projector_from_shortcut(self, attribute)
         if not projector:
-            raise Exception("Entity {} has no attribute {}".format(self.key, attribute))
+            raise Exception(u"Entity {} has no attribute {}".format(self.key, attribute))
         return projector
 
     @classmethod
@@ -174,11 +173,11 @@ def check_variable_defined_for_entity(self, variable_name):
 
     def check_array_compatible_with_entity(self, array):
         if not self.count == array.size:
-            raise ValueError("Input {} is not a valid value for the entity {}".format(array, self.key))
+            raise ValueError(u"Input {} is not a valid value for the entity {}".format(array, self.key))
 
     def check_role_validity(self, role):
         if role is not None and not type(role) == Role:
-            raise ValueError("{} is not a valid role".format(role))
+            raise ValueError(u"{} is not a valid role".format(role))
 
     def check_period_validity(self, variable_name, period):
         if period is None:
@@ -249,7 +248,7 @@ def value_from_partner(self, array, entity, role):
         self.check_role_validity(role)
 
         if not role.subroles or not len(role.subroles) == 2:
-            raise Exception('Projection to partner is only implemented for roles having exactly two subroles.')
+            raise Exception(u'Projection to partner is only implemented for roles having exactly two subroles.')
 
         [subrole_1, subrole_2] = role.subroles
         value_subrole_1 = entity.project(entity.value_from_person(array, subrole_1))
@@ -306,7 +305,7 @@ def init_from_json(self, entities_json):
         if self.persons_to_allocate:
             unallocated_person = self.persons_to_allocate.pop()
             raise SituationParsingError([self.plural],
-                '{0} has been declared in {1}, but is not a member of any {2}. All {1} must be allocated to a {2}.'.format(
+                u'{0} has been declared in {1}, but is not a member of any {2}. All {1} must be allocated to a {2}.'.format(
                     unallocated_person, self.simulation.persons.plural, self.key)
                 )
 
@@ -320,12 +319,12 @@ def init_members(self, roles_json, entity_id):
                 check_type(person_id, basestring, [self.plural, entity_id, role_id, str(index)])
                 if person_id not in self.simulation.persons.ids:
                     raise SituationParsingError([self.plural, entity_id, role_id],
-                        "Unexpected value: {0}. {0} has been declared in {1} {2}, but has not been declared in {3}.".format(
+                        u"Unexpected value: {0}. {0} has been declared in {1} {2}, but has not been declared in {3}.".format(
                             person_id, entity_id, role_id, self.simulation.persons.plural)
                         )
                 if person_id not in self.persons_to_allocate:
                     raise SituationParsingError([self.plural, entity_id, role_id],
-                        "{} has been declared more than once in {}".format(
+                        u"{} has been declared more than once in {}".format(
                             person_id, self.plural)
                         )
                 self.persons_to_allocate.discard(person_id)
@@ -423,7 +422,7 @@ def value_from_person(self, array, role, default = 0):
         self.check_role_validity(role)
         if role.max != 1:
             raise Exception(
-                'You can only use value_from_person with a role that is unique in {}. Role {} is not unique.'
+                u'You can only use value_from_person with a role that is unique in {}. Role {} is not unique.'
                 .format(self.key, role.key)
                 )
         self.simulation.persons.check_array_compatible_with_entity(array)

openfisca_core/periods.py

@@ -123,8 +123,8 @@ def period(self, unit, size = 1):
         >>> instant('2014-2-3').period('day', size = 2)
         Period((u'day', Instant((2014, 2, 3)), 2))
         """
-        assert unit in (u'day', u'month', u'year'), 'Invalid unit: {} of type {}'.format(unit, type(unit))
-        assert isinstance(size, int) and size >= 1, 'Invalid size: {} of type {}'.format(size, type(size))
+        assert unit in (u'day', u'month', u'year'), u'Invalid unit: {} of type {}'.format(unit, type(unit))
+        assert isinstance(size, int) and size >= 1, u'Invalid size: {} of type {}'.format(size, type(size))
         return Period((unicode(unit), self, size))
 
     def offset(self, offset, unit):
@@ -212,18 +212,18 @@ def offset(self, offset, unit):
             if unit == u'month':
                 day = 1
             else:
-                assert unit == u'year', 'Invalid unit: {} of type {}'.format(unit, type(unit))
+                assert unit == u'year', u'Invalid unit: {} of type {}'.format(unit, type(unit))
                 month = 1
                 day = 1
         elif offset == 'last-of':
             if unit == u'month':
                 day = calendar.monthrange(year, month)[1]
             else:
-                assert unit == u'year', 'Invalid unit: {} of type {}'.format(unit, type(unit))
+                assert unit == u'year', u'Invalid unit: {} of type {}'.format(unit, type(unit))
                 month = 12
                 day = 31
         else:
-            assert isinstance(offset, int), 'Invalid offset: {} of type {}'.format(offset, type(offset))
+            assert isinstance(offset, int), u'Invalid offset: {} of type {}'.format(offset, type(offset))
             if unit == u'day':
                 day += offset
                 if offset < 0:
@@ -256,7 +256,7 @@ def offset(self, offset, unit):
                 if day > month_last_day:
                     day = month_last_day
             else:
-                assert unit == u'year', 'Invalid unit: {} of type {}'.format(unit, type(unit))
+                assert unit == u'year', u'Invalid unit: {} of type {}'.format(unit, type(unit))
                 year += offset
                 # Handle february month of leap year.
                 month_last_day = calendar.monthrange(year, month)[1]
@@ -343,7 +343,7 @@ def __str__(self):
 
     @property
     def date(self):
-        assert self.size == 1, '"date" is undefined for a period of size > 1: {}'.format(self)
+        assert self.size == 1, u'"date" is undefined for a period of size > 1: {}'.format(self)
         return self.start.date
 
     @property
@@ -622,7 +622,7 @@ def stop(self):
                     year += 1
                     month -= 12
             else:
-                assert unit == u'year', 'Invalid unit: {} of type {}'.format(unit, type(unit))
+                assert unit == u'year', u'Invalid unit: {} of type {}'.format(unit, type(unit))
                 year += size
             day -= 1
             if day < 1:

openfisca_core/simulations.py

@@ -76,7 +76,7 @@ def instantiate_entities(self, simulation_json):
 
             if not persons_json:
                 raise SituationParsingError([self.tax_benefit_system.person_entity.plural],
-                    'No {0} found. At least one {0} must be defined to run a simulation.'.format(self.tax_benefit_system.person_entity.key).encode('utf-8'))
+                    u'No {0} found. At least one {0} must be defined to run a simulation.'.format(self.tax_benefit_system.person_entity.key))
             self.persons = self.tax_benefit_system.person_entity(self, persons_json)
         else:
             self.persons = self.tax_benefit_system.person_entity(self)
@@ -254,7 +254,7 @@ def check_type(input, type, path = []):
 
     if not isinstance(input, type):
         raise SituationParsingError(path,
-            "Invalid type: must be of type '{}'.".format(json_type_map[type]))
+            u"Invalid type: must be of type '{}'.".format(json_type_map[type]))
 
 
 class SituationParsingError(Exception):

openfisca_web_api_preview/app.py

@@ -50,6 +50,7 @@ def create_app(tax_benefit_system,
     app.wsgi_app = ProxyFix(app.wsgi_app, num_proxies = 1)  # Fix request.remote_addr to get the real client IP address
     CORS(app, origins = '*')
 
+    app.config['JSON_AS_ASCII'] = False  # When False, lets jsonify encode to utf-8
     app.url_map.strict_slashes = False  # Accept url like /parameters/
 
     data = build_data(tax_benefit_system)
@@ -96,23 +97,29 @@ def calculate():
             simulation = Simulation(tax_benefit_system = tax_benefit_system, simulation_json = input_data)
         except SituationParsingError as e:
             abort(make_response(jsonify(e.error), e.code or 400))
+        except UnicodeEncodeError as e:
+            abort(make_response(jsonify({"error": "'" + e[1] + "' is not a valid ASCII value."}), 400))
 
         requested_computations = dpath.util.search(input_data, '*/*/*/*', afilter = lambda t: t is None, yielded = True)
 
-        for computation in requested_computations:
-            path = computation[0]
-            entity_plural, entity_id, variable_name, period = path.split('/')
-            variable = tax_benefit_system.get_variable(variable_name)
-            result = simulation.calculate(variable_name, period)
-            entity = simulation.get_entity(plural = entity_plural)
-            entity_index = entity.ids.index(entity_id)
+        try:
+            for computation in requested_computations:
+                path = computation[0]
+                entity_plural, entity_id, variable_name, period = path.split('/')
+                variable = tax_benefit_system.get_variable(variable_name)
+                result = simulation.calculate(variable_name, period)
+                entity = simulation.get_entity(plural = entity_plural)
+                entity_index = entity.ids.index(entity_id)
 
-            if variable.value_type == Enum:
-                entity_result = result.decode()[entity_index].name
-            else:
-                entity_result = result.tolist()[entity_index]
+                if variable.value_type == Enum:
+                    entity_result = result.decode()[entity_index].name
+                else:
+                    entity_result = result.tolist()[entity_index]
+
+                dpath.util.set(input_data, path, entity_result)
 
-            dpath.util.set(input_data, path, entity_result)
+        except UnicodeEncodeError as e:
+            abort(make_response(jsonify({"error": "'" + e[1] + "' is not a valid ASCII value."}), 400))
 
         return jsonify(input_data)
 
@@ -157,7 +164,12 @@ def track_requests(response):
 
     @app.errorhandler(500)
     def internal_server_error(e):
-        response = jsonify({"error": "Internal server error: " + e.message.strip(os.linesep).replace(os.linesep, ' ')})
+        if type(e) == UnicodeEncodeError:
+            response = jsonify({"error": "Internal server error: '" + e[1] + "' is not a valid ASCII value."})
+        elif e.message:
+            response = jsonify({"error": "Internal server error: " + e.message.strip(os.linesep).replace(os.linesep, ' ')})
+        else:
+            response = jsonify({"error": "Internal server error: " + str(e)})
         response.status_code = 500
         return response
 

setup.py

@@ -7,7 +7,7 @@
 
 setup(
     name = 'OpenFisca-Core',
-    version = '21.0.2',
+    version = '21.0.3',
     author = 'OpenFisca Team',
     author_email = 'contact@openfisca.fr',
     classifiers = [

tests/web_api/basic_case/test_calculate.py

@@ -181,3 +181,88 @@ def test_enum_wrong_value():
         "Possible values are ['owner', 'tenant', 'free_lodger', 'homeless']",
         dpath.get(response_json, "households/_/housing_occupancy_status/2017-01")
         )
+
+
+def test_encoding_period_value():
+    simulation_json = json.dumps({
+        "persons": {
+            "toto": {}
+            },
+        "households": {
+            "_": {
+                "housing_occupancy_status": {
+                    "2017-07": "Locataire ou sous-locataire d‘un logement loué vide non-HLM"
+
+                    },
+                "parent": [
+                    "toto",
+                    ]
+                }
+            }
+        })
+
+    # No UnicodeDecodeError
+    expected_response = "'Locataire ou sous-locataire d‘un logement loué vide non-HLM' is not a valid value for 'housing_occupancy_status'. Possible values are "
+    response = post_json(simulation_json)
+    assert expected_response in response.data, expected_response + os.linesep + " NOT FOUND IN: " + os.linesep + response.data
+    assert_equal(response.status_code, BAD_REQUEST, response.data)
+
+
+def test_encoding_entity_name():
+    simulation_json = json.dumps({
+        "persons": {
+            "O‘Ryan": {},
+            "Renée": {}
+            },
+        "households": {
+            "_": {
+                "parents": [
+                    "O‘Ryan",
+                    "Renée"
+                    ]
+                }
+            }
+        })
+
+    # No UnicodeDecodeError
+    expected_response = "'O‘Ryan' is not a valid ASCII value."
+    response = post_json(simulation_json)
+    assert expected_response in response.data, str(response.status_code) + " " + response.data
+    assert_equal(response.status_code, BAD_REQUEST, response.data)
+
+
+def test_encoding_period_id():
+    simulation_json = json.dumps({
+        "persons": {
+            "bill": {
+                "salary": {
+                    "2017": 60000
+                    }
+                },
+            "bell": {
+                "salary": {
+                    "2017": 60000
+                    }
+                }
+            },
+        "households": {
+            "_": {
+                "parents": ["bill", "bell"],
+                "housing_tax": {
+                    "à": 400
+                    },
+                "accomodation_size": {
+                    "2017-01": 300
+                    },
+                "housing_occupancy_status": {
+                    "2017-01": "tenant"
+                    }
+                }
+            }
+        })
+
+    # No UnicodeDecodeError
+    expected_response = "'à' is not a valid ASCII value."
+    response = post_json(simulation_json)
+    assert expected_response in response.data, str(response.status_code) + " " + response.data
+    assert_equal(response.status_code, BAD_REQUEST, response.data)