10000 ✨ Add context manager for handling tools errors (#18) · ARMmbed/mbed-tools-lib@16de72b · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Jul 27, 2020. It is now read-only.

Commit 16de72b

Browse files
madchutneymadchutney
authored
✨ Add context manager for handling tools errors (#18)
* Fix azure badge - previously showed failure as check "CI Checkpoint" was not being used. * Upload coverage for Python 3.6 to improve coverage percentage. * Create a context manager to suppress ToolErrors and improve UX: * This will show a message on how to get more information by increasing the verbosity. * This can be also be used by CI scripts rather than just the CLI.
1 parent cf2cbce commit 16de72b

File tree

5 files changed

+174
-3
lines changed

5 files changed

+174
-3
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
[![PyPI - Python Version](https://img.shields.io/pypi/pyversions/mbed-tools-lib)](https://pypi.org/project/mbed-tools-lib/)
88
[![License](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](https://github.com/ARMmbed/mbed-tools-lib/blob/master/LICENSE)
99

10-
[![Build Status](https://dev.azure.com/mbed-tools/mbed-tools-lib/_apis/build/status/ARMmbed.mbed-tools-lib?branchName=master)](https://dev.azure.com/mbed-tools/mbed-tools-lib/_build/latest?definitionId=1&branchName=master)
10+
[![Build Status](https://dev.azure.com/mbed-tools/mbed-tools-lib/_apis/build/status/ARMmbed.mbed-tools-lib?branchName=master&stageName=CI%20Checkpoint)](https://dev.azure.com/mbed-tools/mbed-tools-lib/_build/latest?definitionId=1&branchName=master)
1111
[![Test Coverage](https://codecov.io/gh/ARMmbed/mbed-tools-lib/branch/master/graph/badge.svg)](https://codecov.io/gh/ARMmbed/mbed-tools-lib)
1212
[![Maintainability](https://api.codeclimate.com/v1/badges/18c13e9ee7ba963c81e9/maintainability)](https://codeclimate.com/github/ARMmbed/mbed-tools-lib/maintainability)
1313

‎azure-pipelines/build-release.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ stages:
2929
Linux_Py_3_6:
3030
python.version: '3.6'
3131
vmImageName: ubuntu-latest
32-
uploadCoverage: "false"
32+
uploadCoverage: "true"
3333

3434
Linux_Py_3_7:
3535
python.version: '3.7'

‎mbed_tools_lib/logging.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,59 @@
33
# SPDX-License-Identifier: Apache-2.0
44
#
55
"""Helpers for logging errors according to severity of the exception."""
6+
from typing import Type, Optional, cast
7+
from types import TracebackType
68
import logging
9+
from mbed_tools_lib.exceptions import ToolsError
710

811
LOGGING_FORMAT = "%(levelname)s: %(message)s"
912

13+
VERBOSITY_HELP = {
14+
logging.CRITICAL: "-v",
15+
logging.ERROR: "-v",
16+
logging.WARNING: "-vv",
17+
logging.INFO: "-vvv",
18+
logging.DEBUG: "--traceback",
19+
}
20+
21+
22+
def _exception_message(err: BaseException, log_level: int, traceback: bool) -> str:
23+
"""Generate a user facing message with help on how to get more information from the logs."""
24+
error_msg = str(err)
25+
if log_level != logging.DEBUG or not traceback:
26+
cli_option = VERBOSITY_HELP.get(log_level, "-v")
27+
error_msg += f"\n\nMore information may be available by using the command line option '{cli_option}'."
28+
return error_msg
29+
30+
31+
class MbedToolsHandler:
32+
"""Context Manager to catch Mbed Tools exceptions and generate a helpful user facing message."""
33+
34+
def __init__(self, logger: logging.Logger, traceback: bool = False):
35+
"""Keep track of the logger to use and whether or not a traceback should be generated."""
36+
self._logger = logger
37+
self._traceback = traceback
38+
39+
def __enter__(self) -> "MbedToolsHandler":
40+
"""Return the Context Manager."""
41+
return self
42+
43+
def __exit__(
44+
self,
45+
exc_type: Optional[Type[BaseException]],
46+
exc_value: Optional[BaseException],
47+
exc_traceback: Optional[TracebackType],
48+
) -> bool:
49+
"""Handle any raised exceptions, suppressing Tools errors and generating an error message instead."""
50+
if exc_type and issubclass(exc_type, ToolsError):
51+
error_msg = _exception_message(cast(BaseException, exc_value), logging.root.level, self._traceback)
52+
self._logger.error(error_msg, exc_info=self._traceback)
53+
# Do not propagate exceptions derived from ToolsError
54+
return True
55+
56+
# Propagate all other exceptions
57+
return False
58+
1059

1160
def log_exception(logger: logging.Logger, exception: Exception, show_traceback: bool = False) -> None:
1261
"""Logs an exception in both normal and verbose forms.

‎news/20200403.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add context manager for handling tools errors and generate a user friendly message about increasing verbosity

‎tests/test_logging.py

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,128 @@
55
import logging
66
from unittest import TestCase, mock
77

8-
from mbed_tools_lib.logging import log_exception, set_log_level, LOGGING_FORMAT
8+
from mbed_tools_lib.exceptions import ToolsError
9+
from mbed_tools_lib.logging import _exception_message, MbedToolsHandler, log_exception, set_log_level, LOGGING_FORMAT
10+
11+
12+
class SubclassedToolsError(ToolsError):
13+
"""An exception subclassing ToolsError."""
14+
15+
16+
class TestExceptionMessage(TestCase):
17+
def test_critical_log_level_with_traceback(self):
18+
message = _exception_message(ToolsError("unlikely string"), logging.CRITICAL, True)
19+
self.assertTrue("'-v'" in message)
20+
self.assertTrue("'--traceback'" not in message)
21+
self.assertTrue("unlikely string" in message)
22+
23+
def test_critical_log_level_without_traceback(self):
24+
message = _exception_message(ToolsError("unlikely string"), logging.CRITICAL, False)
25+
self.assertTrue("'-v'" in message)
26+
self.assertTrue("'--traceback'" not in message)
27+
self.assertTrue("unlikely string" in message)
28+
29+
def test_error_log_level_with_traceback(self):
30+
message = _exception_message(ToolsError("unlikely string"), logging.ERROR, True)
31+
self.assertTrue("'-v'" in message)
32+
self.assertTrue("'--traceback'" not in message)
33+
self.assertTrue("unlikely string" in message)
34+
35+
def test_error_log_level_without_traceback(self):
36+
message = _exception_message(ToolsError("unlikely string"), logging.ERROR, False)
37+
self.assertTrue("'-v'" in message)
38+
self.assertTrue("'--traceback'" not in message)
39+
self.assertTrue("unlikely string" in message)
40+
41+
def test_warning_log_level_with_traceback(self):
42+
message = _exception_message(ToolsError("unlikely string"), logging.WARNING, True)
43+
self.assertTrue("'-vv'" in message)
44+
self.assertTrue("'--traceback'" not in message)
45+
self.assertTrue("unlikely string" in message)
46+
47+
def test_warning_log_level_without_traceback(self):
48+
message = _exception_message(ToolsError("unlikely string"), logging.WARNING, False)
49+
self.assertTrue("'-vv'" in message)
50+
self.assertTrue("'--traceback'" not in message)
51+
self.assertTrue("unlikely string" in message)
52+
53+
def test_info_log_level_with_traceback(self):
54+
message = _exception_message(ToolsError("unlikely string"), logging.INFO, True)
55+
self.assertTrue("'-vvv'" in message)
56+
self.assertTrue("'--traceback'" not in message)
57+
self.assertTrue("unlikely string" in message)
58+
59+
def test_info_log_level_without_traceback(self):
60+
message = _exception_message(ToolsError("unlikely string"), logging.INFO, False)
61+
self.assertTrue("'-vvv'" in message)
62+
self.assertTrue("'--traceback'" not in message)
63+
self.assertTrue("unlikely string" in message)
64+
65+
def test_debug_log_level_with_traceback(self):
66+
message = _exception_message(ToolsError("unlikely string"), logging.DEBUG, True)
67+
self.assertTrue("-v" not in message)
68+
self.assertTrue("'--traceback'" not in message)
69+
self.assertTrue("unlikely string" in message)
70+
71+
def test_debug_log_level_without_traceback(self):
72+
message = _exception_message(ToolsError("unlikely string"), logging.DEBUG, False)
73+
self.assertTrue("-v" not in message)
74+
self.assertTrue("'--traceback'" in message)
75+
self.assertTrue("unlikely string" in message)
76+
77+
def test_log_level_not_set_with_traceback(self):
78+
message = _exception_message(ToolsError("unlikely string"), logging.NOTSET, True)
79+
self.assertTrue("'-v'" in message)
80+
self.assertTrue("'--traceback'" not in message)
81+
self.assertTrue("unlikely string" in message)
82+
83+
def test_log_level_not_set_without_traceback(self):
84+
message = _exception_message(ToolsError("unlikely string"), logging.NOTSET, False)
85+
self.assertTrue("'-v'" in message)
86+
self.assertTrue("'--traceback'" not in message)
87+
self.assertTrue("unlikely string" in message)
88+
89+
90+
class TestMbedToolsHandler(TestCase):
91+
exception_string: str = "A Message"
92+
expected_log_message: str = "A Message\n\nMore information may be available by using the command line option '-vv'."
93+
94+
def test_no_exception_raised(self):
95+
mock_logger = mock.Mock(spec_set=logging.Logger)
96+
with MbedToolsHandler(mock_logger, traceback=False):
97+
pass
98+
self.assertFalse(mock_logger.error.called, "Error should not be logger when an exception is not raised.")
99+
100+
def test_tools_error_with_traceback(self):
101+
mock_logger = mock.Mock(spec_set=logging.Logger)
102+
with MbedToolsHandler(mock_logger, traceback=True):
103+
raise ToolsError(self.exception_string)
104+
mock_logger.error.assert_called_once_with(self.expected_log_message, exc_info=True)
105+
106+
def test_tools_error_without_traceback(self):
107+
mock_logger = mock.Mock(spec_set=logging.Logger)
108+
with MbedToolsHandler(mock_logger, traceback=False):
109+
raise ToolsError(self.exception_string)
110+
mock_logger.error.assert_called_once_with(self.expected_log_message, exc_info=False)
111+
112+
def test_subclassed_tools_error_with_traceback(self):
113+
mock_logger = mock.Mock(spec_set=logging.Logger)
114+
with MbedToolsHandler(mock_logger, traceback=True):
115+
raise SubclassedToolsError(self.exception_string)
116+
mock_logger.error.assert_called_once_with(self.expected_log_message, exc_info=True)
117+
118+
def test_subclassed_tools_error_without_traceback(self):
119+
mock_logger = mock.Mock(spec_set=logging.Logger)
120+
with MbedToolsHandler(mock_logger, traceback=False):
121+
raise SubclassedToolsError(self.exception_string)
122+
mock_logger.error.assert_called_once_with(self.expected_log_message, exc_info=False)
123+
124+
def test_other_exceptions(self):
125+
mock_logger = mock.Mock(spec_set=logging.Logger)
126+
with self.assertRaises(ValueError):
127+
with MbedToolsHandler(mock_logger, traceback=False):
128+
raise ValueError(self.exception_string)
129+
self.assertFalse(mock_logger.error.called, "Error should not be logger when a tools error is not raised.")
9130

10131

11132
class TestLogException(TestCase):

0 commit comments

Comments
 (0)
0