Skip to content

v2.1.x Chore - Refactor exception logging #386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
coderabbitai bot opened this issue May 1, 2025 · 1 comment
Open

v2.1.x Chore - Refactor exception logging #386

coderabbitai bot opened this issue May 1, 2025 · 1 comment
Assignees
Labels
Chore Miscellaneous chores to maintain the project Python Lang Changes to Python source code

Comments

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 1, 2025

Description

This issue tracks the refactoring of redundant exception logging instances throughout the codebase.

Problem

Several places in the codebase use redundant exception objects in logging.exception() calls. Since logging.exception() already automatically includes the exception traceback and message, passing the exception string explicitly is redundant.

Example:

except Exception as _cause:  # pragma: no branch
    _LOGGER.debug(str(type(_cause)))
    _LOGGER.exception(str(_cause))  # Redundant - exception already included
    _LOGGER.debug(str((_cause.args)))

Recommended Fix

Replace redundant exception logging with a descriptive message:

except Exception as _cause:  # pragma: no branch
    _LOGGER.debug(str(type(_cause)))
    _LOGGER.exception("Exception occurred while processing")  # Better
    _LOGGER.debug(str((_cause.args)))

Or, if no additional context is needed:

except Exception as _cause:  # pragma: no branch
    _LOGGER.debug(str(type(_cause)))
    _LOGGER.exception("")  # Let the exception details speak for themselves
    _LOGGER.debug(str((_cause.args)))

Files to Refactor

  • tests/__init__.py: Line ~197
  • [Add additional instances found during implementation]

Related

@reactive-firewall reactive-firewall moved this to Regressions in Multicast Project May 2, 2025
@reactive-firewall reactive-firewall added question Further information is requested Python Lang Changes to Python source code labels May 2, 2025
@reactive-firewall
Copy link
Owner

reactive-firewall commented May 2, 2025

Additional Occurrences

multicast/multicast/send.py

Lines 399 to 407 in 19d38db

try:
# Read configured amount of bytes at a time - matches read size by default
# skipcq: PYL-W0212
chunk = sys.stdin.read(
multicast._MCAST_DEFAULT_BUFFER_SIZE, # skipcq: PYL-W0212 - module ok
)
except OSError:
_logger.exception("[CWE-228] Error reading from stdin.")
break

try:
_bar = str("-" * 20)
logger = logging.getLogger(__module__)
logger.info(f"{_bar}GROUP{_bar}") # skipcq PYL-W1203 - test code ok
logger.info(f"{args.group}:{args.category}") # skipcq PYL-W1203 - test code ok
logger.info(f"{_bar}START{_bar}") # skipcq PYL-W1203 - test code ok
suite = get_test_suite(args.group, args.category)
runner = unittest.TextTestRunner(verbosity=2)
result = runner.run(suite)
logger.info(f"{_bar} END {_bar}") # skipcq PYL-W1203 - test code ok
del _bar # skipcq - cleanup any object leaks early
sys.exit(not result.wasSuccessful())
except ValueError: # pragma: no branch
logger.exception("Error occurred")
sys.exit(1)

multicast/tests/__init__.py

Lines 277 to 291 in 19d38db

try:
doc_suite.addTests(doctest.DocTestSuite(module=module, test_finder=finder))
except ValueError as _cause:
# ValueError is raised when no tests are found
_LOGGER.warning(
"No doctests found in %s: %s", # lazy formatting to avoid PYL-W1203
module.__name__,
_cause, # log as just warning level, instead of exception (error), but still detailed.
exc_info=True,
)
except Exception:
_LOGGER.exception(
"Error loading doctests from %s", # lazy formatting to avoid PYL-W1203
module.__name__,
)

multicast/tests/__init__.py

Lines 150 to 200 in 19d38db

try:
from tests import profiling as profiling # skipcq: PYL-C0414
from tests import test_basic
from tests import test_exceptions
from tests import test_deps
from tests import test_install_requires
from tests import test_manifest
from tests import test_build
from tests import test_usage
from tests import test_hear_server
from tests import test_hear_server_activate
from tests import test_hear_cleanup
from tests import test_hear_data_processing
from tests import test_hear_keyboard_interrupt
depends = [
profiling,
test_basic,
test_deps,
test_install_requires,
test_build,
test_manifest,
test_usage,
test_hear_server_activate,
test_hear_cleanup,
test_hear_data_processing,
test_exceptions,
test_hear_keyboard_interrupt,
test_hear_server
]
try:
from tests import test_fuzz
depends.insert(10, test_fuzz)
except Exception: # pragma: no branch
_LOGGER.exception("Error loading optional Fuzzing tests")
for unit_test in depends:
try:
if unit_test.__name__ is None: # pragma: no branch
raise ImportError(
f"Test module failed to import even the {str(unit_test)} tests."
) from None
except Exception as _root_cause: # pragma: no branch
raise ImportError("[CWE-758] Test module failed completely.") from _root_cause
except Exception as _cause: # pragma: no branch
_LOGGER.debug(str(type(_cause)))
_LOGGER.exception(str(_cause))
_LOGGER.debug(str((_cause.args)))
del _cause # skipcq - cleanup any error leaks early
exit(0) # skipcq: PYL-R1722 - intentionally allow overwriteing exit for testing

already including:

multicast/tests/__init__.py

Lines 181 to 185 in 19d38db

try:
from tests import test_fuzz
depends.insert(10, test_fuzz)
except Exception: # pragma: no branch
_LOGGER.exception("Error loading optional Fuzzing tests")


Search of all uses of exception()

On GitHub.com:

https://github.com/search?q=%22.exception%28%22+repo%3Areactive-firewall%2Fmulticast&type=code

and locally:

git grep -F ".exception("

@reactive-firewall reactive-firewall removed the question Further information is requested label May 2, 2025
@reactive-firewall reactive-firewall added the Chore Miscellaneous chores to maintain the project label Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chore Miscellaneous chores to maintain the project Python Lang Changes to Python source code
Projects
Status: Regressions
Development

No branches or pull requests

1 participant