From 23b1cecb135c7d02272d0881b7e340d19bbf089c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Zaoral?= Date: Fri, 12 Jan 2024 10:17:17 +0100 Subject: [PATCH 1/3] tback: make Traceback printing compatible with Python 3 Because kobo only supports Python 3.6+, we can just return a concatenated unicode string with the given exception. Fixes the following incorrectly formatted output: ``` $ cat test.py from kobo.tback import set_except_hook set_except_hook() assert False $ python3 test.py b'Traceback (most recent call last):\n File "/Users/lzaoral/redhat/OpenScanHub/kobo/test.py", line 4, in \n assert False\nAssertionError\nFrame in /Users/lzaoral/redhat/OpenScanHub/kobo/test.py at line 4\n\n 1 from kobo.tback import set_except_hook\n 2 set_except_hook()\n 3 \n--> 4 assert False\n\n\n __annotations__ = {}\n __builtins__ = \n __cached__ = None\n __doc__ = None\n __file__ = \'/Users/lzaoral/redhat/OpenScanHub/kobo/test.py\'\n __loader__ = <_frozen_importlib_external.SourceFileLoader object at 0x100711650>\n __name__ = \'__main__\'\n __package__ = None\n __spec__ = None\n set_except_hook = \n\n' ``` --- kobo/decorators.py | 6 +++--- kobo/tback.py | 12 +----------- kobo/worker/logger.py | 2 +- kobo/worker/taskmanager.py | 2 +- tests/test_tback.py | 22 ++++++++++------------ 5 files changed, 16 insertions(+), 28 deletions(-) diff --git a/kobo/decorators.py b/kobo/decorators.py index 5b7a3c60..ef3e6b8f 100644 --- a/kobo/decorators.py +++ b/kobo/decorators.py @@ -62,10 +62,10 @@ def new_func(*args, **kwargs): import datetime import kobo.shortcuts import kobo.tback - date = datetime.datetime.strftime(datetime.datetime.now(), "%F %R:%S").encode() - data = b"--- TRACEBACK BEGIN: " + date + b" ---\n" + date = datetime.datetime.strftime(datetime.datetime.now(), "%F %R:%S") + data = "--- TRACEBACK BEGIN: " + date + " ---\n" data += kobo.tback.Traceback().get_traceback() - data += b"--- TRACEBACK END: " + date + b" ---\n\n\n" + data += "--- TRACEBACK END: " + date + " ---\n\n\n" kobo.shortcuts.save_to_file(log_file, data, append=True) raise return new_func diff --git a/kobo/tback.py b/kobo/tback.py index a4aef194..c42face1 100644 --- a/kobo/tback.py +++ b/kobo/tback.py @@ -169,18 +169,8 @@ def get_traceback(self): obj_value_str = self._to_str(obj_value, "%.200r") result.append("%20s = %s" % ("self." + self._to_str(obj_key), obj_value_str)) result.append("") - s = u'' - for i in result: - line = i.replace(r'\\n', '\n').replace(r'\n', '\n') - - if type(i) == six.text_type: - s += i - else: - s += six.text_type(str(i), errors='replace') - s += '\n' - - return s.encode('ascii', 'replace') + return '\n'.join(result) def print_traceback(self): """Print a traceback string to stderr.""" diff --git a/kobo/worker/logger.py b/kobo/worker/logger.py index 138ec4ef..ce295a9e 100644 --- a/kobo/worker/logger.py +++ b/kobo/worker/logger.py @@ -77,7 +77,7 @@ def run(self): if self._logger: msg = "\n".join([ "Fatal error in LoggingThread", - kobo.tback.Traceback().get_traceback().decode(), + kobo.tback.Traceback().get_traceback(), ]) self._logger.log_critical(msg) raise diff --git a/kobo/worker/taskmanager.py b/kobo/worker/taskmanager.py index 5fce5e2d..95f3b607 100644 --- a/kobo/worker/taskmanager.py +++ b/kobo/worker/taskmanager.py @@ -445,7 +445,7 @@ def run_task(self, task_info): message = "ERROR: %s\n" % kobo.tback.get_exception() message += "See traceback.log for details (admin only).\n" hub.upload_task_log(BytesIO(message.encode()), task.task_id, "error.log") - hub.upload_task_log(BytesIO(kobo.tback.Traceback().get_traceback()), task.task_id, "traceback.log", mode=0o600) + hub.upload_task_log(BytesIO(kobo.tback.Traceback().get_traceback().encode()), task.task_id, "traceback.log", mode=0o600) failed = True finally: thread.stop() diff --git a/tests/test_tback.py b/tests/test_tback.py index eb3a1caa..5c0deec4 100755 --- a/tests/test_tback.py +++ b/tests/test_tback.py @@ -3,7 +3,6 @@ import re -import six import unittest from kobo.tback import get_traceback, Traceback @@ -20,40 +19,39 @@ def test_text(self): try: raise Exception('Simple text') except: - str_regexp = re.compile(r'Traceback \(most recent call last\):\n *File ".*test_tback.py", line .+, in test_text\n *raise Exception\(\'Simple text\'\)\n *Exception: Simple text', re.M) - bytes_regexp = re.compile(rb'Traceback \(most recent call last\):\n *File ".*test_tback.py", line .+, in test_text\n *raise Exception\(\'Simple text\'\)\n *Exception: Simple text', re.M) + regexp = re.compile(r'Traceback \(most recent call last\):\n *File ".*test_tback.py", line .+, in test_text\n *raise Exception\(\'Simple text\'\)\n *Exception: Simple text', re.M) - self.assertRegex(get_traceback(), str_regexp) + self.assertRegex(get_traceback(), regexp) tb = Traceback(show_traceback = True, show_code = False, show_locals = False, show_environ = False, show_modules = False) - self.assertRegex(tb.get_traceback(), bytes_regexp) + self.assertRegex(tb.get_traceback(), regexp) def test_Traceback(self): try: raise Exception('Simple text') except: tb = Traceback(show_traceback = False, show_code = False, show_locals = False, show_environ = False, show_modules = False) - self.assertEqual(b'', tb.get_traceback()) + self.assertEqual('', tb.get_traceback()) tb.show_code = True - self.assertRegex(tb.get_traceback(), re.compile(rb'.*--> *\d+ *raise Exception.*<\/CODE>$', re.M | re.S)) + self.assertRegex(tb.get_traceback(), re.compile(r'.*--> *\d+ *raise Exception.*<\/CODE>$', re.M | re.S)) tb.show_code = False tb.show_locals = True - self.assertRegex(tb.get_traceback(), re.compile(rb'.*tb = .*<\/LOCALS>$', re.M | re.S)) + self.assertRegex(tb.get_traceback(), re.compile(r'.*tb = .*<\/LOCALS>$', re.M | re.S)) tb.show_locals = False tb.show_environ = True - self.assertRegex(tb.get_traceback(), re.compile(rb'.*<\/ENVIRON>\n.*$', re.M | re.S)) + self.assertRegex(tb.get_traceback(), re.compile(r'.*<\/ENVIRON>\n.*$', re.M | re.S)) tb.show_environ = False tb.show_modules = True - self.assertRegex(tb.get_traceback(), re.compile(rb'.*<\/MODULES>$', re.M | re.S)) + self.assertRegex(tb.get_traceback(), re.compile(r'.*<\/MODULES>$', re.M | re.S)) def test_encoding(self): try: a = ''.join([chr(i) for i in range(256)]) - b = u''.join([unichr(i) for i in range(65536)]) + b = b''.join([chr(i).encode() for i in range(65536)]) raise Exception() except: tb = Traceback(show_code = False, show_traceback = False) output = tb.get_traceback() - self.assertIsInstance(output, six.binary_type) + self.assertIsInstance(output, str) def test_uninitialized_variables(self): class Foo(object): From c2577ce795e1fb3f8b875955d0c05b852b9be999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Zaoral?= Date: Fri, 12 Jan 2024 10:20:32 +0100 Subject: [PATCH 2/3] tback: remove extra newline printed by the except hook --- kobo/tback.py | 1 - 1 file changed, 1 deletion(-) diff --git a/kobo/tback.py b/kobo/tback.py index c42face1..eae5ec55 100644 --- a/kobo/tback.py +++ b/kobo/tback.py @@ -260,7 +260,6 @@ def _hook(exctype, value, tb): tback.show_locals = True logger and logger.error(tback.get_traceback()) tback.print_traceback() - print() _hook.__doc__ = sys.excepthook.__doc__ _hook.__name__ = sys.excepthook.__name__ From 461bc2a247c437adef21d89eadd170977c2a1cb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Zaoral?= Date: Fri, 12 Jan 2024 10:35:53 +0100 Subject: [PATCH 3/3] client: simplify formatting ot XML-RPC faults `Traceback.get_traceback` no longer produces broken output. Therefore, our reimplementation of `xmlrpc.client.Fault.__repr__` can be greatly simplified. --- kobo/client/__init__.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/kobo/client/__init__.py b/kobo/client/__init__.py index 9b47b4f1..c929e5fb 100644 --- a/kobo/client/__init__.py +++ b/kobo/client/__init__.py @@ -510,21 +510,13 @@ def upload_task_log(self, file_obj, task_id, remote_file_name, append=True, mode self._hub.worker.upload_task_log(task_id, remote_file_name, mode, chunk_start, chunk_len, chunk_checksum, encoded_chunk) -from six.moves.xmlrpc_client import Fault -import sys - +from xmlrpc.client import Fault # default implementation of Fault.__repr__ is: # "" % (self.faultCode, repr(self.faultString)) -# repr of string does not escape newlines ('\n') and produces very ugly output -# so using direct string is much nicer for users +# repr of string escapes everything (e.g. newlines) and produces a very ugly +# output so using it directly is much nicer for users def fault_repr(self): - fault = self.faultString - if sys.version_info[0] > 2 and fault.startswith("b\'Traceback"): - # properly deserialize tracebacks - import ast - fault = ast.literal_eval(fault).decode("unicode-escape") - - return "" % (self.faultCode, str(fault)) + return "" % (self.faultCode, self.faultString) Fault.__repr__ = fault_repr