refactor(protocols): keep a handled next-layer refusal quiet

docs/source/contributing/conventions/index.rst

@@ -17,8 +17,9 @@ House Conventions
      wrappers of :mod:`pcapkit.foundation.registry`;
    * :ref:`extension-header-subclassing` -- a protocol class hierarchy;
    * :ref:`protocol-family-grouping` -- how a protocol family's base and members
-     spell :meth:`~pcapkit.protocols.protocol.Protocol.id`, and how a protocol class
-     is named after its module;
+     spell :meth:`~pcapkit.protocols.protocol.Protocol.id`, how a protocol class
+     is named after its module, and how quietly a next-layer fallback keeps a
+     refused layer as :class:`~pcapkit.protocols.misc.raw.Raw`;
    * :ref:`protocol-member-order` -- the order of members in a protocol class body;
    * :ref:`protocol-layer-placement` -- which subpackage a dissector belongs in;
    * :ref:`process` -- the repository rather than any of its code;

docs/source/contributing/conventions/protocol-family-grouping.rst

@@ -72,6 +72,64 @@ dispatcher gives :class:`~pcapkit.protocols.application.httpv2.HTTPv2` that its
 then refuses, such as a preface and three octets, are
 :class:`~pcapkit.protocols.misc.raw.Raw` with the parser's ``error``.
 
+.. _protocol-family-refusals:
+
+Handled Refusals
+~~~~~~~~~~~~~~~~
+
+**A refusal a next-layer fallback handles is quiet**, for every protocol and not only
+a family's members (:issue:`2588`). The fallback is
+:func:`~pcapkit.corekit.decorators.beholder`. It decorates
+:meth:`Protocol._import_next_layer <pcapkit.protocols.protocol.Protocol._import_next_layer>`
+and the overrides that parse a next layer themselves.
+:meth:`Application._import_next_layer
+<pcapkit.protocols.application.application.Application._import_next_layer>` and
+:meth:`TLS._import_next_layer <pcapkit.protocols.application.tls.TLS._import_next_layer>`
+are not decorated: they delegate to decorated methods, the base's or TLS's
+``_import_next_record``. The decorated call runs inside a
+:class:`~pcapkit.utilities.exceptions.CaughtByCaller` block naming
+:exc:`~pcapkit.utilities.exceptions.ProtocolError`. A refused layer, the HTTP/2 case
+above or a GRE header of a nonzero version, is then recorded on the ``error`` of the
+:class:`~pcapkit.protocols.misc.raw.Raw` that replaces it, and nowhere else: nothing
+reaches the ``pcapkit`` logger at any level, and neither :func:`sys.excepthook` nor
+:func:`threading.excepthook` is replaced. A refusal the parser handles is the control
+flow :class:`~pcapkit.utilities.exceptions.BaseError`'s quiet mode exists for, and
+installing a hook from an error the library caught itself is a process-wide side
+effect.
+
+**The block spans the whole call to the next layer**, not the refusal alone. Every
+:exc:`~pcapkit.utilities.exceptions.ProtocolError` built while that call is on the
+stack is quiet, at any depth, unless an inner
+:class:`~pcapkit.utilities.exceptions.CaughtByCaller` says otherwise:
+
+* one a parser below catches itself, such as the ``supported_versions`` peek of
+  :meth:`SSL._tls_negotiates <pcapkit.protocols.application.ssl.SSL._tls_negotiates>`
+  or TLS's SSLv2 trial, which is then recorded nowhere;
+* one reporting library state rather than the octets, such as a
+  :class:`~pcapkit.corekit.module.ModuleDescriptor` entry whose class its module has
+  since lost -- by a later removal, or by a write to ``__proto__`` directly, since
+  :func:`~pcapkit.foundation.registry.protocols.register_tcp` refuses such an entry --
+  which ends as the ``Raw`` layer's ``error``;
+* one raised by an inner fallback's recovery, since the block of the fallback around
+  it is still active.
+
+What stays loud:
+
+* **Building the refusing layer directly.** ``GRE(...)`` or ``IPv4(...)`` on the same
+  octets raises an error that logs at :data:`logging.CRITICAL` and, outside development
+  mode, installs the hooks.
+* **Any other error class.** An error the fallback absorbs is as loud as it was
+  constructed, and :func:`~pcapkit.corekit.decorators.beholder` logs it at
+  :data:`logging.WARNING` as well.
+* **A refusal built under an inner** ``CaughtByCaller()``. A block naming no class,
+  such as the one :meth:`~pcapkit.corekit.enum.EnumRegistry.register` enters, makes
+  errors loud again inside the fallback's.
+* **The outermost fallback's recovery.** Reading the payload and building the ``Raw``
+  run after the block has ended, so under the outermost
+  :func:`~pcapkit.corekit.decorators.beholder` of a parse an error raised there reaches
+  the caller loud. Each block restores the one around it on exit, so none outlives
+  that parse.
+
 .. _protocol-family-shapes:
 
 Member Shapes

