From f44d0bc5366c47bec75345f6a55c5e69c0258969 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 5 Oct 2026 15:36:53 -0400 Subject: [PATCH 1/3] fix(foundation): warn once when an extraction engine is unavailable (#1045) Extractor.import_test warned that the engine package was missing and returned None; run()'s else branch then warned again, so one problem raised two EngineWarnings. run() now stays silent there, and import_test's message names the module (engine PyPCAP (`pcap`) is not installed). --- pcapkit/foundation/extraction.py | 6 ++--- tests/foundation/test_extraction.py | 39 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index 0ccae748e..f6c342a01 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -690,8 +690,8 @@ def run(self) -> 'None': # pylint: disable=inconsistent-return-statements self.record_frames() return else: - warn(f'engine {eng.name} (`{eng.module}`) is not installed; ' - 'using default engine instead', EngineWarning, stacklevel=stacklevel()) + # ``import_test`` has already warned that the package is absent; + # warning again here would report one problem twice. self._exnam = 'default' # using default/pcapkit engine if self._exnam not in ('default', 'pcapkit'): @@ -734,7 +734,7 @@ def import_test(engine: 'str', *, name: 'Optional[str]' = None) -> 'Optional[Mod except ImportError: module = None logger.debug('engine module %r is not importable', engine) - warn(f"extraction engine '{name or engine}' not available; " + warn(f'engine {name or engine} (`{engine}`) is not installed; ' 'using default engine instead', EngineWarning, stacklevel=stacklevel()) return module diff --git a/tests/foundation/test_extraction.py b/tests/foundation/test_extraction.py index 9f150b0a7..f3816e886 100644 --- a/tests/foundation/test_extraction.py +++ b/tests/foundation/test_extraction.py @@ -772,6 +772,45 @@ def test_run_selects_registered_default_and_error_engines(self) -> None: with mock.patch('pcapkit.foundation.extraction.PCAPNG_Engine', types.SimpleNamespace(MAGIC_NUMBER=(b'ng!!',))): bad.run() + def _run_fallback(self, engine: 'type') -> 'tuple[list[str], typing.Any, typing.Any]': + """Run an extractor requesting ``engine``; return its EngineWarnings and state.""" + from pcapkit.utilities.warnings import EngineWarning + + extractor = self._bare_extractor() + extractor._exnam = 'fake' + extractor.__engine__ = {'fake': engine} + extractor._magic = b'pcap' + extractor.record_frames = mock.Mock() + fake_pcap = type('FakePCAP', (FakeEngine,), {'MAGIC_NUMBER': (b'pcap',)}) + with mock.patch('pcapkit.foundation.extraction.PCAP_Engine', fake_pcap): + with mock.patch('pcapkit.foundation.extraction.PCAPNG_Engine', + types.SimpleNamespace(MAGIC_NUMBER=(b'ng!!',))): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + extractor.run() + messages = [str(w.message) for w in caught if issubclass(w.category, EngineWarning)] + return messages, extractor, fake_pcap + + def test_missing_engine_package_warns_once_and_falls_back(self) -> None: + # The module does not exist, so the result is independent of what CI installs. + absent = type('AbsentEngine', (FakeEngine,), { + 'name': 'Absent', 'module': 'missing_engine_module_for_unit_tests'}) + messages, extractor, fake_pcap = self._run_fallback(absent) + self.assertEqual(messages, ['engine Absent (`missing_engine_module_for_unit_tests`) ' + 'is not installed; using default engine instead']) + self.assertEqual(extractor._exnam, 'default') + self.assertIsInstance(extractor._exeng, fake_pcap) + + def test_unsupported_engine_warns_once_and_falls_back(self) -> None: + blocked = type('BlockedEngine', (FakeEngine,), { + 'name': 'Blocked', + 'unsupported_reason': classmethod(lambda cls: 'unit test says no')}) + messages, extractor, fake_pcap = self._run_fallback(blocked) + self.assertEqual(messages, ['engine Blocked is not supported on this interpreter ' + '(unit test says no); using default engine instead']) + self.assertEqual(extractor._exnam, 'default') + self.assertIsInstance(extractor._exeng, fake_pcap) + def test_record_header_record_frames_iteration_call_and_cleanup(self) -> None: from pcapkit.foundation.extraction import Extractor from pcapkit.utilities.exceptions import CallableError, FormatError, IterableError From cb3a4fd704a5b7f76af38f6fc6f7428f5edad2f7 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 5 Oct 2026 15:49:42 -0400 Subject: [PATCH 2/3] fix(extraction): name the module once when import_test has no display name (#1045) import_test printed engine foo (`foo`) when called without a display name. The (`module`) suffix is now emitted only when name is given. Adds a regression test (failed before the fix: 2 != 1), a Fixed changelog entry, regenerated CHANGELOG.md, and moves the entry-count pin 162 -> 163. --- CHANGELOG.md | 3 ++- docs/source/changelog/1.5.0.rst | 11 ++++++++++- docs/source/contributing/conventions/process.rst | 2 +- pcapkit/foundation/extraction.py | 3 ++- tests/foundation/test_extraction.py | 12 ++++++++++++ 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecc73b998..e9e085b21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,7 +75,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) an #### Added - three extraction engines: `engine='pypcap'` and `engine='pcap_ct'`, two independent distributions of the same `libpcap` interface, and `engine='pypcapfile'` ([#386](https://github.com/JarryShaw/PyPCAPKit/pull/386), [#405](https://github.com/JarryShaw/PyPCAPKit/pull/405)). They buy speed by doing less -- neither `pypcap` nor `pcap_ct` dissects at all, so they offer no reassembly or flow tracing, and `pypcapfile` has no IPv6 decoder. Install only **one** of `pypcap` and `pcap-ct`: both own the top-level `pcap` module, and with both present `pcap-ct` wins the import and the other becomes unselectable. The missing interface constants `PyPCAP`, `PCAP_CT` and `PyPCAPFile` are now exported alongside `DPKT`, `Scapy`, `PyShark` and `PCAPKit` ([#412](https://github.com/JarryShaw/PyPCAPKit/pull/412)). That makes seven built-in engines; 3.11 is the last interpreter on which every one can run, and even there two cannot coexist. -- `EngineBase.unsupported_reason`, a preflight every engine answers and `Extractor.run` consults before anything is imported. An engine that cannot run in the current environment now gives one warning naming the real cause (a Python version, a missing `tshark` or `libpcap`, the wrong `pcap` distribution) and a clean fall back to `pcapkit`'s own parser, rather than an error from inside the third-party package ([#396](https://github.com/JarryShaw/PyPCAPKit/pull/396), [#405](https://github.com/JarryShaw/PyPCAPKit/pull/405)). +- `EngineBase.unsupported_reason`, a preflight every engine answers and `Extractor.run` consults before anything is imported. An engine that cannot run in the current environment now gives one warning naming the real cause (a Python version, a missing `tshark` or `libpcap`, the wrong `pcap` distribution) and a clean fall back to `pcapkit`'s own parser, rather than an error from inside the third-party package ([#396](https://github.com/JarryShaw/PyPCAPKit/pull/396), [#405](https://github.com/JarryShaw/PyPCAPKit/pull/405)). The same holds for an engine whose package is not installed at all ([#1045](https://github.com/JarryShaw/PyPCAPKit/issues/1045)). - `conflict` on the reassembly data models: absolute, inclusive ranges where two fragments claimed the same span with different bytes, previously lost silently on both the IP ([#482](https://github.com/JarryShaw/PyPCAPKit/pull/482)) and TCP ([#443](https://github.com/JarryShaw/PyPCAPKit/issues/443), [#478](https://github.com/JarryShaw/PyPCAPKit/pull/478)) paths. #### Changed @@ -101,6 +101,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) an - `register_protocol`'s overwrite warning could claim a protocol was replaced with itself. The guard was correct (`incumbent is not protocol`, an identity check from [#681](https://github.com/JarryShaw/PyPCAPKit/pull/681)), but the message built both operands with a bare `repr()`. A factory defining a same-named closure-local class on every call (as `tests/protocols/test_construction_keyword_check_unit.py`'s `_protocol_class` does) produces two distinct classes sharing one `__module__` and `__qualname__`, so a real overwrite read "overwriting X with X." The message now compares the two reprs and, only when they coincide, appends each object's `id()`. `__module__`/`__qualname__` was rejected as the disambiguator: for the reported shape they are exactly what the coinciding repr already renders. The new `test_register_protocol_disambiguates_classes_sharing_a_repr` fails against the unfixed message ([#710](https://github.com/JarryShaw/PyPCAPKit/issues/710)). - five more registrars warned on mere key presence rather than an actual overwrite, outside the wording of [#718](https://github.com/JarryShaw/PyPCAPKit/issues/718)'s identity guard (`incumbent is not None and incumbent is not new`, landed by [#726](https://github.com/JarryShaw/PyPCAPKit/pull/726)), whose issue named only code-keyed registrars: `register_engine`, `register_reassembly` and `register_traceflow` on `Extractor`, and `register_dumper` on both `Extractor` and `TraceFlow`. All five now compare the incumbent by identity before warning; both `register_dumper` sites compare only the stored dumper, so re-registering with just a new file extension stays silent too, judged defensible rather than comparing the full `(dumper, ext)` pair. Non-breaking: a correct caller sees strictly fewer warnings and no change to return value or exception. Five new tests pin the silent/warns-anyway split ([#739](https://github.com/JarryShaw/PyPCAPKit/issues/739)). - **six registrars** now raise `RegistryError` for a non-class argument, where a bare `TypeError` ("issubclass() arg 1 must be a class") used to escape -- from the guard itself for the two `register_dumper` methods, and from inside `abc` for the other four. They are `register_dumper`, `register_engine`, `register_reassembly` and `register_traceflow` on `Extractor`, `TraceFlow.register_dumper` (reached through `register_traceflow_dumper`), and `pcapkit.foundation.registry.protocols.register_protocol`. Each guard gains an explicit `isinstance(x, type)` test ahead of its `issubclass`; in the five that accept a `ModuleDescriptor` it runs after the descriptor is unwrapped, so a descriptor naming a non-class attribute is rejected too, while `register_protocol` takes no descriptor. No guard's target class changes, and a wrong class still raises `RegistryError` as before. `RegistryError` subclasses `TypeError`, so `except TypeError` still catches it; a caller matching the exact type, or the old message, does not. **Not every site is covered**: six of the thirteen bare `issubclass` guards in the package are fixed here, and the same guard remains in the `register` classmethods of `ProtocolBase`, `Frame`, `PCAPNG`, `SCTP`, `Link`, `Internet` and `Transport`, all of which still leak `TypeError` for a non-class. `Transport.register` is reachable despite its `UnsupportedCall`, which is gated on `cls is Transport` and so fires only for the abstract class -- the guard below it leaks through `TCP.register` and `UDP.register`, which is the only way it is ever called. [#1026](https://github.com/JarryShaw/PyPCAPKit/issues/1026) tracks all seven ([#1021](https://github.com/JarryShaw/PyPCAPKit/issues/1021)). +- a missing extraction engine package now gives one `EngineWarning` rather than two ([#1045](https://github.com/JarryShaw/PyPCAPKit/issues/1045), [#1046](https://github.com/JarryShaw/PyPCAPKit/pull/1046)). `Extractor.import_test` warned that the package was absent, then `Extractor.run` warned again when it fell back to the default engine; `run` now stays silent there. The message text changed. It was `extraction engine '' not available; using default engine instead` and is now `engine () is not installed; using default engine instead`. An `import_test` call with no display name names the module once, not twice. Code that matches the old wording, or expects two warnings, must be updated. ### pcapkit.protocols diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 418bc946b..72688ba01 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -764,7 +764,8 @@ Added run in the current environment now gives one warning naming the real cause (a Python version, a missing ``tshark`` or ``libpcap``, the wrong ``pcap`` distribution) and a clean fall back to ``pcapkit``'s own parser, rather than an - error from inside the third-party package (:pr:`396`, :pr:`405`). + error from inside the third-party package (:pr:`396`, :pr:`405`). The same holds + for an engine whose package is not installed at all (:issue:`1045`). * ``conflict`` on the reassembly data models: absolute, inclusive ranges where two fragments claimed the same span with different bytes, previously lost silently on both the IP (:pr:`482`) and TCP (:issue:`443`, :pr:`478`) paths. @@ -1029,6 +1030,14 @@ Fixed is gated on ``cls is Transport`` and so fires only for the abstract class -- the guard below it leaks through ``TCP.register`` and ``UDP.register``, which is the only way it is ever called. :issue:`1026` tracks all seven (:issue:`1021`). +* a missing extraction engine package now gives one ``EngineWarning`` rather than two + (:issue:`1045`, :pr:`1046`). ``Extractor.import_test`` warned that the package was + absent, then ``Extractor.run`` warned again when it fell back to the default engine; + ``run`` now stays silent there. The message text changed. It was + ``extraction engine '' not available; using default engine instead`` and is + now ``engine () is not installed; using default engine instead``. + An ``import_test`` call with no display name names the module once, not twice. Code + that matches the old wording, or expects two warnings, must be updated. pcapkit.protocols ----------------- diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index ef5c66b49..c7691d066 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -101,7 +101,7 @@ commands down instead of a figure that will be stale by the next merge: The grouping scheme was settled on :issue:`918`: **a section per top-level module, with** ``Added``/``Changed``/``Fixed`` **nested inside each** -- module granularity, not per-file and not per-subpackage. The file carries **9** module-level sections holding -162 entries, and no entry carries an inline kind label:: +163 entries, and no entry carries an inline kind label:: $ grep -cE '^\* \*\*(Added|Changed|Fixed)\*\*' docs/source/changelog/1.5.0.rst 0 diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index f6c342a01..ce5cfce1f 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -734,7 +734,8 @@ def import_test(engine: 'str', *, name: 'Optional[str]' = None) -> 'Optional[Mod except ImportError: module = None logger.debug('engine module %r is not importable', engine) - warn(f'engine {name or engine} (`{engine}`) is not installed; ' + label = f'{name} (`{engine}`)' if name else engine + warn(f'engine {label} is not installed; ' 'using default engine instead', EngineWarning, stacklevel=stacklevel()) return module diff --git a/tests/foundation/test_extraction.py b/tests/foundation/test_extraction.py index f3816e886..47e4fd90e 100644 --- a/tests/foundation/test_extraction.py +++ b/tests/foundation/test_extraction.py @@ -801,6 +801,18 @@ def test_missing_engine_package_warns_once_and_falls_back(self) -> None: self.assertEqual(extractor._exnam, 'default') self.assertIsInstance(extractor._exeng, fake_pcap) + def test_import_test_without_display_name_names_module_once(self) -> None: + from pcapkit.foundation.extraction import Extractor + from pcapkit.utilities.warnings import EngineWarning + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + self.assertIsNone(Extractor.import_test('definitely_missing_mod')) + messages = [str(w.message) for w in caught if issubclass(w.category, EngineWarning)] + self.assertEqual(len(messages), 1) + self.assertEqual(messages[0].count('definitely_missing_mod'), 1) + self.assertNotIn('(`definitely_missing_mod`)', messages[0]) + def test_unsupported_engine_warns_once_and_falls_back(self) -> None: blocked = type('BlockedEngine', (FakeEngine,), { 'name': 'Blocked', From 0b0bf7ab11377ec4d1eb55086cd2a817c7e1a324 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 5 Oct 2026 15:58:20 -0400 Subject: [PATCH 3/3] docs(changelog): correct the #1045 entry's scope (#1045) Drop the claim that an unnamed import_test call named the module twice (only true in this PR's first commit, never shipped), and narrow who breaks: the surviving text equals the warning run() already emitted on main. --- CHANGELOG.md | 2 +- docs/source/changelog/1.5.0.rst | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9e085b21..cd5ef2e7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,7 +101,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) an - `register_protocol`'s overwrite warning could claim a protocol was replaced with itself. The guard was correct (`incumbent is not protocol`, an identity check from [#681](https://github.com/JarryShaw/PyPCAPKit/pull/681)), but the message built both operands with a bare `repr()`. A factory defining a same-named closure-local class on every call (as `tests/protocols/test_construction_keyword_check_unit.py`'s `_protocol_class` does) produces two distinct classes sharing one `__module__` and `__qualname__`, so a real overwrite read "overwriting X with X." The message now compares the two reprs and, only when they coincide, appends each object's `id()`. `__module__`/`__qualname__` was rejected as the disambiguator: for the reported shape they are exactly what the coinciding repr already renders. The new `test_register_protocol_disambiguates_classes_sharing_a_repr` fails against the unfixed message ([#710](https://github.com/JarryShaw/PyPCAPKit/issues/710)). - five more registrars warned on mere key presence rather than an actual overwrite, outside the wording of [#718](https://github.com/JarryShaw/PyPCAPKit/issues/718)'s identity guard (`incumbent is not None and incumbent is not new`, landed by [#726](https://github.com/JarryShaw/PyPCAPKit/pull/726)), whose issue named only code-keyed registrars: `register_engine`, `register_reassembly` and `register_traceflow` on `Extractor`, and `register_dumper` on both `Extractor` and `TraceFlow`. All five now compare the incumbent by identity before warning; both `register_dumper` sites compare only the stored dumper, so re-registering with just a new file extension stays silent too, judged defensible rather than comparing the full `(dumper, ext)` pair. Non-breaking: a correct caller sees strictly fewer warnings and no change to return value or exception. Five new tests pin the silent/warns-anyway split ([#739](https://github.com/JarryShaw/PyPCAPKit/issues/739)). - **six registrars** now raise `RegistryError` for a non-class argument, where a bare `TypeError` ("issubclass() arg 1 must be a class") used to escape -- from the guard itself for the two `register_dumper` methods, and from inside `abc` for the other four. They are `register_dumper`, `register_engine`, `register_reassembly` and `register_traceflow` on `Extractor`, `TraceFlow.register_dumper` (reached through `register_traceflow_dumper`), and `pcapkit.foundation.registry.protocols.register_protocol`. Each guard gains an explicit `isinstance(x, type)` test ahead of its `issubclass`; in the five that accept a `ModuleDescriptor` it runs after the descriptor is unwrapped, so a descriptor naming a non-class attribute is rejected too, while `register_protocol` takes no descriptor. No guard's target class changes, and a wrong class still raises `RegistryError` as before. `RegistryError` subclasses `TypeError`, so `except TypeError` still catches it; a caller matching the exact type, or the old message, does not. **Not every site is covered**: six of the thirteen bare `issubclass` guards in the package are fixed here, and the same guard remains in the `register` classmethods of `ProtocolBase`, `Frame`, `PCAPNG`, `SCTP`, `Link`, `Internet` and `Transport`, all of which still leak `TypeError` for a non-class. `Transport.register` is reachable despite its `UnsupportedCall`, which is gated on `cls is Transport` and so fires only for the abstract class -- the guard below it leaks through `TCP.register` and `UDP.register`, which is the only way it is ever called. [#1026](https://github.com/JarryShaw/PyPCAPKit/issues/1026) tracks all seven ([#1021](https://github.com/JarryShaw/PyPCAPKit/issues/1021)). -- a missing extraction engine package now gives one `EngineWarning` rather than two ([#1045](https://github.com/JarryShaw/PyPCAPKit/issues/1045), [#1046](https://github.com/JarryShaw/PyPCAPKit/pull/1046)). `Extractor.import_test` warned that the package was absent, then `Extractor.run` warned again when it fell back to the default engine; `run` now stays silent there. The message text changed. It was `extraction engine '' not available; using default engine instead` and is now `engine () is not installed; using default engine instead`. An `import_test` call with no display name names the module once, not twice. Code that matches the old wording, or expects two warnings, must be updated. +- a missing extraction engine package now gives one `EngineWarning` rather than two ([#1045](https://github.com/JarryShaw/PyPCAPKit/issues/1045), [#1046](https://github.com/JarryShaw/PyPCAPKit/pull/1046)). `Extractor.import_test` warned that the package was absent, then `Extractor.run` warned again when it fell back to the default engine; `run` now stays silent there. The surviving message is the one `run` already emitted, `engine () is not installed; using default engine instead` (the module name is in backticks in the real message, which this changelog's Markdown generator cannot show literally), so `import_test` now says the same. Only code that matched `import_test`'s old wording, `extraction engine '' not available; using default engine instead`, or that expects two warnings, breaks. ### pcapkit.protocols diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 72688ba01..26d8eee95 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -1033,11 +1033,13 @@ Fixed * a missing extraction engine package now gives one ``EngineWarning`` rather than two (:issue:`1045`, :pr:`1046`). ``Extractor.import_test`` warned that the package was absent, then ``Extractor.run`` warned again when it fell back to the default engine; - ``run`` now stays silent there. The message text changed. It was - ``extraction engine '' not available; using default engine instead`` and is - now ``engine () is not installed; using default engine instead``. - An ``import_test`` call with no display name names the module once, not twice. Code - that matches the old wording, or expects two warnings, must be updated. + ``run`` now stays silent there. The surviving message is the one ``run`` already + emitted, ``engine () is not installed; using default engine instead`` + (the module name is in backticks in the real message, which this changelog's + Markdown generator cannot show literally), so ``import_test`` now says the same. + Only code that matched ``import_test``'s old wording, + ``extraction engine '' not available; using default engine instead``, or + that expects two warnings, breaks. pcapkit.protocols -----------------