diff --git a/docs/source/pcapkit/utilities/exceptions.rst b/docs/source/pcapkit/utilities/exceptions.rst index d0daeec3c..970de3957 100644 --- a/docs/source/pcapkit/utilities/exceptions.rst +++ b/docs/source/pcapkit/utilities/exceptions.rst @@ -29,7 +29,7 @@ It is still an ordinary exception carrying its message, so ``except`` clauses an :func:`repr` are unaffected. An error is also quiet when it is constructed inside a -:class:`~pcapkit.utilities.exceptions._CaughtByCaller` block naming one of its +:class:`~pcapkit.utilities.exceptions.CaughtByCaller` block naming one of its classes. That is how :mod:`pcapkit` marks as control flow an error raised by code it calls but does not own, such as a generated ``_validate_value`` override, which cannot know to pass ``quiet=True`` itself (:issue:`1845`). @@ -54,7 +54,7 @@ override, which cannot know to pass ``quiet=True`` itself (:issue:`1845`). :param \*args: Arbitrary positional arguments. :param \*\*kwargs: Arbitrary keyword arguments. -.. autoclass:: pcapkit.utilities.exceptions._CaughtByCaller +.. autoclass:: pcapkit.utilities.exceptions.CaughtByCaller :no-members: :exc:`TypeError` Category diff --git a/pcapkit/const/pcapng/option_type.py b/pcapkit/const/pcapng/option_type.py index caae049e8..ec309571e 100644 --- a/pcapkit/const/pcapng/option_type.py +++ b/pcapkit/const/pcapng/option_type.py @@ -16,7 +16,7 @@ from aenum import StrEnum, extend_enum from pcapkit.corekit.enum import EnumRegistry -from pcapkit.utilities.exceptions import EnumValueError +from pcapkit.utilities.exceptions import CaughtByCaller, EnumValueError __all__ = ['OptionType'] @@ -277,7 +277,10 @@ def register(cls, value: 'int', name: 'str') -> 'OptionType': if existing is not None: raise ValueError(f'{value!r} is already registered on {cls.__name__} as ' f'{existing.name!r} in namespace {space!r}') - cls._validate_value(value) + # NOTE: GitHub issue #1845. Declares that nothing is caught, so the + # rejection stays loud even inside a ``get`` block that would quiet it. + with CaughtByCaller(): + cls._validate_value(value) try: return extend_enum(cls, name, value, name) except TypeError as error: diff --git a/pcapkit/const/reg/apptype/apptype.py b/pcapkit/const/reg/apptype/apptype.py index 984732e67..353f5e2cb 100644 --- a/pcapkit/const/reg/apptype/apptype.py +++ b/pcapkit/const/reg/apptype/apptype.py @@ -17,7 +17,7 @@ from pcapkit.corekit.enum import EnumLookup, EnumRegistry from pcapkit.corekit.sentinels import NO_DEFAULT -from pcapkit.utilities.exceptions import EnumValueError +from pcapkit.utilities.exceptions import CaughtByCaller, EnumValueError __all__ = ['TransportProtocol', 'AppType'] @@ -2831,7 +2831,10 @@ def register(cls, port: 'int', name: 'str') -> 'AppType': # pylint: disable=arg f'({", ".join(member.svc for member in existing)}); use ' f'{cls.__name__}.register_alias() to add a further service name ' 'for it') - cls._validate_value(port) + # NOTE: GitHub issue #1845. Declares that nothing is caught, so the + # rejection stays loud even inside a ``get`` block that would quiet it. + with CaughtByCaller(): + cls._validate_value(port) identifier = cls._sanitize_identifier(name) if not identifier.isidentifier(): diff --git a/pcapkit/corekit/enum.py b/pcapkit/corekit/enum.py index f8c8cf678..ef53f152d 100644 --- a/pcapkit/corekit/enum.py +++ b/pcapkit/corekit/enum.py @@ -100,7 +100,7 @@ from aenum import extend_enum from pcapkit.corekit.sentinels import NO_DEFAULT -from pcapkit.utilities.exceptions import BaseError, EnumKeyError, EnumValueError, _CaughtByCaller +from pcapkit.utilities.exceptions import BaseError, CaughtByCaller, EnumKeyError, EnumValueError if TYPE_CHECKING: from typing import Any @@ -200,7 +200,7 @@ def _validate_value(cls, value: 'Any') -> 'None': :meth:`get`'s own ``except ValueError`` and falls back to ``default`` just as any other unresolvable value does. It is also quiet: :meth:`get` always calls this hook inside a - :class:`~pcapkit.utilities.exceptions._CaughtByCaller` block for + :class:`~pcapkit.utilities.exceptions.CaughtByCaller` block for :exc:`ValueError`, so an override need not pass ``quiet=True`` for its rejection to log nothing there (:issue:`1845`). An override raising something outside that hierarchy would instead propagate past @@ -342,7 +342,7 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': given on :meth:`_validate_value` itself. The hook and ``cls(key)`` run inside a - :class:`~pcapkit.utilities.exceptions._CaughtByCaller` block for + :class:`~pcapkit.utilities.exceptions.CaughtByCaller` block for :exc:`ValueError`, because every :exc:`ValueError` they raise is this method's own lookup miss: caught here, then answered with ``default`` or raised to the caller as the miss. A loud @@ -445,7 +445,7 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': # this method's own lookup miss -- answered with ``default`` below, # or raised as the miss -- so a loud one is quiet here, whoever # constructed it. - with _CaughtByCaller(ValueError): + with CaughtByCaller(ValueError): cls._validate_value(key) return cls(key) # type: ignore[call-arg] except ValueError as error: @@ -546,7 +546,7 @@ def register(cls, value: 'Any', name: 'str') -> 'Self': hook, because there the rejection is a lookup miss for the caller to handle; here it refuses a mint the caller explicitly asked for, which is a fault to report. So this calls the hook in a - :class:`~pcapkit.utilities.exceptions._CaughtByCaller` block declaring + :class:`~pcapkit.utilities.exceptions.CaughtByCaller` block declaring that nothing is caught -- loud even if ``register`` is reached from inside a ``get`` (:issue:`1845`). @@ -577,7 +577,7 @@ def register(cls, value: 'Any', name: 'str') -> 'Self': f'to add a further name for it') # NOTE: GitHub issue #1845. Declares that nothing is caught, so the # rejection stays loud even inside a ``get`` block that would quiet it. - with _CaughtByCaller(): + with CaughtByCaller(): cls._validate_value(value) return cls._extend(value, name) diff --git a/pcapkit/utilities/exceptions.py b/pcapkit/utilities/exceptions.py index 4e37cca12..e7aa85e81 100644 --- a/pcapkit/utilities/exceptions.py +++ b/pcapkit/utilities/exceptions.py @@ -34,7 +34,7 @@ from typing import Any, Callable, Optional, Type __all__ = [ - 'stacklevel', + 'stacklevel', 'CaughtByCaller', 'BaseError', # Exception 'DigitError', 'IntError', 'RealError', 'ComplexError', # TypeError @@ -161,7 +161,7 @@ def stacklevel() -> 'int': #: ContextVar[tuple[Type[BaseException], ...]]: The exception classes the #: :mod:`pcapkit` code running in this context has undertaken to catch itself, -#: as declared by the innermost :class:`_CaughtByCaller` block. A +#: as declared by the innermost :class:`CaughtByCaller` block. A #: :class:`BaseError` that is an instance of one of them is constructed quiet, #: whatever its own ``quiet`` says. #: @@ -173,15 +173,19 @@ def stacklevel() -> 'int': ) # type: contextvars.ContextVar[tuple[Type[BaseException], ...]] -class _CaughtByCaller: +class CaughtByCaller: """Construct errors of the given classes quietly for the duration of a block. - For :mod:`pcapkit` code that is about to call something it does not own -- - a generated :meth:`~pcapkit.corekit.enum.EnumLookup._validate_value` - override, say -- and will catch the named exceptions itself. An error the - callee raises is then the quiet control flow it is at that call site, - without the callee having to pass ``quiet=True``, which it cannot know to - do (:issue:`1845`). Only the named classes are affected: anything else the + A :class:`BaseError` constructed inside the block that is an instance of one + of ``types`` behaves exactly as if it had been given ``quiet=True``: nothing + is logged and no exception hook is installed. Every other error stays loud. + + Use it around a call into code the caller does not own -- a generated + :meth:`~pcapkit.corekit.enum.EnumLookup._validate_value` override, say -- + whose named exceptions the caller then catches itself. An error the callee + raises is then the quiet control flow it is at that call site, without the + callee having to pass ``quiet=True``, which it cannot know to do + (:issue:`1845`). Only the named classes are affected: anything else the callee raises escapes the caller and so stays loud. Given no classes, it declares that nothing is caught, which makes errors @@ -191,7 +195,9 @@ class _CaughtByCaller: Re-entrant: on exit a block restores whatever the enclosing block declared. An instance holds the token of the one ``with`` it is entered by, so it is - built afresh for each. + built afresh for each. The declaration is per thread and per + :mod:`contextvars` context, so a block never quiets an error raised on + another thread. A class rather than :func:`contextlib.contextmanager`, because :meth:`EnumLookup.get ` enters one on @@ -201,6 +207,8 @@ class _CaughtByCaller: Args: *types: Exception classes the caller catches. + .. versionadded:: 1.5.0 + """ __slots__ = ('_types', '_token') @@ -239,7 +247,7 @@ class BaseError(Exception): catches it. An error is also quiet when it is constructed inside a - :class:`_CaughtByCaller` block naming one of its classes, which is how a + :class:`CaughtByCaller` block naming one of its classes, which is how a :mod:`pcapkit` caller marks as control flow an error raised by code that does not pass ``quiet=True`` itself. The effect is exactly that of ``quiet=True``. @@ -279,7 +287,7 @@ class BaseError(Exception): def __init__(self, *args: 'Any', quiet: 'bool' = False, **kwargs: 'Any') -> 'None': # log error -- a quiet error emits nothing and mutates nothing, and so - # does one a :class:`_CaughtByCaller` block says its caller catches + # does one a :class:`CaughtByCaller` block says its caller catches if not quiet and not isinstance(self, _caught_here.get()): if DEVMODE: logger.critical('%s: %s', type(self).__name__, str(self), diff --git a/pcapkit/vendor/pcapng/option_type.py b/pcapkit/vendor/pcapng/option_type.py index 948b88c8c..26654fe14 100644 --- a/pcapkit/vendor/pcapng/option_type.py +++ b/pcapkit/vendor/pcapng/option_type.py @@ -44,7 +44,7 @@ from aenum import StrEnum, extend_enum from pcapkit.corekit.enum import EnumRegistry -from pcapkit.utilities.exceptions import EnumValueError +from pcapkit.utilities.exceptions import CaughtByCaller, EnumValueError __all__ = ['{NAME}'] @@ -187,7 +187,10 @@ def register(cls, value: 'int', name: 'str') -> '{NAME}': if existing is not None: raise ValueError(f'{{value!r}} is already registered on {{cls.__name__}} as ' f'{{existing.name!r}} in namespace {{space!r}}') - cls._validate_value(value) + # NOTE: GitHub issue #1845. Declares that nothing is caught, so the + # rejection stays loud even inside a ``get`` block that would quiet it. + with CaughtByCaller(): + cls._validate_value(value) try: return extend_enum(cls, name, value, name) except TypeError as error: diff --git a/pcapkit/vendor/reg/apptype/apptype.py b/pcapkit/vendor/reg/apptype/apptype.py index 095ab5fad..2af795b2b 100644 --- a/pcapkit/vendor/reg/apptype/apptype.py +++ b/pcapkit/vendor/reg/apptype/apptype.py @@ -109,7 +109,7 @@ from pcapkit.corekit.enum import EnumLookup, EnumRegistry from pcapkit.corekit.sentinels import NO_DEFAULT -from pcapkit.utilities.exceptions import EnumValueError +from pcapkit.utilities.exceptions import CaughtByCaller, EnumValueError __all__ = ['TransportProtocol', '{NAME}'] @@ -785,7 +785,10 @@ def register(cls, port: 'int', name: 'str') -> '{NAME}': # pylint: disable=argu f'({{", ".join(member.svc for member in existing)}}); use ' f'{{cls.__name__}}.register_alias() to add a further service name ' 'for it') - cls._validate_value(port) + # NOTE: GitHub issue #1845. Declares that nothing is caught, so the + # rejection stays loud even inside a ``get`` block that would quiet it. + with CaughtByCaller(): + cls._validate_value(port) identifier = cls._sanitize_identifier(name) if not identifier.isidentifier(): diff --git a/tests/const/test_const_enum_builtin_parity.py b/tests/const/test_const_enum_builtin_parity.py index 3d7fa7c57..39558eb0f 100644 --- a/tests/const/test_const_enum_builtin_parity.py +++ b/tests/const/test_const_enum_builtin_parity.py @@ -435,7 +435,7 @@ def test_the_exception_is_not_an_in_library_one(self) -> None: its constructor unless it is quiet, so it fires even for an exception that is caught and discarded. The base :meth:`~pcapkit.corekit.enum.EnumLookup.get` constructs its lookup inside - a ``_CaughtByCaller(ValueError)`` block (#1845), which quiets a + a ``CaughtByCaller(ValueError)`` block (#1845), which quiets a :exc:`ValueError` subclass there, but a direct ``cls(value)`` caught by its caller has no such block: a guard raising a :class:`~pcapkit.utilities.exceptions.BaseError` subclass would log once diff --git a/tests/const/test_const_register_loud_in_quiet_block_1916_unit.py b/tests/const/test_const_register_loud_in_quiet_block_1916_unit.py new file mode 100644 index 000000000..c272d9703 --- /dev/null +++ b/tests/const/test_const_register_loud_in_quiet_block_1916_unit.py @@ -0,0 +1,77 @@ +# -*- coding: utf-8 -*- +"""The generated ``register`` overrides stay loud inside a quiet block. + +GitHub issue #1916. :meth:`~pcapkit.corekit.enum.EnumRegistry.register` runs +:meth:`~pcapkit.corekit.enum.EnumLookup._validate_value` inside a +:class:`~pcapkit.utilities.exceptions.CaughtByCaller` block declaring that +nothing is caught, so its rejection is logged even when an enclosing ``get`` +has quieted :exc:`ValueError` (#1845). The overrides on +:class:`~pcapkit.const.reg.apptype.apptype.AppType` and +:class:`~pcapkit.const.pcapng.option_type.OptionType` called the hook bare, so +inside such a block their rejection raised and logged nothing. + +Each test opens the quiet block the way ``get`` does and checks that the +rejection still leaves its record on the ``pcapkit`` logger. The record comes +from :mod:`pcapkit.utilities.exceptions`'s own logger, a child of that one. + +This module is unit-tier and reads no capture. + +""" +from __future__ import annotations + +import logging +import sys +import threading +import unittest +from typing import TYPE_CHECKING + +from tests.utilities._harness import capture + +if TYPE_CHECKING: + from typing import Callable + + +class RegisterLoudInQuietBlock1916Tests(unittest.TestCase): + """``register`` on a generated registry logs its rejection inside a quiet block.""" + + def setUp(self) -> 'None': + # A loud error outside development mode replaces both hooks; put the + # real ones back so this test does not leak pcapkit's into later ones. + saved_sys, saved_threading = sys.excepthook, threading.excepthook + + def _restore() -> 'None': + sys.excepthook = saved_sys + threading.excepthook = saved_threading + + self.addCleanup(_restore) + + def assertLoudInsideQuietBlock(self, registry: 'type', call: 'Callable[[], object]', + message: 'str') -> 'None': + """Assert ``call`` raises ``message`` and logs it, inside a quiet block.""" + from pcapkit.utilities.exceptions import CaughtByCaller, EnumValueError + + members = dict(registry.__members__) # type: ignore[attr-defined] + with capture(logging.getLogger('pcapkit')) as package: + with CaughtByCaller(ValueError): + with self.assertRaises(EnumValueError) as caught: + call() + self.assertEqual(str(caught.exception), message) + self.assertEqual(package.messages, [('CRITICAL', f'EnumValueError: {message}')], + 'the rejection was quieted by the enclosing block') + self.assertEqual(dict(registry.__members__), members) # type: ignore[attr-defined] + + def test_apptype_register(self) -> 'None': + from pcapkit.const.reg.apptype import TCP + + self.assertLoudInsideQuietBlock(TCP, lambda: TCP.register(70000, 'x'), + '70000 is not a valid TCP') + + def test_option_type_register(self) -> 'None': + from pcapkit.const.pcapng.option_type import OptionType + + self.assertLoudInsideQuietBlock(OptionType, lambda: OptionType.register(-1, 'x'), + '-1 is not a valid OptionType') + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/corekit/test_enum_get_quiet_fallback_1845_unit.py b/tests/corekit/test_enum_get_quiet_fallback_1845_unit.py index 9fe3ed465..67e15b789 100644 --- a/tests/corekit/test_enum_get_quiet_fallback_1845_unit.py +++ b/tests/corekit/test_enum_get_quiet_fallback_1845_unit.py @@ -10,7 +10,7 @@ ``default`` or without one -- without, the miss is still raised, just not reported -- and kept ``register`` loud. The quiet path lives in :mod:`pcapkit.corekit`, so the generated overrides are untouched: ``get`` runs -them inside a :class:`~pcapkit.utilities.exceptions._CaughtByCaller` block, which +them inside a :class:`~pcapkit.utilities.exceptions.CaughtByCaller` block, which makes a :exc:`ValueError`-derived :class:`~pcapkit.utilities.exceptions.BaseError` constructed there quiet. @@ -286,16 +286,16 @@ def test_register_reached_from_a_raising_get_stays_loud(self) -> 'None': def test_register_inside_a_quiet_block_stays_loud(self) -> 'None': with _records() as (_, module): - with exceptions._CaughtByCaller(ValueError): + with exceptions.CaughtByCaller(ValueError): with self.assertRaises(EnumValueError): ToSPrecedence.register(OUT_OF_RANGE, 'Block_1845') self.assertEqual(module.messages, [LOUD_RECORD]) def test_blocks_nest_and_restore(self) -> 'None': with _records() as (_, module): - with exceptions._CaughtByCaller(ValueError): + with exceptions.CaughtByCaller(ValueError): EnumValueError('outer, quiet') - with exceptions._CaughtByCaller(): + with exceptions.CaughtByCaller(): EnumValueError('inner, loud') EnumValueError('outer again, quiet') IntError('not a ValueError, loud')