From 87193b4a510c4f8f4c11012dea74c4b294506a23 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 1 Feb 2022 15:43:42 +0000 Subject: [PATCH 1/8] eddn: Be more paranoid about system name augmentation --- plugins/eddn.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/eddn.py b/plugins/eddn.py index d58eac26..ee8a52dd 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -802,7 +802,12 @@ class EDDN: # If 'SystemName' or 'System' is there, it's directly from a journal event. # If they're not there *and* 'StarSystem' isn't either, then we add the latter. if 'SystemName' not in entry and 'System' not in entry and 'StarSystem' not in entry: - entry['StarSystem'] = system_name + if system_name is not None and system_name != '': + entry['StarSystem'] = system_name + + else: + # Bad assumptions if this is the case + logger.error(f'No system name in entry, and system_name was not set either! entry:\n{entry!r}\n') if 'SystemAddress' not in entry: if this.systemaddress is None: From 67019e91b258c0342573f77a23cfdeeeb81f885a Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 1 Feb 2022 16:11:35 +0000 Subject: [PATCH 2/8] eddn: Extra System (name) augmentation paranoia --- plugins/eddn.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/eddn.py b/plugins/eddn.py index ee8a52dd..5eefc46a 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -802,12 +802,13 @@ class EDDN: # If 'SystemName' or 'System' is there, it's directly from a journal event. # If they're not there *and* 'StarSystem' isn't either, then we add the latter. if 'SystemName' not in entry and 'System' not in entry and 'StarSystem' not in entry: - if system_name is not None and system_name != '': - entry['StarSystem'] = system_name + if system_name is None or not isinstance(system_name, str) or system_name == '': + # Bad assumptions if this is the case + logger.warning(f'No system name in entry, and system_name was not set either! entry:\n{entry!r}\n') + return "passed-in system_name is empty, can't add System" else: - # Bad assumptions if this is the case - logger.error(f'No system name in entry, and system_name was not set either! entry:\n{entry!r}\n') + entry['StarSystem'] = system_name if 'SystemAddress' not in entry: if this.systemaddress is None: From f144c199ae4e8f2088023b878860d1f6bc08b56c Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 1 Feb 2022 16:12:05 +0000 Subject: [PATCH 3/8] eddn: Be paranoid about Status.json BodyName value --- plugins/eddn.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/eddn.py b/plugins/eddn.py index 5eefc46a..95317c3c 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -1596,10 +1596,12 @@ def dashboard_entry(cmdr: str, is_beta: bool, entry: Dict[str, Any]) -> None: :param is_beta: Whether non-live game version was detected. :param entry: The latest Status.json data. """ + this.status_body_name = None if 'BodyName' in entry: - this.status_body_name = entry['BodyName'] + if not isinstance(entry['BodyName'], str): + logger.warning(f'BodyName was present but not a string! "{entry["BodyName"]}"') - else: - this.status_body_name = None + else: + this.status_body_name = entry['BodyName'] tracking_ui_update() From e2e34674c26dc6c0b88e997ac849f5b88d8df5d4 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 1 Feb 2022 16:22:12 +0000 Subject: [PATCH 4/8] eddn: codexentry - add paranoia post-processing check of values --- plugins/eddn.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/eddn.py b/plugins/eddn.py index 95317c3c..7cfda84b 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -951,6 +951,10 @@ class EDDN: entry['BodyID'] = this.body_id ####################################################################### + for k, v in entry.items(): + if v is None or isinstance(v, str) and v == '': + logger.warning(f'post-processing entry contains entry["{k}"] = {v}]') + msg = { '$schemaRef': f'https://eddn.edcd.io/schemas/codexentry/1{"/test" if is_beta else ""}', 'message': entry From 4fa64eec40b17a3e356ab091ee1ce40e6f04658f Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 1 Feb 2022 16:43:19 +0000 Subject: [PATCH 5/8] eddn: warnings improvements * Log the type of unexpected values. * Say 'falsey' not 'None' when we only `if not ...` --- plugins/eddn.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/plugins/eddn.py b/plugins/eddn.py index 7cfda84b..ea80404f 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -895,7 +895,7 @@ class EDDN: this.eddn.export_journal_entry(cmdr, entry, msg) return None - def export_journal_codexentry( + def export_journal_codexentry( # noqa: CCR001 self, cmdr: str, is_beta: bool, entry: MutableMapping[str, Any] ) -> Optional[str]: """ @@ -948,12 +948,16 @@ class EDDN: # Only set BodyID if journal BodyName matches the Status.json one. # This avoids binary body issues. if this.status_body_name == this.body_name: - entry['BodyID'] = this.body_id + if this.body_id is not None and isinstance(this.body_id, int): + entry['BodyID'] = this.body_id + + else: + logger.warning(f'this.body_id was not set properly: "{this.body_id}" ({type(this.body_id)})') ####################################################################### for k, v in entry.items(): if v is None or isinstance(v, str) and v == '': - logger.warning(f'post-processing entry contains entry["{k}"] = {v}]') + logger.warning(f'post-processing entry contains entry["{k}"] = {v} {(type(v))}') msg = { '$schemaRef': f'https://eddn.edcd.io/schemas/codexentry/1{"/test" if is_beta else ""}', @@ -1423,14 +1427,14 @@ def journal_entry( # noqa: C901, CCR001 # add mandatory StarSystem, StarPos and SystemAddress properties to Scan events if 'StarSystem' not in entry: if not system: - logger.warning("system is None, can't add StarSystem") + logger.warning("system is falsey, can't add StarSystem") return "system is None, can't add StarSystem" entry['StarSystem'] = system if 'StarPos' not in entry: if not this.coordinates: - logger.warning("this.coordinates is None, can't add StarPos") + logger.warning("this.coordinates is falsey, can't add StarPos") return "this.coordinates is None, can't add StarPos" # Gazelle[TD] reported seeing a lagged Scan event with incorrect @@ -1443,7 +1447,7 @@ def journal_entry( # noqa: C901, CCR001 if 'SystemAddress' not in entry: if not this.systemaddress: - logger.warning("this.systemaddress is None, can't add SystemAddress") + logger.warning("this.systemaddress is falsey, can't add SystemAddress") return "this.systemaddress is None, can't add SystemAddress" entry['SystemAddress'] = this.systemaddress @@ -1603,7 +1607,7 @@ def dashboard_entry(cmdr: str, is_beta: bool, entry: Dict[str, Any]) -> None: this.status_body_name = None if 'BodyName' in entry: if not isinstance(entry['BodyName'], str): - logger.warning(f'BodyName was present but not a string! "{entry["BodyName"]}"') + logger.warning(f'BodyName was present but not a string! "{entry["BodyName"]}" ({type(entry["BodyName"])})') else: this.status_body_name = entry['BodyName'] From 2bf70aae14bbea7ff2876ad6c474e9e2091f59fe Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 1 Feb 2022 16:59:44 +0000 Subject: [PATCH 6/8] eddn: Be paranoid about this.status_body_name type and value As it was we'd blindly set codexentry['BodyName'] from it if not None. --- plugins/eddn.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/eddn.py b/plugins/eddn.py index ea80404f..39c28eb5 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -943,7 +943,11 @@ class EDDN: entry = ret # Set BodyName if it's available from Status.json - if this.status_body_name is not None: + if this.status_body_name is None or not isinstance(this.status_body_name, str): + logger.warning(f'this.status_body_name was not set properly:' + f' "{this.status_body_name}" ({type(this.status_body_name)})') + + else: entry['BodyName'] = this.status_body_name # Only set BodyID if journal BodyName matches the Status.json one. # This avoids binary body issues. From 45b78f55c4e888fef1e4562f1880f3ce5cefdb74 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 1 Feb 2022 18:05:28 +0000 Subject: [PATCH 7/8] eddn: say 'is falsey' in error messages as well --- plugins/eddn.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/eddn.py b/plugins/eddn.py index 39c28eb5..5e4f5749 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -1432,14 +1432,14 @@ def journal_entry( # noqa: C901, CCR001 if 'StarSystem' not in entry: if not system: logger.warning("system is falsey, can't add StarSystem") - return "system is None, can't add StarSystem" + return "system is falsey, can't add StarSystem" entry['StarSystem'] = system if 'StarPos' not in entry: if not this.coordinates: logger.warning("this.coordinates is falsey, can't add StarPos") - return "this.coordinates is None, can't add StarPos" + return "this.coordinates is falsey, can't add StarPos" # Gazelle[TD] reported seeing a lagged Scan event with incorrect # augmented StarPos: @@ -1452,7 +1452,7 @@ def journal_entry( # noqa: C901, CCR001 if 'SystemAddress' not in entry: if not this.systemaddress: logger.warning("this.systemaddress is falsey, can't add SystemAddress") - return "this.systemaddress is None, can't add SystemAddress" + return "this.systemaddress is falsey, can't add SystemAddress" entry['SystemAddress'] = this.systemaddress From 1bb48ed5960aef0951df0291b0cfeea9f7cd519e Mon Sep 17 00:00:00 2001 From: Athanasius Date: Thu, 3 Feb 2022 13:59:52 +0000 Subject: [PATCH 8/8] eddn/codexentry: Bail and show error if empty string in message --- plugins/eddn.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/eddn.py b/plugins/eddn.py index 5e4f5749..afab0458 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -962,6 +962,10 @@ class EDDN: for k, v in entry.items(): if v is None or isinstance(v, str) and v == '': logger.warning(f'post-processing entry contains entry["{k}"] = {v} {(type(v))}') + # We should drop this message and VERY LOUDLY inform the + # user, in the hopes they'll open a bug report with the + # raw Journal event that caused this. + return 'CodexEntry had empty string, PLEASE ALERT THE EDMC DEVELOPERS' msg = { '$schemaRef': f'https://eddn.edcd.io/schemas/codexentry/1{"/test" if is_beta else ""}',