fix(corekit): quiet get's fallback over an out-of-range value (#1845)

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<br/>quiet, derives KeyError"]
        NAME -->|"hit,<br/>or miss, key is a registered value,<br/>or miss, default is a registered value"| OK["canonical member"]
        VALUE -->|"resolves, or \_missing\_ answers,<br/>or ValueError, default is a registered value"| OK
-       VALUE -->|"ValueError, no usable default"| VALERR["EnumValueError<br/>loud, derives ValueError"]
+       VALUE -->|"ValueError, no usable default"| VALERR["EnumValueError<br/>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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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
           <pcapkit.corekit.fields.numbers.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