docs/source/pcapkit/utilities/exceptions.rst

@@ -33,6 +33,10 @@ An error is also quiet when it is constructed inside a
 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`).
+:func:`~pcapkit.corekit.decorators.beholder` runs the whole call to a next layer in
+such a block for :exc:`~pcapkit.utilities.exceptions.ProtocolError`, so a refusal it
+keeps as :class:`~pcapkit.protocols.misc.raw.Raw` is recorded on that layer's ``error``
+alone (:issue:`2588`).
 
 .. versionchanged:: 1.5.0
 

examples/generators/pcapng.py

@@ -132,11 +132,13 @@
 identically at both ends, and that the block chain covers the file exactly. So
 the complaints above are pcapkit's readings, not malformed fixtures.
 
-The complaints about one packet are expected rather than a defect:
-``test.pcapng`` carries a deliberately truncated packet (``captured_len`` below
-``original_len``, to exercise snapshot-length handling), and dissecting it
-reports ``packet length < 0: -2`` and an IPv4 header that runs past the end of
-the data, which is simply what a snapped frame looks like to a dissector.
+One packet is cut short on purpose rather than by a defect: ``test.pcapng``
+carries a deliberately truncated packet (``captured_len`` below
+``original_len``, to exercise snapshot-length handling), whose IPv4 header runs
+past the end of the data, which is simply what a snapped frame looks like to a
+dissector. The frame keeps that header as ``Raw``, whose ``error`` says so, and
+logs nothing for it (GitHub issue #2588), so verifying the fixture notes only
+the ``EOF reached`` its extraction ends on.
 
 Usage
 -----
@@ -979,11 +981,14 @@ class _LogCapture(logging.Handler):
 
     pcapkit reports a parse complaint on its ``pcapkit`` logger, and a
     complaint raised as a warning goes through :mod:`warnings` as well, where
-    the application's filters decide its fate. Only the logger sees all of
-    them: :func:`~pcapkit.corekit.decorators.beholder` logs a dissector
-    failure it recovers from without issuing a warning at all. So the logger
-    is what this listens to. Duplicates are dropped, because one complaint per block in a capture
-    with many packets is not that many pieces of information.
+    the application's filters decide its fate. The logger sees more of them:
+    :func:`~pcapkit.corekit.decorators.beholder` logs a dissector failure it
+    recovers from without issuing a warning at all -- other than a
+    :exc:`~pcapkit.utilities.exceptions.ProtocolError`, a refusal it keeps
+    quiet and records only on the replacing ``Raw`` layer's ``error`` (GitHub
+    issue #2588). So the logger is what this listens to. Duplicates are
+    dropped, because one complaint per block in a capture with many packets is
+    not that many pieces of information.
 
     """
 

pcapkit/corekit/decorators.py

@@ -31,7 +31,8 @@
 import traceback
 from typing import TYPE_CHECKING, cast
 
-from pcapkit.utilities.exceptions import StreamEOFError, StructError, stacklevel
+from pcapkit.utilities.exceptions import (CaughtByCaller, ProtocolError, StreamEOFError,
+                                          StructError, stacklevel)
 from pcapkit.utilities.logging import DEVMODE, VERBOSE, get_logger
 
 if TYPE_CHECKING:
@@ -113,10 +114,30 @@ def beholder(
         This decorator function is designed for decorating *instance methods*.
 
     This decorator calls the decorated function. Should any exception be
-    raised, it logs the error and re-parses the current layer's payload as
+    raised, it re-parses the current layer's payload as
     :class:`~pcapkit.protocols.misc.raw.Raw` -- or as
     :class:`~pcapkit.protocols.misc.null.NoPayload`, if the error is an
-    end-of-file :exc:`~pcapkit.utilities.exceptions.StructError`.
+    end-of-file :exc:`~pcapkit.utilities.exceptions.StructError` -- whose
+    ``error`` is the exception's message.
+
+    The decorated call runs inside a
+    :class:`~pcapkit.utilities.exceptions.CaughtByCaller` block naming
+    :exc:`~pcapkit.utilities.exceptions.ProtocolError` (:issue:`2588`). The
+    block spans the whole call, so every such error built while it runs is
+    quiet, at any depth and whether or not the code that built it catches it,
+    unless an inner :class:`~pcapkit.utilities.exceptions.CaughtByCaller` says
+    otherwise: the next layer refusing its octets, a parser's own trial, a
+    registry entry whose class its module has since lost. One that reaches
+    here is recorded on ``error`` and is not logged here either. Any other
+    exception is logged here at :data:`logging.WARNING`, and is as loud as it
+    was constructed.
+
+    The recovery runs after the block has ended. Under the outermost
+    :func:`beholder` of a parse, a
+    :exc:`~pcapkit.utilities.exceptions.ProtocolError` it raises therefore
+    reaches the caller loud; under any other, the block of the
+    :func:`beholder` around it is still active, so that error is quiet, and
+    becomes the enclosing replacement's ``error``.
 
     When :meth:`Protocol._parse_next_layer
     <pcapkit.protocols.protocol.Protocol._parse_next_layer>` held the
@@ -138,6 +159,12 @@ def beholder(
 
        Logs the exception it absorbs, and prints its traceback in verbose mode.
 
+    .. versionchanged:: 1.6.0
+
+       A :exc:`~pcapkit.utilities.exceptions.ProtocolError` the decorated
+       function builds is quiet, and one this decorator absorbs is not logged
+       here either (:issue:`2588`).
+
     :param func: decorated function
     :meta decorator:
     """
@@ -158,7 +185,10 @@ def behold(*args: 'P.args', **kwargs: 'P.kwargs') -> 'R_beholder':
             # call method
             token = _beholding.set(True)
             try:
-                return func(*args, **kwargs)
+                # NOTE: The whole call is quiet for a ProtocolError, which the ``except`` below
+                # records; the recovery runs after it, loud unless an outer block is active (#2588).
+                with CaughtByCaller(ProtocolError):
+                    return func(*args, **kwargs)
             finally:
                 _beholding.reset(token)
         except Exception as exc:
@@ -168,12 +198,14 @@ def behold(*args: 'P.args', **kwargs: 'P.kwargs') -> 'R_beholder':
                 from pcapkit.protocols.misc.raw import Raw as protocol  # type: ignore[assignment] # isort: skip # pylint: disable=import-outside-toplevel
             # error = traceback.format_exc(limit=1).strip().rsplit(os.linesep, maxsplit=1)[-1]
 
-            # log error
-            logger.warning(str(exc), stack_info=DEVMODE, stacklevel=stacklevel())
+            # NOTE: Keyed on the class, not on quietness: a quiet end-of-file StructError is still
+            # logged here, and a ProtocolError built loud has logged at CRITICAL already (#2588).
+            if not isinstance(exc, ProtocolError):
+                logger.warning(str(exc), stack_info=DEVMODE, stacklevel=stacklevel())
 
-            if VERBOSE:
-                logger.error('The following error occurred while parsing the packet:')
-                traceback.print_exc()
+                if VERBOSE:
+                    logger.error('The following error occurred while parsing the packet:')
+                    traceback.print_exc()
 
             # NOTE: ``self._get_payload()`` rather than
             # ``self.__header__.get_payload()``, which it wraps. The two agree

pcapkit/protocols/internet/ipv6.py

@@ -566,7 +566,7 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None,
             # A layer with no ``next`` field ends the walk, as RFC 8200 section 4
             # has a node discard the packet on a malformed header (#2477).
             # It is not recorded as an extension header.
-            # A Raw is reused as the upper layer, so its error logs once.
+            # A Raw is reused as the upper layer, so the header is not parsed again.
             if not hasattr(info, 'next'):
                 from pcapkit.protocols.misc.raw import Raw  # isort: skip # pylint: disable=import-outside-toplevel
                 # Exact type on purpose: Raw subclasses exist, so isinstance would over-match.