Make sure all errors messages are correctly encoded #595

sandcha · 2017-11-16T16:44:07Z

Fixes #587
Connected to #580

Bug fix

Fix API response encoding from ascii to utf-8 (to manage non ascii characters).
- Improve user message by displaying UnicodeDecodeError information in json error response

Anna-Livia

A/ Would it be helpful to check Openfisca-france for any error message ?
B/ Would you create an accentfull enum in country template and have test in core checking that the encoding works in error messages ?
C/ When testing entity names with accent, I got an internal server error

{
	"persons": {
		"bill": {
			"salary": {
				"2017": 60000
			}
		},
		"béll": {
			"salary": {
				"2017": 60000
			}
		}
	},
	"households": {
		"_": {
			"parents": ["bill", "béll"],
			"housing_tax": {
				"2017": null
			},
			"accomodation_size": {
				"2017-01": 300
			},
			"housing_occupancy_status": {
				"2017-01": "Tenant"
			}
		}
	}
}

resulted in :

  "error": "Internal server error: 'ascii' codec can't encode character u'\\xe9' in position 1: ordinal not in range(128)"

also, accents in place of periods make the server angry:

{
	"persons": {
		"bill": {
			"salary": {
				"2017": 60000
			}
		},
		"bell": {
			"salary": {
				"2017": 60000
			}
		}
	},
	"households": {
		"_": {
			"parents": ["bill", "bell"],
			"housing_tax": {
				"à": null
			},
			"accomodation_size": {
				"2017-01": 300
			},
			"housing_occupancy_status": {
				"2017-01": "Tenant"
			}
		}
	}
}

results in:

  "error": "Internal server error: 'ascii' codec can't encode character u'\\xe0' in position 0: ordinal not in range(128)"

Anna-Livia · 2017-11-17T18:34:29Z

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}}')


When tested with curl, got :

For instance: {\"salary\": {\"2017-01\": 2000, \"2017-02\": 2500}}"

I was expecting :

For instance: {"salary": {"2017-01": 2000, "2017-02": 2500}}"

For instance: {\"salary\": {\"2017-01\": 2000, \"2017-02\": 2500}} is correct as it is included in a JSON response like:

{ "persons": { "bill": { "salary": "Invalid type: must be of type object. Input variables must be set for specific periods. For instance: {\"salary\": {\"2017-01\": 2000, \"2017-02\": 2500}}" } } }

Anna-Livia · 2017-11-17T18:36:32Z

openfisca_core/entities.py

-                                "'{}' is not a valid value for '{}'. Possible values are ['{}'].".format(
-                                    value, variable_name, "', '".join(holder.variable.possible_values.list)
-                                    ).encode('utf-8')
+                                u"'{}' is not a valid value for '{}'. Possible values are ['{}'].".format(


Works great when Value has accents. Woudl it be possible to add a accentfull enum to country template to test if possible values works also ?

Anna-Livia · 2017-11-17T18:39:55Z

openfisca_core/entities.py

                                )
                    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))


I don't see how to test for this 🤔

Anna-Livia · 2017-11-17T18:40:17Z

openfisca_core/entities.py

@@ -115,7 +114,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),


I don't see how to test for this 🤔

Anna-Livia · 2017-11-17T18:42:48Z

openfisca_core/entities.py

@@ -145,7 +144,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))


I don't see how to test for this 🤔

Anna-Livia · 2017-11-17T18:46:06Z

openfisca_core/entities.py

@@ -319,12 +318,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(


Tested, works well :)

Anna-Livia · 2017-11-17T18:46:37Z

openfisca_core/entities.py

@@ -305,7 +304,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(


Works well :)

Anna-Livia · 2017-11-17T19:01:52Z

openfisca_core/simulations.py

@@ -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]))


sandcha · 2017-11-20T18:25:59Z

B/ On enums with accents:
Before merging enums pr #589, enums with accents work. This is conditioned to the use of unicode on key names. Str keys with accent raise KeyError on enumerations.py:16. But this file disappears in #589.

sandcha · 2017-11-22T17:16:14Z

Definition of done:

When a non ASCII encoding is sent to the API, print a 500 error message with the incriminated character.
Add unit tests on encoding in API requests

sandcha · 2018-01-17T07:17:35Z

Before, on UnicodeDecodeError :

Error 500

{
  "error": "Internal server error: 'ascii' codec can't encode character u'\\u2018' in position 1: ordinal not in range(128)"
}

After:

Error 400 (500 if something was missed):

{
  "error": "'O‘Ryan' is not a valid ASCII value."
}

Adapt test to enums of core v21.0.2

sandcha requested a review from Anna-Livia November 16, 2017 17:00

Anna-Livia requested changes Nov 17, 2017

View reviewed changes

bonjourmauko assigned sandcha Jan 11, 2018

Anna-Livia approved these changes Jan 16, 2018

View reviewed changes

sandcha added 10 commits January 17, 2018 18:03

Encode to utf-8 API json responses

41052ee

Display UnicodeEncodeError on client side

efca2d3

Set exceptions to unicode

c33c444

Identify non ascii string on UnicodeEncodeError

0b1e911

Add encoding tests

7521bab

Check calculate response encoding

ae1e196

Move json encoding test to test_calculate

2a99c3c

Add tests on data encoding in api

5d973ed

Move UnicodeEncodeError from internal server error to bad request

96d7018

Adapt test to enums of core v21.0.2

Bump version number to 21.0.3

eeaf219

sandcha force-pushed the msg-encoding-587 branch from ef23702 to eeaf219 Compare January 17, 2018 18:14

sandcha merged commit 8e784a6 into master Jan 17, 2018

sandcha deleted the msg-encoding-587 branch January 17, 2018 18:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sure all errors messages are correctly encoded #595

Make sure all errors messages are correctly encoded #595

sandcha commented Nov 16, 2017 •

edited

Loading

Anna-Livia left a comment

Anna-Livia Nov 17, 2017

sandcha Nov 21, 2017

Anna-Livia Nov 17, 2017

Anna-Livia Nov 17, 2017

Anna-Livia Nov 17, 2017

Anna-Livia Nov 17, 2017

Anna-Livia Nov 17, 2017

Anna-Livia Nov 17, 2017

Anna-Livia Nov 17, 2017

sandcha commented Nov 20, 2017 •

edited

Loading

sandcha commented Nov 22, 2017 •

edited

Loading

sandcha commented Jan 17, 2018 •

edited

Loading

Make sure all errors messages are correctly encoded #595

Make sure all errors messages are correctly encoded #595

Conversation

sandcha commented Nov 16, 2017 • edited Loading

Bug fix

Anna-Livia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandcha commented Nov 20, 2017 • edited Loading

sandcha commented Nov 22, 2017 • edited Loading

sandcha commented Jan 17, 2018 • edited Loading

sandcha commented Nov 16, 2017 •

edited

Loading

sandcha commented Nov 20, 2017 •

edited

Loading

sandcha commented Nov 22, 2017 •

edited

Loading

sandcha commented Jan 17, 2018 •

edited

Loading