toconnell / kdm-manager

An interactive campaign manager for the game "Monster", by Kingdom Death. Development blog and release notes at https://blog.kdm-manager.com This project has no affiliation with Kingdom Death and is a totally independent, fan-maintained project.
http://kdm-manager.com
Other
26 stars 11 forks source link

/new/survivor endpoint throws errors when parent names contain upper ascii characters #481

Closed toconnell closed 6 years ago

toconnell commented 6 years ago

From the auto-alert emails:

Method: POST URL: http://api.thewatcher.io/new/survivor JSON: {u'father': u'5a68a7d98740d97398408040', u'sex': u'F', u'public': False, u'primary_donor_parent': u'father', u'mother': u'5a80bf108740d951e57efb22', u'settlement': u'5a31a4aa8740d96dac139971', u'email': u'manu161@hotmail.com'}

The full traceback is as follows:

Traceback (most recent call last):
File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1639, in full_dispatch_request
 rv = self.dispatch_request()
File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1625, in dispatch_request
 return self.view_functions[rule.endpoint](**req.view_args)
File "/home/toconnell/kdm-manager/v2/api/utils.py", line 505, in wrapped_function
 resp = make_response(f(*args, **kwargs))
File "/home/toconnell/kdm-manager/v2/api/api.py", line 251, in new_asset
 return request_broker.new_user_asset(asset_type)
File "/home/toconnell/kdm-manager/v2/api/request_broker.py", line 145, in new_user_asset
 S = survivors.Survivor()
File "/home/toconnell/kdm-manager/v2/api/models/survivors.py", line 145, in __init__
 Models.UserAsset.__init__(self, *args, **kwargs)
File "/home/toconnell/kdm-manager/v2/api/Models.py", line 557, in __init__
 self.new()
File "/home/toconnell/kdm-manager/v2/api/models/survivors.py", line 333, in new
 self.survivor_birth(attribs)
File "/home/toconnell/kdm-manager/v2/api/models/survivors.py", line 477, in survivor_birth
 process_one_parent(parent_type, attribs[parent_type], primary_donor_parent)
File "/home/toconnell/kdm-manager/v2/api/models/survivors.py", line 452, in process_one_parent
 self.log_event(action="inherit", key=p_log_str, value=attr, agent="automation", event_type="inherit")
File "/home/toconnell/kdm-manager/v2/api/Models.py", line 919, in log_event
 d['action']['repr'] = " ".join([action_word, "'%s'" % value, action_preposition, str(key)])
UnicodeEncodeError: 'ascii' codec can't encode character u'\xd1' in position 0: ordinal not in range(128)

The offending survivor's name appears to be Ñordel<br> (not sure why the HTML stripper isn't clobbering that line break: check that out too).

toconnell commented 6 years ago

I added HTML-stripping to the normalize() call:

diff --git a/v2/api/models/survivors.py b/v2/api/models/survivors.py
index 3314c8a..507531b 100644
--- a/v2/api/models/survivors.py
+++ b/v2/api/models/survivors.py
@@ -538,6 +538,15 @@ class Survivor(Models.UserAsset):
             self.convert_weapon_proficiency_type()
             self.perform_save = True

+
+        #
+        #   user asset normalization
+        #
+
+        if self.survivor['name'] != utils.html_stripper(self.survivor['name']):
+            self.survivor['name'] = utils.html_stripper(self.survivor['name'])
+            self.perform_Save = True
+

It's worth the performance hit, I think.

toconnell commented 6 years ago

So, what's really happening here is that we're trying to compose a UTF-8 string for the Event Log and we've got these upper ASCII elements and the whole thing is bombing out.

The good news is that the Survivor still gets made, but the bad news is that the API call comes back as a 500, and the user doesn't SEE the survivor in the webapp.

I think the fix is going to be to bite the bullet and wrap semantic logging in a wrapper that lets it fail gracefully (which is going to suck, but I don't want to rack up a bunch of debt here playing with string normalization, encoding, etc.).

toconnell commented 6 years ago

Alright, _logevent() is wrapped, and errors dump to error.log after an alert email is sent (quietly) in the background. This will go out in the Timeline release.