Fix bit-team#820 (Unhandled exception "TypeError: 'NoneType' object i…

…s not callable")

* Fix bug: Unhandled exception "TypeError: 'NoneType' object is not callable" in tools.py function __log_keyring_warning (bit-team#820).
   Logging thread removed and logger module correctly initialized as fix. Is "Heisenbug" so 100 % retesting was not possible.
* Refactor: Solved circular dependency between tools.py and logger.py to fix bit-team#820

CHANGES

@@ -3,10 +3,13 @@ Back In Time
 Version 1.4.2-dev (development of upcoming release)
 * Fix bug: RTE: module 'qttools' has no attribute 'initate_translator' with encFS when prompting the user for a password (#1553).
 * Fix bug: Schedule dropdown menu used "minutes" instead of "hours".
+* Fix bug: Unhandled exception "TypeError: 'NoneType' object is not callable" in tools.py function __log_keyring_warning (#820).
+           Logging thread removed and logger module correctly initialized as fix. Is "Heisenbug" so 100 % retesting was not possible.
 * Build: Use PyLint in unit testing to catch E1101 (no-member) errors.
 * Build: Activate PyLint warning W1401 (anomalous-backslash-in-string).
 * Build: Add codespell config.
 * Translation: Minor modifications in source strings and updating language files.
+* Refactor: Solved circular dependency between tools.py and logger.py to fix #820
 
 Version 1.4.1 (2023-10-01)
 * Dependency: Add "qt translations" to GUI runtime dependencies (#1538).

common/logger.py

@@ -20,7 +20,6 @@
 import sys
 import atexit
 
-import tools
 import bcolors
 
 DEBUG = False            # Set to "True" when passing "--debug" as cmd arg
@@ -51,7 +50,7 @@ def closelog():
 
 
 def _do_syslog(message: str, level: int) -> str:
-    for line in tools.wrapLine(message):
+    for line in wrapLine(message):
         syslog.syslog(level, '{}{}: {}'.format(
             SYSLOG_MESSAGE_PREFIX, _level_names[level], line))
 
@@ -136,3 +135,43 @@ def _debugHeader(parent, traceDepth):
 
     func = frame.f_code.co_name
     return '[%s/%s:%s %s%s]' % (fmodule, fname, line, fclass, func)
+
+
+# This function was moved from tools.py to here to solve a circular
+# import dependency between "tools" and "logger".
+def wrapLine(msg, size=950, delimiters='\t ', new_line_indicator = 'CONTINUE: '):
+    """
+    Wrap line ``msg`` into multiple lines with each shorter than ``size``. Try
+    to break the line on ``delimiters``. New lines will start with
+    ``new_line_indicator``.
+
+    Args:
+        msg (str):                  string that should get wrapped
+        size (int):                 maximum length of returned strings
+        delimiters (str):           try to break ``msg`` on these characters
+        new_line_indicator (str):   start new lines with this string
+
+    Yields:
+        str:                        lines with max ``size`` length
+    """
+
+    # TODO Use "textwrap.wrap" instead (https://docs.python.org/3/library/textwrap.html)
+    #      (which may change the output formatting and may affect unit tests then)
+    #      To avoid duplicated argument values in calls this function could
+    #      act as a wrapper-
+
+    if len(new_line_indicator) >= size - 1:
+        new_line_indicator = ''
+    while msg:
+        if len(msg) <= size:
+            yield(msg)
+            break
+        else:
+            line = ''
+            for look in range(size-1, size//2, -1):
+                if msg[look] in delimiters:
+                    line, msg = msg[:look+1], new_line_indicator + msg[look+1:]
+                    break
+            if not line:
+                line, msg = msg[:size], new_line_indicator + msg[size:]
+            yield(line)

common/tools.py

@@ -36,17 +36,30 @@
 from datetime import datetime
 from packaging.version import Version
 from time import sleep
-keyring = None
-keyring_warn = False
+
+import logger
+
+# Try to import keyring
+is_keyring_available = False
 try:
+    # Jan 4, 2024 aryoda: The env var BIT_USE_KEYRING is neither documented
+    #                     anywhere nor used at all in the code.
+    #                     Via "git blame" I have found a commit message saying:
+    #                     "block subsequent 'import keyring' if it failed once"
+    #                     So I assume it is an internal temporary env var only.
+    # Note: os.geteuid() is used instead of tools.isRoot() here
+    #       because the latter is still not available here in the global
+    #       module code.
     if os.getenv('BIT_USE_KEYRING', 'true') == 'true' and os.geteuid() != 0:
         import keyring
         from keyring import backend
         import keyring.util.platform_
-except:
-    keyring = None
+        is_keyring_available = True
+except Exception as e:
+    is_keyring_available = False
+    # block subsequent 'import keyring' if it failed once before
     os.putenv('BIT_USE_KEYRING', 'false')
-    keyring_warn = True
+    logger.warning(f"'import keyring' failed with: {repr(e)}")
 
 # getting dbus imports to work in Travis CI is a huge pain
 # use conditional dbus import
@@ -57,13 +70,12 @@
     import dbus
 except ImportError:
     if ON_TRAVIS or ON_RTD:
-        #python-dbus doesn't work on Travis yet.
+        # python-dbus doesn't work on Travis yet.
         dbus = None
     else:
         raise
 
 import configfile
-import logger
 import bcolors
 from applicationinstance import ApplicationInstance
 from exceptions import Timeout, InvalidChar, InvalidCmd, LimitExceeded, PermissionDeniedByPolicy
@@ -1099,7 +1111,14 @@ def envSave(f):
     env_file.save(f)
 
 def keyringSupported():
-    if keyring is None:
+    """
+    Checks if a keyring (supported by BiT) is available
+
+    Returns:
+         bool: ``True`` if a supported keyring could be loaded
+    """
+
+    if not is_keyring_available:
         logger.debug('No keyring due to import error.')
         return False
 
@@ -1162,7 +1181,7 @@ def keyringSupported():
             # Load the backend step-by-step.
             # e.g. When the target is "keyring.backends.Gnome.Keyring" then in
             # a first step "Gnome" part is loaded first and if successful the
-            # "Keyring" part.
+            # "keyring" part.
             for b in backends:
                 result = getattr(result, b)
 
@@ -1192,12 +1211,14 @@ def keyringSupported():
 
 
 def password(*args):
-    if not keyring is None:
+
+    if is_keyring_available:
         return keyring.get_password(*args)
     return None
 
 def setPassword(*args):
-    if not keyring is None:
+
+    if is_keyring_available:
         return keyring.set_password(*args)
     return False
 
@@ -1455,36 +1476,6 @@ def filesystemMountInfo():
                 [mount_line.strip('\n').split(' ')[:2] for mount_line in mounts]
                 if uuidFromDev(items[0]) != None}
 
-def wrapLine(msg, size=950, delimiters='\t ', new_line_indicator = 'CONTINUE: '):
-    """
-    Wrap line ``msg`` into multiple lines with each shorter than ``size``. Try
-    to break the line on ``delimiters``. New lines will start with
-    ``new_line_indicator``.
-
-    Args:
-        msg (str):                  string that should get wrapped
-        size (int):                 maximum length of returned strings
-        delimiters (str):           try to break ``msg`` on these characters
-        new_line_indicator (str):   start new lines with this string
-
-    Yields:
-        str:                        lines with max ``size`` length
-    """
-    if len(new_line_indicator) >= size - 1:
-        new_line_indicator = ''
-    while msg:
-        if len(msg) <= size:
-            yield(msg)
-            break
-        else:
-            line = ''
-            for look in range(size-1, size//2, -1):
-                if msg[look] in delimiters:
-                    line, msg = msg[:look+1], new_line_indicator + msg[look+1:]
-                    break
-            if not line:
-                line, msg = msg[:size], new_line_indicator + msg[size:]
-            yield(line)
 
 def syncfs():
     """
@@ -1503,14 +1494,18 @@ def isRoot():
     Returns:
         bool:   ``True`` if we are root
     """
+
+    # The EUID (Effective UID) may be different from the UID (user ID)
+    # in case of SetUID or using "sudo" (where EUID is "root" and UID
+    # is the original user who executed "sudo").
     return os.geteuid() == 0
 
 def usingSudo():
     """
     Check if 'sudo' was used to start this process.
 
     Returns:
-        bool:   ``True`` if process was started with sudo
+        bool:   ``True`` if the process was started with sudo
     """
     return isRoot() and os.getenv('HOME', '/root') != '/root'
 
@@ -2741,13 +2736,33 @@ def run(self):
         """
         pass
 
-def __logKeyringWarning():
-    from time import sleep
-    sleep(0.1)
-    logger.warning('import keyring failed')
-
-if keyring is None and keyring_warn:
-    #delay warning to give logger some time to import
-    from threading import Thread
-    thread = Thread(target = __logKeyringWarning, args = ())
-    thread.start()
+# def __logKeyringWarning():
+#
+#     from time import sleep
+#     sleep(0.1)
+#     # TODO This function may not be thread-safe
+#     logger.warning('import keyring failed')
+#
+#
+#
+# if is_keyring_available:
+#
+#     # delay warning to give logger some time to import
+#
+#     # Jan 4, 2024 aryoda:
+#     # This is an assumed work-around for #820 (unhandled exception: NoneType)
+#     # but does not seem to fix the problem.
+#     # So I have refactored the possible name shadowing of "keyring"
+#     # as described in
+#     # https://github.com/bit-team/backintime/issues/820#issuecomment-1472971734
+#     # and left this code unchanged to wait for user feed-back if it works now.
+#     # If the error still occurs I would move the log output call
+#     # to the client of this module so that it is certain to assume it is
+#     # correctly initialized.
+#     # Maybe use backintime.py and app.py for logging...
+#     # (don't call tools.keyringSupported() for that because
+#     # it produces too much debug logging output whenever it is called
+#     # but just query tools.is_keyring_available.
+#     from threading import Thread
+#     thread = Thread(target=__logKeyringWarning, args=())
+#     thread.start()

qt/qtsystrayicon.py

@@ -31,11 +31,12 @@
 import qttools
 qttools.registerBackintimePath('common')
 
+import logger
+
 # Workaround until the codebase allows a single place to init all translations
 import tools
 tools.initiate_translation(None)
 
-import logger
 import snapshots
 import progress
 import logviewdialog
@@ -171,11 +172,11 @@ def updateInfo(self):
                 self.last_message = message
                 if self.decode:
                     message = (message[0], self.decode.log(message[1]))
-                self.menuStatusMessage.setText('\n'.join(tools.wrapLine(message[1],\
-                                                                         size = 80,\
-                                                                         delimiters = '',\
-                                                                         new_line_indicator = '') \
-                                                                       ))
+                self.menuStatusMessage.setText('\n'.join(logger.wrapLine(message[1], \
+                                                                                size = 80, \
+                                                                                delimiters = '', \
+                                                                                new_line_indicator = '') \
+                                                         ))
                 self.status_icon.setToolTip(message[1])
 
         pg = progress.ProgressFile(self.config)