Skip to content

fix(foundation): keep a user's verbose callback on every engine (#1874) - #1880

Merged
JarryShaw merged 6 commits into
mainfrom
fix/1874-engine-verbose-callback
Oct 17, 2026
Merged

JarryShaw merged 6 commits into
mainfrom
fix/1874-engine-verbose-callback

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Stacked on #1867. It contains #1867's commits until that merges, so merge #1867 first. This PR's own commits are 9cbb10e4b (the fix) and e24f384fb (docs and CI).

  • Extractor records in a private _vfunc_default flag whether
    verbose=True asked for the default printer; a callable leaves it unset.
  • The scapy, dpkt, pyshark, pypcap, pcap-ct and pypcapfile engines install
    their own printer only when that flag is set, so a caller's callback is
    no longer replaced. The verbose=True output is unchanged.
  • Note the fix in Extractor's 1.5.0 versionchanged block, and correct the
    _flag_v comment, which said it gated the per-frame call.
  • Test each installed engine end to end on in.pcap, with a recording
    callback and with verbose=True, and add fake-extractor cases for the
    engines that cannot run here.

Closes #1874

Follow-up commit: ext.rst's custom-engine example now tests ext._vfunc_default, _vfunc_default is documented on the extraction page, and the new test module is added to every engine leg's TEST_PATHS (an in-place, line-neutral edit). The PyPCAP and PyPCAPFile end-to-end cases first run in CI, because neither engine builds here.

- Uncomment the `_vfunc` annotation under its `#:` comment, so Sphinx attaches
  the description and the type the extraction page asks for. Typed
  `Callable[[Extractor, Any], Any]` rather than `VerboseHandler`, whose
  `Packet` union makes mypy reject the default and engine callbacks; mypy
  output on the extractor and the engines is unchanged.
- Add a test that the comment attaches, read the way Sphinx's parser reads
  it, plus a cross-check against Sphinx itself where it is installed.

Closes #1839
…1839)

- Type `VerboseHandler` as `Callable[['Extractor', Packet], object]` and
  annotate `_vfunc` with it. The return is `object`, not `None`: every
  caller discards it, and `None` would make mypy reject a user callback
  that returns a value.
- Narrow the frame to the engine's own packet type with `cast` inside the
  default callback and each engine's callback, so they accept the alias's
  `Packet` union. Runtime output is unchanged.
- Pin the annotation and the alias's signature in the #1839 test.

Refs #1839
- Replace `sys.stdout.write`, which takes one argument and so is no verbose
  callback at all, with `lambda e, f: sys.stdout.write(str(f))`: it takes the
  extractor and the frame and returns a value, so `object` accepts it where
  `None` would not.

Refs #1839
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 17, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at e24f384fb. A Sonnet cross-review found no defects.

  • Runtime. On in.pcap, a verbose= callable now runs once per frame on the default, dpkt and Scapy engines. Each call gets (extractor, frame), and the frame is the same object as extractor.frame[i]. On 817208c8a it ran zero times and the engine printed its own lines.
  • No change for bools. verbose=True output is byte-identical to before, and verbose=False prints nothing on either tree.
  • Engines. All six third-party engines now read _vfunc_default. No reader of _flag_v is left in the package.
  • Fail-before. Run against the old pcapkit/, the new tests fail for the right reasons. Reverting any single engine to _flag_v breaks exactly one test.
  • Workflow. The five TEST_PATHS lines differ only by the new module, and the YAML still parses.
  • Tests: tests/foundation tests/cli tests/interface gave 523 passed and 17 skipped; tests/project gave 992 passed and 1 skipped (unrelated).
  • Unverified. Real extraction on PyShark, PyPCAP, PCAP_CT and PyPCAPFile, because those libraries are not installable on 3.14 here. The mock-based tests cover their gates.
  • Note, not a blocker. _flag_v is still set and documented but nothing in the package reads it now. A custom engine written to the old docs keeps its old behaviour.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 17, 2026
- Type `VerboseHandler`'s return as `Any` rather than `object`, per the
  owner's preference on #1867: every caller discards the result, and `Any`
  accepts any two-argument callback, whatever it returns.
- Rewrite the NOTE above the alias to give that reason.
- Pin `Any` in the signature test; the exact comparison still fails on a
  return of `None` or `object`.

Refs #1839
- Extractor records in a private `_vfunc_default` flag whether
  `verbose=True` asked for the default printer; a callable leaves it unset.
- The scapy, dpkt, pyshark, pypcap, pcap-ct and pypcapfile engines install
  their own printer only when that flag is set, so a caller's callback is
  no longer replaced. The `verbose=True` output is unchanged.
- Note the fix in Extractor's 1.5.0 versionchanged block, and correct the
  `_flag_v` comment, which said it gated the per-frame call.
- Test each installed engine end to end on in.pcap, with a recording
  callback and with `verbose=True`, and add fake-extractor cases for the
  engines that cannot run here.

Closes #1874
…ack (#1874)

- The custom-engine example in ext.rst tests `ext._vfunc_default`, not
  `ext._flag_v`, and says why: a `verbose=` callable must be kept.
- Document `Extractor._vfunc_default` on the extraction page.
- Run the verbose-callback module on every engine-tests leg, and update
  the tier guard test that pins the PyShark leg's TEST_PATHS line.

Refs #1874
@JarryShaw
JarryShaw force-pushed the fix/1874-engine-verbose-callback branch from e24f384 to 657ef26 Compare October 17, 2026 02:07
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 17, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 17, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 657ef260f. This is the rebase onto #1867's 5b0e9fdbe.

Merge #1867 first.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 17, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Not following, if there's a user provided callback, how will it be called then? My initial plan is to have user's callback replace the default printer and being called in the printer's place when version flag is set.

@JarryShaw

Copy link
Copy Markdown
Owner Author

That is what this PR does. The user's callback takes the printer's place, and nothing else changes. At 657ef260f:

  • verbose=<callable>. Extractor stores the callback as _vfunc, sets _flag_v = True and sets _vfunc_default = False (extraction.py:1311-1313).
  • verbose=True. It sets _vfunc_default = True and installs the PCAP/PCAP-NG printer (:1295-1307).
  • Every engine calls ext._vfunc(ext, frame) once per frame, for example engines/scapy.py:431 and engines/dpkt.py:530.
  • Third-party engines replace _vfunc with their own printer only when _vfunc_default is set (scapy.py:387), because their frames are not pcapkit Frames.

The bug in #1874 was that check. The engines tested _flag_v, which is also True for a callback, so they overwrote the user's callback with their printer. Now the callback stays in _vfunc and runs where the printer would have run. Measured on in.pcap: a recording callback runs once per frame on the default, dpkt and Scapy engines, where before it never ran on dpkt or Scapy. verbose=True output is byte-identical to before.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 90.05% (unit tier, Python 3.15, 657ef260f, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 2 20 2 96.77%
pcapkit/const 19595 881 2618 867 92.11%
pcapkit/corekit 2462 69 780 35 96.55%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2916 164 1016 34 93.59%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17406 224 4580 189 98.10%
pcapkit/toolkit 872 177 298 8 77.18%
pcapkit/utilities 512 2 158 4 99.10%
pcapkit/vendor 4407 2342 1010 157 43.27%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit eec2c56 into main Oct 17, 2026
57 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 17, 2026
@JarryShaw
JarryShaw deleted the fix/1874-engine-verbose-callback branch October 17, 2026 13:22
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(foundation): third-party engines replace a user's verbose callback with their own printer

1 participant