diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index acb90f54a..7833f7b17 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -106,13 +106,10 @@ What is easy to get wrong about it: ``get`` re-raises a :exc:`ValueError` that is already a :exc:`~pcapkit.utilities.exceptions.BaseError` exactly as the override raised it, and converts only ``aenum``'s and :mod:`enum`'s own "no member carries this - value". The consequences are the point of the discrimination rather - than side effects: the override's **own message** survives to the caller instead - of being replaced by the base's, and the error is **logged once** rather than - twice, since :class:`~pcapkit.utilities.exceptions.BaseError` logs on - construction and re-wrapping would construct a second one. So an override should - say in its message what it rejected and why; that message is what the caller - sees. + value". The consequence is the point of the discrimination rather than a side + effect: the override's **own message** survives to the caller instead of being + replaced by the base's. So an override should say in its message what it rejected + and why; that message is what the caller sees. * **A** ``str`` **key never reaches it.** That path never calls ``cls(key)``, so it resolves only against already-populated lookup tables, where every value present is legal by construction. ``register`` does call it, after its duplicate check. @@ -162,7 +159,7 @@ Which branch a key takes, and what each miss ends in: NAME -->|"miss, no usable default"| KEYERR["EnumKeyError
quiet, derives KeyError"] NAME -->|"hit,
or miss, key is a registered value,
or miss, default is a registered value"| OK["canonical member"] VALUE -->|"resolves, or \_missing\_ answers,
or ValueError, default is a registered value"| OK - VALUE -->|"ValueError, no usable default"| VALERR["EnumValueError
loud, derives ValueError"] + VALUE -->|"ValueError, no usable default"| VALERR["EnumValueError
quiet, derives ValueError"] Do not "improve" on the shape by making both misses report identically. Converting one into the other is exactly what :issue:`923` retired, and it was retired in several @@ -172,19 +169,36 @@ base's :exc:`KeyError` into a :exc:`ValueError`, and :exc:`~pcapkit.utilities.exceptions.EnumValueError` for a name miss, so that the two ways of getting it wrong reported identically. -One asymmetry between the two is deliberate and is **not** visible from the -exception class: the **name** miss is raised quietly -(:class:`~pcapkit.utilities.exceptions.BaseError`'s ``quiet=True``, so nothing is -logged and :data:`sys.tracebacklimit` is left alone) while the **value** miss stays -loud. A name miss is in-library control flow at several call sites, and at -``Method.get`` it is part of a *successful* call -- -that override catches it in order to mint. A loud error there would put a -:data:`logging.CRITICAL` record on every such call and set -:data:`sys.tracebacklimit` to ``0`` process-wide, which is the -:issue:`362` defect ``quiet`` -exists for. So a ``get`` override that catches a name miss as control flow is -following the convention; one that catches a *value* miss that way is silencing a -logged error, and needs a reason. +One more thing is not visible from the exception class, and it is deliberate: +whether a miss is **reported**. **Every miss** ``get`` **reports is quiet** -- by name +or by value, whether ``default`` answers it or it is raised -- while ``register`` +refusing the same value is loud. Quiet means what +:class:`~pcapkit.utilities.exceptions.BaseError`'s ``quiet=True`` means: nothing is +logged and no exception hook is installed. A raised miss is still an ordinary +exception that reaches the caller; left uncaught, it is printed by whichever hook is +installed, the interpreter's own unless an earlier loud error installed +:mod:`pcapkit`'s. + +* **The name miss** is raised with ``quiet=True``. It is in-library control flow at + several call sites, and at ``Method.get`` it is part of a *successful* call -- + that override catches it in order to mint. +* **The value miss** is quiet with or without a usable ``default``. The owner ruled + for that on :issue:`1845`, on the ground that ``get`` is mostly called from + protocol parsing, where a value that does not resolve is input for the caller to + handle rather than a fault to report. ``get`` runs ``_validate_value`` and + ``cls(key)`` in a block declaring that it catches :exc:`ValueError` itself, so the + loud :exc:`~pcapkit.utilities.exceptions.EnumValueError` a generated override + raises is constructed quietly without the override passing ``quiet=True``, and + the error ``get`` converts from ``aenum``'s own is raised quiet as well. An error + of any other class is not a lookup miss, and stays loud. +* :meth:`~pcapkit.corekit.enum.EnumRegistry.register` **stays loud**, even when + reached from inside such a block: refusing a mint the caller explicitly asked for + is a fault to report, not a lookup that missed. + +A loud miss would put a :data:`logging.CRITICAL` record on every lookup that misses +-- the :issue:`362` defect ``quiet`` exists for -- and, outside development mode, +install :mod:`pcapkit`'s exception hooks. So catching either miss as control flow, in +a ``get`` override or anywhere else, follows the convention and silences nothing. ``get`` Override Obligations ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/pcapkit/corekit/enum.py b/pcapkit/corekit/enum.py index db1a9c08c..4daf1b256 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 +from pcapkit.utilities.exceptions import BaseError, EnumKeyError, EnumValueError, _CaughtByCaller if TYPE_CHECKING: from typing import Any @@ -198,14 +198,17 @@ def _validate_value(cls, value: 'Any') -> 'None': :meth:`get` path: :exc:`~pcapkit.utilities.exceptions.EnumValueError` subclasses :exc:`ValueError`, so a rejection here is caught by :meth:`get`'s own ``except ValueError`` and falls back to ``default`` - just as any other unresolvable value does. An override raising something - outside that hierarchy would instead propagate past ``default``, which is - a real difference in behaviour rather than a stylistic preference. With - no usable ``default``, a rejection from this hook reaches the caller - exactly as the override raised it -- :meth:`get` re-raises an in-library - ``ValueError`` unchanged rather than re-wrapping it, so the override's own - message and the single log record it already emitted are what the caller - sees. + 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 + :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 + ``default``, unquieted, which is a real difference in behaviour rather + than a stylistic preference. With no usable ``default``, a rejection + from this hook reaches the caller exactly as the override raised it -- + :meth:`get` re-raises an in-library ``ValueError`` unchanged rather than + re-wrapping it, so the override's own message is what the caller sees. Called from the places below, and the omissions are deliberate: @@ -216,7 +219,10 @@ def _validate_value(cls, value: 'Any') -> 'None': every value present is legal by construction, so there is nothing left to validate. * :meth:`EnumRegistry.register`, before minting -- the one point at which - a caller can introduce a value no member carries yet. + a caller can introduce a value no member carries yet. Its rejection + stays loud even when ``register`` runs inside :meth:`get`'s quiet + block, since a refused mint is a fault to report rather than a + lookup that missed. Of the registries that override :meth:`EnumRegistry.register` to mint through a path of their own, @@ -335,6 +341,17 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': on the same terms; the ``str`` path does not call the hook, for the reason given on :meth:`_validate_value` itself. + The hook and ``cls(key)`` run inside a + :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 + :exc:`~pcapkit.utilities.exceptions.EnumValueError` an override + constructs there is therefore quiet, though the override never passes + ``quiet=True`` -- the generated overrides cannot know which caller they + serve. An error of any other class is not a lookup miss and escapes + this method as it was raised, so it stays loud. + Both failure paths raise from :mod:`pcapkit.utilities.exceptions` rather than a builtin, per the ruling recorded on GitHub issue :issue:`923`: in-library code raises from ``pcapkit.utilities.exceptions`` @@ -348,27 +365,34 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': These details are worth stating, since the exception type alone does not show them: - * **The name miss is raised quietly** -- - :exc:`~pcapkit.utilities.exceptions.EnumKeyError` with ``quiet=True``, - so nothing is logged and :data:`sys.tracebacklimit` is left alone. - That is not a cosmetic choice: this method's name miss is in-library - control flow, and at + * **Every miss is quiet**, by name or by value, whether answered with + ``default`` or raised: nothing is logged and no exception hook is + installed. A raised miss still reaches the caller as an ordinary + exception; left uncaught, it is printed by whichever hook is installed + -- the interpreter's own, unless an earlier loud error installed + :mod:`pcapkit`'s. The name miss is + :exc:`~pcapkit.utilities.exceptions.EnumKeyError` with + ``quiet=True``, because it is in-library control flow -- at :meth:`~pcapkit.const.http.method.Method.get` it is part of a - *successful* call -- that override catches it in order to build an - unregistered member. A loud - error there would put a :data:`logging.CRITICAL` record on every such - call and set :data:`sys.tracebacklimit` to ``0`` process-wide, which - is exactly the GitHub issue :issue:`362` defect + *successful* call, since that override catches it in order to build + an unregistered member. The value miss is quiet through the block + described above, as ruled on :issue:`1845` on the ground that this + method is mostly called from protocol parsing: a value it cannot + resolve is input for the caller to handle, not a fault to report. A + loud miss would put a :data:`logging.CRITICAL` record on every such + call -- the GitHub issue :issue:`362` defect :class:`~pcapkit.utilities.exceptions.BaseError` documents ``quiet`` - for. The value miss takes no such fallback anywhere in this tree, so - it stays loud. + for -- and, outside development mode, install :mod:`pcapkit`'s + exception hooks. :meth:`EnumRegistry.register` refusing the same + value stays loud. * **An in-library rejection propagates unchanged.** A ``ValueError`` that is already a :exc:`~pcapkit.utilities.exceptions.BaseError` -- typically :exc:`~pcapkit.utilities.exceptions.EnumValueError` from a subclass's :meth:`_validate_value` -- is re-raised as it stands rather - than wrapped, so the subclass's own message survives and the error is - logged once instead of twice. Only :mod:`aenum`'s and :mod:`enum`'s - own "no member carries this value" is converted. This is the same + than wrapped, so the subclass's own message survives and no second + error is constructed. Only :mod:`aenum`'s and :mod:`enum`'s own "no + member carries this value" is converted, into a quiet + :exc:`~pcapkit.utilities.exceptions.EnumValueError`. This is the same discrimination :meth:`EnumField.post_process ` already makes for the same reason. @@ -393,10 +417,17 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': rejection reaches the caller as, since that hook is documented to raise this very class and it is passed through rather than re-wrapped. A :exc:`ValueError`, so an - ``except ValueError`` caller is unaffected. + ``except ValueError`` caller is unaffected. Quiet. EnumKeyError: If a name does not resolve and there is no usable default. A :exc:`KeyError`, so an ``except KeyError`` caller is - unaffected. + unaffected. Quiet. + + .. versionchanged:: 1.5.0 + + A value miss is quiet, whether ``default`` answers it or it is + raised: it logs nothing and installs no exception hook, while + :meth:`EnumRegistry.register` still reports the same range + rejection loudly (:issue:`1845`). """ if isinstance(key, str): @@ -410,13 +441,18 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': quiet=True) from None return cls._value2member_map_[default] try: - cls._validate_value(key) - return cls(key) # type: ignore[call-arg] + # NOTE: GitHub issue #1845. Every ValueError raised in this block is + # 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): + cls._validate_value(key) + return cls(key) # type: ignore[call-arg] except ValueError as error: if default is NO_DEFAULT or default not in cls._value2member_map_: if isinstance(error, BaseError): raise - raise EnumValueError(str(error)) from error + raise EnumValueError(str(error), quiet=True) from error return cls._value2member_map_[default] @classmethod @@ -506,6 +542,14 @@ def register(cls, value: 'Any', name: 'str') -> 'Self': for it than a range complaint would be, and validation is left to guard only the genuinely new value that is about to be minted. + That rejection is always loud. :meth:`~EnumLookup.get` quiets the same + 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 + that nothing is caught -- loud even if ``register`` is reached from + inside a ``get`` (:issue:`1845`). + Args: value: Value of the new member. name: Name of the new member. Required rather than derived, which @@ -531,7 +575,10 @@ def register(cls, value: 'Any', name: 'str') -> 'Self': raise ValueError(f'{value!r} is already registered on {cls.__name__} as ' f'{existing.name!r}; use {cls.__name__}.register_alias() ' f'to add a further name for it') - 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) return cls._extend(value, name) @classmethod diff --git a/pcapkit/utilities/exceptions.py b/pcapkit/utilities/exceptions.py index 338d08610..276fa0743 100644 --- a/pcapkit/utilities/exceptions.py +++ b/pcapkit/utilities/exceptions.py @@ -14,6 +14,7 @@ .. _tbtrim: https://github.com/gousaiyang/tbtrim """ +import contextvars import inspect import io import os @@ -158,6 +159,64 @@ 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 +#: :class:`BaseError` that is an instance of one of them is constructed quiet, +#: whatever its own ``quiet`` says. +#: +#: A :class:`~contextvars.ContextVar` rather than a module global, so that +#: entering a block on one thread never changes what another thread reads, and +#: one thread's lookup cannot silence an unrelated error on another. +_caught_here = contextvars.ContextVar( + 'pcapkit.utilities.exceptions._caught_here', default=(), +) # type: contextvars.ContextVar[tuple[Type[BaseException], ...]] + + +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 + callee raises escapes the caller and so stays loud. + + Given no classes, it declares that nothing is caught, which makes errors + loud again inside an enclosing block -- how + :meth:`~pcapkit.corekit.enum.EnumRegistry.register` keeps its rejection + loud whoever calls it. + + 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. + + A class rather than :func:`contextlib.contextmanager`, because + :meth:`EnumLookup.get ` enters one on + every call with a non-:class:`str` key, and the generator machinery + measured at several times the cost of the lookup it would wrap. + + Args: + *types: Exception classes the caller catches. + + """ + + __slots__ = ('_types', '_token') + + def __init__(self, *types: 'Type[BaseException]') -> 'None': + self._types = types + + def __enter__(self) -> 'None': + # NOTE: set here rather than in ``__init__``, since a token exists only + # once the variable has been set, which is what entering the block does. + self._token = _caught_here.set(self._types) # pylint: disable=attribute-defined-outside-init + + def __exit__(self, *exc_info: 'object') -> 'None': + _caught_here.reset(self._token) + + class BaseError(Exception): """Base error class of all kinds. @@ -179,6 +238,12 @@ class BaseError(Exception): still a perfectly ordinary exception, carrying its message for whoever 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 + :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``. + Important: * The terseness is deliberately not done by setting @@ -213,8 +278,9 @@ class BaseError(Exception): """ def __init__(self, *args: 'Any', quiet: 'bool' = False, **kwargs: 'Any') -> 'None': - # log error -- a quiet error emits nothing and mutates nothing - if not quiet: + # log error -- a quiet error emits nothing and mutates nothing, and so + # 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), exc_info=self if VERBOSE else False, diff --git a/tests/corekit/test_enum_get_exception_provenance_923_unit.py b/tests/corekit/test_enum_get_exception_provenance_923_unit.py index 33a51bbde..4e0d80db6 100644 --- a/tests/corekit/test_enum_get_exception_provenance_923_unit.py +++ b/tests/corekit/test_enum_get_exception_provenance_923_unit.py @@ -59,6 +59,7 @@ import pathlib import re import sys +import threading import unittest from typing import TYPE_CHECKING @@ -210,7 +211,7 @@ def test_an_in_library_rejection_is_passed_through_not_rewrapped(self) -> 'None' """A ``_validate_value`` override's own :exc:`~pcapkit.utilities.exceptions.EnumValueError` is what the caller sees, rather than a second one wrapping it -- so the override's message - survives and the error is logged once, not twice.""" + survives and no second error is constructed.""" with self.assertRaises(EnumValueError) as caught: _Ranged.get(99) self.assertIn('is not a valid _Ranged', str(caught.exception)) @@ -248,6 +249,9 @@ class BaseGetQuietnessTests(unittest.TestCase): ``quiet`` for, and :mod:`pcapkit.protocols.protocol` states the same rule inline: *"A pcapkit exception here would log at CRITICAL for something that is handled two lines later."* + + Since GitHub issue #1845 the value miss is quiet as well, so the control + that shows the recorder can see a loud error constructs one directly. """ def setUp(self) -> 'None': @@ -285,13 +289,29 @@ def test_the_minting_override_stays_silent_end_to_end(self) -> 'None': self.assertEqual(recorder.messages, []) self.assertEqual(minted.value, 'QUIET-923-PROBE') - def test_the_value_miss_is_loud_which_is_what_quiet_is_measured_against(self) -> 'None': - """The control. Without it ``recorder.messages == []`` above would also - pass if :class:`~pcapkit.utilities.exceptions.BaseError` had simply - stopped logging, rather than this one raise being quiet.""" + def test_the_value_miss_is_quiet_too(self) -> 'None': + """GitHub issue #1845: the owner ruled the value miss quiet as well, so + this was the loud control until then. It is still raised -- only not + reported.""" with capture(logger) as recorder: with self.assertRaises(EnumValueError): _Closed.get(99) + self.assertEqual(recorder.messages, []) + + def test_a_loud_error_still_logs_which_is_what_quiet_is_measured_against(self) -> 'None': + """The control. Without it ``recorder.messages == []`` above would also + pass if :class:`~pcapkit.utilities.exceptions.BaseError` had simply + stopped logging, rather than these raises being quiet. Constructed + directly, now that no ``get`` miss is loud.""" + saved_sys, saved_threading = sys.excepthook, threading.excepthook + + def _restore() -> 'None': + sys.excepthook = saved_sys + threading.excepthook = saved_threading + + self.addCleanup(_restore) + with capture(logger) as recorder: + EnumValueError('99 is not a valid _Closed') self.assertEqual([level for level, _ in recorder.messages], ['CRITICAL']) self.assertIn('EnumValueError', recorder.messages[0][1]) diff --git a/tests/corekit/test_enum_get_quiet_fallback_1845_unit.py b/tests/corekit/test_enum_get_quiet_fallback_1845_unit.py new file mode 100644 index 000000000..9fe3ed465 --- /dev/null +++ b/tests/corekit/test_enum_get_quiet_fallback_1845_unit.py @@ -0,0 +1,329 @@ +# -*- coding: utf-8 -*- +"""GitHub issue #1845: ``get`` is quiet for a value it cannot resolve; ``register`` is loud. + +#1823 gave every generated registry a ``_validate_value`` override raising a loud +:exc:`~pcapkit.utilities.exceptions.EnumValueError`, and +:meth:`~pcapkit.corekit.enum.EnumLookup.get` calls that hook before ``cls(key)``. +So ``get(out_of_range, default)`` returned ``default`` but logged a +:data:`logging.CRITICAL` record and installed :mod:`pcapkit`'s exception hooks +on the way. The owner ruled ``get`` quiet for such a value, with a usable +``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 +makes a :exc:`ValueError`-derived :class:`~pcapkit.utilities.exceptions.BaseError` +constructed there quiet. + +The generated registry under test is +:class:`~pcapkit.const.ipv4.tos_pre.ToSPrecedence`, whose override accepts +``0``-``7`` only. ``LLACode``, the registry the issue measured on, is avoided +because one of its members is being renamed in parallel (#1848). + +""" + +from __future__ import annotations + +import contextlib +import os +import subprocess # nosec B404 +import sys +import textwrap +import threading +import unittest +from typing import TYPE_CHECKING + +from aenum import IntEnum + +from pcapkit.const.ipv4.tos_pre import ToSPrecedence +from pcapkit.corekit.enum import EnumLookup, EnumRegistry +from pcapkit.utilities import exceptions +from pcapkit.utilities.exceptions import EnumValueError, IntError +from pcapkit.utilities.logging import DEVMODE +from pcapkit.utilities.logging import logger as package_logger +from tests.utilities._harness import capture + +if TYPE_CHECKING: + from typing import Any, Iterator + + from tests.utilities._harness import Recorder + +__all__ = ['QuietFallbackTests', 'QuietRaiseTests', 'LoudPathTests', 'MechanismTests'] + +#: A value :class:`ToSPrecedence`'s ``_validate_value`` refuses: the field is 3 bits. +OUT_OF_RANGE = 8 + +#: The message every rejection of :data:`OUT_OF_RANGE` carries. +MESSAGE = f'{OUT_OF_RANGE} is not a valid ToSPrecedence' + +#: The one record a loud rejection of :data:`OUT_OF_RANGE` leaves. +LOUD_RECORD = ('CRITICAL', f'EnumValueError: {MESSAGE}') + +#: Repository root, handed to a child interpreter as ``PYTHONPATH`` so it imports +#: this tree rather than whatever :mod:`pcapkit` is installed. +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +#: Set by :class:`_Blocking`'s hook once it is parked inside ``get``. +_INSIDE = threading.Event() +#: Set by the test to let :class:`_Blocking`'s hook reject and return. +_RELEASE = threading.Event() + + +@contextlib.contextmanager +def _records() -> 'Iterator[tuple[Recorder, Recorder]]': + """Capture the package logger and, separately, the exceptions module's own.""" + with capture(package_logger) as package, capture(exceptions.logger) as module: + yield package, module + + +class _Sparse(EnumLookup, IntEnum): + """No range hook and no ``_missing_``: a miss is :mod:`aenum`'s own error.""" + + one = 1 + + +class _Nested(EnumRegistry, IntEnum): + """A registry whose range hook calls ``register`` on another one. + + Contrived on purpose: nothing in :mod:`pcapkit.const` does this, and it is + the only way ``register`` can run while a ``get`` is in progress. + + """ + + zero = 0 + + @classmethod + def _validate_value(cls, value: 'Any') -> 'None': + try: + ToSPrecedence.register(OUT_OF_RANGE, 'Nested_1845') + except ValueError: + pass + if not 0 <= value <= 7: + raise EnumValueError(f'{value!r} is not a valid {cls.__name__}') + + +class _Blocking(EnumRegistry, IntEnum): + """A registry whose range hook parks until told to reject.""" + + zero = 0 + + @classmethod + def _validate_value(cls, value: 'Any') -> 'None': + _INSIDE.set() + _RELEASE.wait(timeout=10) + raise EnumValueError(f'{value!r} is not a valid {cls.__name__}') + + +class _HookGuard(unittest.TestCase): + """Install sentinel exception hooks, and restore the real ones afterwards. + + A loud error outside development mode replaces :data:`sys.excepthook` and + :data:`threading.excepthook`, so the loud controls below would otherwise leak + :mod:`pcapkit`'s hooks into every later test. The sentinels also make "left + the hooks alone" an identity check rather than a guess about what was there. + + """ + + def setUp(self) -> 'None': + saved_sys, saved_threading = sys.excepthook, threading.excepthook + + def _restore() -> 'None': + sys.excepthook = saved_sys + threading.excepthook = saved_threading + + self.addCleanup(_restore) + + def _sys_sentinel(*args: 'Any') -> 'None': + saved_sys(*args) + + def _threading_sentinel(args: 'Any') -> 'None': + saved_threading(args) + + self.sys_sentinel = _sys_sentinel + self.threading_sentinel = _threading_sentinel + sys.excepthook = _sys_sentinel + threading.excepthook = _threading_sentinel + + def assert_quiet(self, package: 'Recorder', module: 'Recorder') -> 'None': + """Nothing logged anywhere in :mod:`pcapkit`, and both hooks untouched.""" + self.assertEqual(module.messages, [], + 'get() logged from pcapkit.utilities.exceptions') + self.assertEqual(package.messages, []) + self.assertIs(sys.excepthook, self.sys_sentinel, 'get() replaced sys.excepthook') + self.assertIs(threading.excepthook, self.threading_sentinel) + + +class QuietFallbackTests(_HookGuard): + """An out-of-range value with a usable ``default`` is answered silently.""" + + def test_returns_the_default(self) -> 'None': + self.assertIs(ToSPrecedence.get(OUT_OF_RANGE, 0), ToSPrecedence.Routine) + self.assertIs(ToSPrecedence.get(-1, ToSPrecedence.Flash.value), ToSPrecedence.Flash) + + def test_logs_nothing_and_leaves_the_hooks_alone(self) -> 'None': + with _records() as (package, module): + ToSPrecedence.get(OUT_OF_RANGE, 0) + self.assert_quiet(package, module) + + def test_mints_nothing(self) -> 'None': + names, values = dict(ToSPrecedence._member_map_), dict(ToSPrecedence._value2member_map_) + with _records(): + ToSPrecedence.get(OUT_OF_RANGE, 0) + self.assertEqual(ToSPrecedence._member_map_, names) + self.assertEqual(ToSPrecedence._value2member_map_, values) + + +class QuietRaiseTests(_HookGuard): + """With no usable ``default`` the miss is still raised -- only not reported.""" + + def test_get_without_default_raises_quietly(self) -> 'None': + with _records() as (package, module): + with self.assertRaises(EnumValueError) as caught: + ToSPrecedence.get(OUT_OF_RANGE) + self.assertEqual(str(caught.exception), MESSAGE) + self.assert_quiet(package, module) + + def test_get_with_an_unregistered_default_raises_quietly(self) -> 'None': + self.assertNotIn(99, ToSPrecedence._value2member_map_) + with _records() as (package, module): + with self.assertRaises(EnumValueError) as caught: + ToSPrecedence.get(OUT_OF_RANGE, 99) + self.assertEqual(str(caught.exception), MESSAGE) + self.assert_quiet(package, module) + + def test_get_all_raises_quietly(self) -> 'None': + with _records() as (package, module): + with self.assertRaises(EnumValueError): + ToSPrecedence.get_all(OUT_OF_RANGE) + self.assert_quiet(package, module) + + def test_a_converted_enumeration_miss_is_quiet(self) -> 'None': + """``get`` converts :mod:`aenum`'s own miss, outside the block, so the + error it constructs has to be quiet in its own right.""" + with _records() as (package, module): + with self.assertRaises(EnumValueError) as caught: + _Sparse.get(5) + self.assertNotIsInstance(caught.exception.__cause__, exceptions.BaseError) + self.assertIsInstance(caught.exception.__cause__, ValueError) + self.assert_quiet(package, module) + + def test_an_uncaught_miss_prints_the_interpreters_own_traceback(self) -> 'None': + """Quiet installs no hook, so the default one prints the full traceback. + + Run in a fresh interpreter, because an uncaught exception is the one + thing a test cannot raise in its own process. :mod:`pcapkit`'s stderr + handler is attached there so that a loud error would also show up as a + ``[CRITICAL]`` line. + + """ + code = textwrap.dedent(f''' + import pcapkit + assert pcapkit.__file__.startswith({REPO_ROOT!r}), pcapkit.__file__ + from pcapkit.utilities.logging import handler, logger + logger.addHandler(handler) + from pcapkit.const.ipv4.tos_pre import ToSPrecedence + ToSPrecedence.get({OUT_OF_RANGE}) + ''') + env = dict(os.environ, PCAPKIT_DEVMODE='0', PYTHONSAFEPATH='1', PYTHONPATH=REPO_ROOT) + proc = subprocess.run([sys.executable, '-c', code], capture_output=True, # nosec B603 + text=True, env=env, timeout=60, check=False) + lines = proc.stderr.strip().splitlines() + self.assertEqual(proc.returncode, 1, proc.stderr) + self.assertNotIn('[CRITICAL]', proc.stderr, 'the miss was logged') + self.assertEqual(lines[0], 'Traceback (most recent call last):', + "pcapkit's terse hook printed the miss, so one was installed") + self.assertTrue(any('corekit' in line and 'enum.py' in line for line in lines), + 'the traceback has no frames inside get()') + self.assertEqual(lines[-1], f'pcapkit.utilities.exceptions.EnumValueError: {MESSAGE}') + + +class LoudPathTests(_HookGuard): + """What stays loud. These pass before the fix too: they guard against a fix + that quiets more than ``get``'s own lookup miss.""" + + def test_register_is_loud(self) -> 'None': + with _records() as (_, module): + with self.assertRaises(EnumValueError): + ToSPrecedence.register(OUT_OF_RANGE, 'Probe_1845') + self.assertEqual(module.messages, [LOUD_RECORD]) + if not DEVMODE: + self.assertIs(sys.excepthook, exceptions._excepthook) + self.assertNotIn('Probe_1845', ToSPrecedence.__members__) + + def test_a_non_value_error_from_the_hook_is_loud(self) -> 'None': + """``get`` catches only :exc:`ValueError`, so only that is a lookup miss.""" + + class _TypeRejecting(EnumRegistry, IntEnum): + zero = 0 + + @classmethod + def _validate_value(cls, value: 'Any') -> 'None': + raise IntError(f'{value!r} is not an int-like {cls.__name__}') + + with _records() as (_, module): + with self.assertRaises(IntError): + _TypeRejecting.get(5, 0) + self.assertEqual([level for level, _ in module.messages], ['CRITICAL']) + + +class MechanismTests(_HookGuard): + """The block is re-entrant, per thread, and cannot quiet ``register``.""" + + def test_register_reached_from_inside_get_stays_loud(self) -> 'None': + with _records() as (_, module): + self.assertIs(_Nested.get(99, 0), _Nested.zero) + self.assertEqual(module.messages, [LOUD_RECORD], + 'expected exactly the register() rejection, loud, with the ' + "get() fallback's own rejection quiet") + + def test_register_reached_from_a_raising_get_stays_loud(self) -> 'None': + with _records() as (_, module): + with self.assertRaises(EnumValueError) as caught: + _Nested.get(99) + self.assertEqual(str(caught.exception), '99 is not a valid _Nested') + self.assertEqual(module.messages, [LOUD_RECORD], + 'expected exactly the register() rejection, loud, with the ' + "miss get() raised quiet") + + def test_register_inside_a_quiet_block_stays_loud(self) -> 'None': + with _records() as (_, module): + 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): + EnumValueError('outer, quiet') + with exceptions._CaughtByCaller(): + EnumValueError('inner, loud') + EnumValueError('outer again, quiet') + IntError('not a ValueError, loud') + EnumValueError('outside, loud') + self.assertEqual([message for _, message in module.messages], + ['EnumValueError: inner, loud', 'IntError: not a ValueError, loud', + 'EnumValueError: outside, loud']) + + def test_one_threads_block_does_not_quiet_another_thread(self) -> 'None': + _INSIDE.clear() + _RELEASE.clear() + result = [] # type: list[object] + worker = threading.Thread(target=lambda: result.append(_Blocking.get(5, 0))) + with _records() as (_, module): + worker.start() + try: + self.assertTrue(_INSIDE.wait(timeout=10), 'the worker never reached the hook') + # The worker is now parked inside get()'s quiet block. + EnumValueError('main thread, loud') + finally: + _RELEASE.set() + worker.join(timeout=10) + self.assertFalse(worker.is_alive()) + self.assertEqual(result, [_Blocking.zero]) + self.assertEqual([message for _, message in module.messages], + ['EnumValueError: main thread, loud'], + "the worker's fallback logged, or its block quieted the main thread") + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index 213724054..4c54d535d 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -623,7 +623,7 @@ class NonCanonicalKeyConvergenceTests(unittest.TestCase): (:mod:`pcapkit.corekit.enum`) branches on ``isinstance(key, str)`` instead, so the same key falls through to :meth:`~pcapkit.corekit.enum.EnumLookup._validate_value` and the - constructor, reaching :meth:`_missing_` and answering with a loud + constructor, reaching :meth:`_missing_` and answering with an :exc:`~pcapkit.utilities.exceptions.EnumValueError`. Deleting the overrides removes the branch that disagreed, so all seven now answer both keys the same way -- measured here rather than assumed, since