Skip to content

Commit

Permalink
Don't configure logging for trait notification handler exceptions (#1161
Browse files Browse the repository at this point in the history
)

* Don't configure logging

* Add changelog entry

* Add test to check that logs are created

* Add note about log configuration to the 'Notes on upgrading' section

* Wording tweak

* Update changelog wording

* More wording tweaks

* Fix another typo
  • Loading branch information
mdickinson authored Jun 1, 2020
1 parent 7e59d63 commit e60feb2
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 16 deletions.
19 changes: 19 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,22 @@ However, there are a few things to be aware of when upgrading.
TraitsUI views, and an error will be raised if you attempt to create a
TraitsUI view.

* Traits now does no logging configuration at all, leaving all such
configuration to the application.

In more detail: trait notification handlers should not raise exceptions in
normal use, so an exception is logged whenever a trait notification handler
raises. This part of the behaviour has not changed. What *has* changed is the
way that logged exception is handled under default exception handling.

Previously, Traits added a :class:`~logging.StreamHandler` to the
top-level ``"traits"`` logger, so that trait notification exceptions would
always be visible. Traits also added a :class:`~logging.NullHandler` to that
logger. Both of those handlers have now been removed. We now rely on
Python's "handler of last resort", which will continue to make notification
exceptions to the user visible in the absence of any application-level
log configuration.

* When listening for changes to the items of a :class:`.List` trait, an index
or slice set operation no longer performs an equality check between the
replaced elements and the replacement elements when deciding whether to issue
Expand Down Expand Up @@ -169,6 +185,9 @@ Changes
result in content that compares equally to the old values. (#1026)
* ``TraitListEvent.index`` reported by mutations to a list is now normalized.
(#1009)
* The default notification error handler for Traits no longer configures
logging, and the top-level ``NullHandler`` log handler has been removed.
(#1161)

Fixes
~~~~~
Expand Down
7 changes: 0 additions & 7 deletions traits/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,3 @@
# hasn't been built, so this isn't a viable Traits installation. OTOH, it
# can be useful if a simple "import traits" doesn't actually fail.
__version__ = "unknown"

# Add a NullHandler so 'traits' loggers don't complain when they get used.
import logging

logging.getLogger(__name__).addHandler(logging.NullHandler())

del logging
23 changes: 23 additions & 0 deletions traits/tests/test_listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import contextlib
import io
import logging
import sys
import threading
import time
Expand Down Expand Up @@ -152,6 +153,28 @@ def _handle_exception(obj, name, old, new):
finally:
trait_notifiers.pop_exception_handler()

def test_exceptions_logged(self):
# Check that default exception handling logs the exception.
ge = GenerateFailingEvents()

traits_logger = logging.getLogger("traits")

with self.assertLogs(
logger=traits_logger, level=logging.ERROR) as log_watcher:
ge.name = "Terry Jones"

self.assertEqual(len(log_watcher.records), 1)
log_record = log_watcher.records[0]

self.assertIn(
"Exception occurred in traits notification handler",
log_record.message,
)

_, exc_value, exc_traceback = log_record.exc_info
self.assertIsInstance(exc_value, RuntimeError)
self.assertIsNotNone(exc_traceback)


class A(HasTraits):
exception = Any
Expand Down
10 changes: 1 addition & 9 deletions traits/trait_notifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"""

import contextlib
import logging
import threading
from threading import local as thread_local
from threading import Thread
Expand Down Expand Up @@ -211,16 +212,7 @@ def _log_exception(self, object, trait_name, old, new):

logger = self.traits_logger
if logger is None:
import logging

self.traits_logger = logger = logging.getLogger("traits")
handler = logging.StreamHandler()
handler.setFormatter(logging.Formatter("%(message)s"))
logger.addHandler(handler)
print(
"Exception occurred in traits notification handler.\n"
"Please check the log file for details."
)

try:
logger.exception(
Expand Down

0 comments on commit e60feb2

Please sign in to comment.