Commit 58bb3b81e76cfa6f46eaa57e686a7bac625c0f48
1 parent
7f315674
log_helper: Test and improve error-handling
If there is an error in log-handling (e.g. wrong args to logger call) do not fail or raise but produce something helpful
Showing
3 changed files
with
28 additions
and
5 deletions
oletools/common/log_helper/_json_formatter.py
| @@ -32,15 +32,25 @@ class JsonFormatter(logging.Formatter): | @@ -32,15 +32,25 @@ class JsonFormatter(logging.Formatter): | ||
| 32 | :py:class:`oletools.common.log_helper.OletoolsLoggerAdapter` or added here | 32 | :py:class:`oletools.common.log_helper.OletoolsLoggerAdapter` or added here |
| 33 | (for input from e.g. captured warnings, third-party libraries) | 33 | (for input from e.g. captured warnings, third-party libraries) |
| 34 | """ | 34 | """ |
| 35 | - msg = self.msg_formatter.format(record) | ||
| 36 | - json_dict = dict(msg=msg.replace('\n', ' '), level=record.levelname) | 35 | + json_dict = dict(msg='', level='', type='') |
| 37 | try: | 36 | try: |
| 37 | + msg = self.msg_formatter.format(record) | ||
| 38 | + json_dict['msg'] = msg.replace('\n', ' ') | ||
| 39 | + json_dict['level'] = record.levelname | ||
| 38 | json_dict['type'] = record.type | 40 | json_dict['type'] = record.type |
| 39 | - except AttributeError: | ||
| 40 | - if record.name == 'py.warnings': # this is the name of the logger | 41 | + except AttributeError: # most probably: record has no "type" field |
| 42 | + if record.name == 'py.warnings': # this is from python's warning-capture logger | ||
| 41 | json_dict['type'] = 'warning' | 43 | json_dict['type'] = 'warning' |
| 42 | else: | 44 | else: |
| 43 | - json_dict['type'] = 'msg' | 45 | + json_dict['type'] = 'msg' # message of unknown origin |
| 46 | + except Exception as exc: | ||
| 47 | + try: | ||
| 48 | + json_dict['msg'] = "Ignore {0} when formatting '{1}': {2}".format(type(exc), record.msg, exc) | ||
| 49 | + except Exception as exc2: | ||
| 50 | + json_dict['msg'] = 'Caught {0} in logging'.format(str(exc2)) | ||
| 51 | + json_dict['type'] = 'log-warning' | ||
| 52 | + json_dict['level'] = 'warning' | ||
| 53 | + | ||
| 44 | formatted_message = ' ' + json.dumps(json_dict) | 54 | formatted_message = ' ' + json.dumps(json_dict) |
| 45 | 55 | ||
| 46 | if self._is_first_line: | 56 | if self._is_first_line: |
tests/common/log_helper/log_helper_test_main.py
| @@ -50,6 +50,7 @@ def main(args): | @@ -50,6 +50,7 @@ def main(args): | ||
| 50 | percent_autoformat = '%-autoformat' in args | 50 | percent_autoformat = '%-autoformat' in args |
| 51 | warn = 'warn' in args | 51 | warn = 'warn' in args |
| 52 | exc_info = 'exc-info' in args | 52 | exc_info = 'exc-info' in args |
| 53 | + wrong_log_args = 'wrong-log-args' in args | ||
| 53 | 54 | ||
| 54 | log_helper_test_imported.logger.setLevel(logging.ERROR) | 55 | log_helper_test_imported.logger.setLevel(logging.ERROR) |
| 55 | 56 | ||
| @@ -71,6 +72,12 @@ def main(args): | @@ -71,6 +72,12 @@ def main(args): | ||
| 71 | except Exception: | 72 | except Exception: |
| 72 | logger.exception('Caught exception') # has exc_info=True | 73 | logger.exception('Caught exception') # has exc_info=True |
| 73 | 74 | ||
| 75 | + if wrong_log_args: | ||
| 76 | + logger.info('Opening file /dangerous/file/with-%s-in-name') | ||
| 77 | + logger.info('The result is %f') | ||
| 78 | + logger.info('No result', 1.23) | ||
| 79 | + logger.info('The result is %f', 'bla') | ||
| 80 | + | ||
| 74 | log_helper.end_logging() | 81 | log_helper.end_logging() |
| 75 | 82 | ||
| 76 | 83 |
tests/common/log_helper/test_log_helper.py
| @@ -224,6 +224,12 @@ class TestLogHelper(unittest.TestCase): | @@ -224,6 +224,12 @@ class TestLogHelper(unittest.TestCase): | ||
| 224 | self.assertIn('Traceback (most recent call last)', output) # start of trace | 224 | self.assertIn('Traceback (most recent call last)', output) # start of trace |
| 225 | self.assertIn(TEST_FILE, output) # part of trace | 225 | self.assertIn(TEST_FILE, output) # part of trace |
| 226 | 226 | ||
| 227 | + def test_json_wrong_args(self): | ||
| 228 | + """Test that too many or missing args do not raise exceptions inside logger""" | ||
| 229 | + output = self._run_test(['enable', 'as-json', 'wrong-log-args', 'info']) | ||
| 230 | + json.loads(output) # check that this does not raise, so json is valid | ||
| 231 | + # do not care about actual contents of output | ||
| 232 | + | ||
| 227 | def _assert_json_messages(self, output, messages): | 233 | def _assert_json_messages(self, output, messages): |
| 228 | try: | 234 | try: |
| 229 | json_data = json.loads(output) | 235 | json_data = json.loads(output) |