fix(foundation): raise RegistryError, not a leaked TypeError, for a non-class

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible

What is the purpose of your pull request?

• 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
• chore — anything else

---

Description of your pull request and other information

Closes #1021, implementing the maintainer's choice of "code follows prose".

Six registrar guards promised RegistryError for a non-class and leaked abc's TypeError instead. Each was a bare if not issubclass(x, Base):, so a non-class was rejected before the raise was reached. Measured with the raise site read off the traceback rather than the message, because the message is identical in both cases:

Extractor.register_dumper      -> TypeError | extraction.py:406
Extractor.register_engine      -> TypeError | <frozen abc>:123
Extractor.register_reassembly  -> TypeError | <frozen abc>:123
Extractor.register_traceflow   -> TypeError | <frozen abc>:123
TraceFlow.register_dumper      -> TypeError | traceflow.py:237
register_protocol              -> TypeError | <frozen abc>:123

Two mechanisms. Dumper's metaclass is plain type, so the builtin issubclass raises in place at the guard line; the other targets carry ABCMeta-derived metaclasses, so the builtin delegates and the raise happens inside abc.

TraceFlow.register_dumper matters beyond symmetry, and was missed in the first revision. It backs the exported register_traceflow_dumper, whose sibling register_extractor_dumper has an identical signature and docstring thirty lines away in the same file — so fixing only the Extractor side left two indistinguishable public functions raising different exceptions.

The check goes after the ModuleDescriptor unwrap in the five registrars that accept one, because that branch unwraps rather than short-circuits, so a descriptor naming a non-class attribute reaches it. register_protocol accepts no descriptor and has no unwrap.

Not breaking. RegistryError subclasses TypeError via BaseError:

RegistryError MRO: ['RegistryError', 'BaseError', 'TypeError', 'Exception', 'BaseException', 'object']

So except TypeError keeps working, and only a caller matching the exact type or the old message text sees a difference. No guard's target class changes — that is #1016's subject and PR #1020's work. A wrong class still raises RegistryError, and the tests pin that it is unchanged.

No docstring changed. The four Raises: clauses already read "is not a class, or not a X subclass"; with code following prose they are simply true.

Scope, stated honestly: six of thirteen bare issubclass guards in pcapkit/. The register classmethods on ProtocolBase, Frame, PCAPNG, SCTP, Link, Internet and Transport all still leak, and #1026 tracks them. An earlier draft called Transport.register exempt — that was wrong. Its UnsupportedCall is gated on cls is Transport, so it fires only for the abstract class; the guard below it is reachable and leaks through TCP.register and UDP.register, which is the only way it is ever called.

New tests cover all six registrars across non-class inputs — instance, string, None, and a descriptor resolving to a non-class — plus the wrong-class case, asserting the message so a stray TypeError cannot pass silently. On main they produce 28 subfailures, every one the leaked TypeError.

tests/foundation + tests/project: 536 passed, 13 skipped, 1319 subtests passed. util/changelog_md.py --check exits 0, and the process.rst entry-count pin is 157, measured rather than incremented.

[JarryShaw]

Cross-review verdict: NEEDS CHANGES (ran on Opus; authored on Sonnet). It found a real gap and three errors in my own prose. All four reproduced by me before acting.

The gap, and it is a public-API asymmetry. TraceFlow.register_dumper (pcapkit/foundation/traceflow/traceflow.py:237) carries a character-identical guard after the same ModuleDescriptor unwrap, and this PR left it out:

register_dumper            -> RegistryError  | extraction.py:407
register_extractor_dumper  -> RegistryError  | extraction.py:407
register_traceflow_dumper  -> TypeError      | traceflow.py:237   <-- still leaks

All three are exported from pcapkit/foundation/registry/__init__.py's __all__, and register_extractor_dumper / register_traceflow_dumper have identical signatures and identical docstrings about thirty lines apart. After this PR one raises the registry's exception and the other leaks abc's. Being fixed, with a test, and the existing dumper test extended to cover both — their being indistinguishable on paper is precisely why nothing caught the divergence.

Three corrections to my own text:

• "It is five registrars, not the four the issue named" is wrong, and the mistake is mine twice over. The four register_* registrars promise RegistryError for a non-class input and raise TypeError instead #1021's body already says five; I wrote that issue, and the PR then framed its own count as correcting an error the issue did not make. Dropping that framing.
• The changelog's "placed after the ModuleDescriptor unwrap" is false for register_protocol, which has no unwrap at all — its guard is the first statement after the docstring.
• My merge-order note was wrong about fix(foundation): name the base class the registrar guard actually checks #1020. Its pushed head touches no changelog file, so it cannot collide on the process.rst pin; only its unpushed rework does. The real risk I failed to flag is that fix(foundation): name the base class the registrar guard actually checks #1020's rework overlaps four of this PR's five non-doc files.

One scope correction that matters more than the rest. grep -rn "if not issubclass" pcapkit/ finds 13 sites. After this round six are fixed. The changelog will say so plainly rather than implying the class of defect is closed — ProtocolBase.register, Frame.register, PCAPNG.register, SCTP.register, Link.register and Internet.register still leak, with Transport.register exempt because it raises UnsupportedCall first. Those are a different surface and get their own issue rather than being swept in here.

Everything else held: the two-mechanism split, the non-breaking claim with no affected except site anywhere in pcapkit/, tests/ or examples/, the unchanged wrong-class path, no scope creep, and 534 passed / 13 skipped / 1298 subtests.

[JarryShaw]

Cross-review verdict: GOOD TO GO at 1f24b9aa9 — round 3, on Opus, the same reviewer that raised both earlier rounds.

Both round-2 findings are fixed, and the reviewer established something that makes the carry-forward sound rather than assumed: the round-2 → round-3 diff is prose-only. I verified it — git diff 3427a2873 1f24b9aa9 -- pcapkit/ tests/ docs/source/contributing/ is empty, and the whole diff is two changelog files. So every measurement from round 2 holds by construction.

It also re-derived the scope claim two ways rather than taking it on trust. grep -rn "if not issubclass" pcapkit/ gives 13 distinct file:line pairs, six fixed here and seven remaining, nothing double-counted — and it walked each remaining site back to its enclosing def to confirm all seven really are register classmethods, rather than assuming the description's phrasing. It then widened past that grep pattern in case the pattern was the blind spot, and found two further unguarded issubclass calls that are not this defect class: corekit/fields/numbers.py:772-774, whose docstring already documents TypeError as the contract, and dumpkit/common.py:247, which is internal with no public registrar. Every other issubclass in the package already pairs with an isinstance(x, type) — so the shape this PR adopts is the one the codebase already uses elsewhere.

One defect it found outside this PR, now fixed: #1026's body still listed six sites and still asserted the Transport.register exemption, so a reader following this PR's pointer landed on the claim this PR refutes. Its title and my comment were right; the body had never been edited. Corrected.

CI on this head: 64 success, 3 skipped, remainder in flight, nothing failed. MERGEABLE but BEHIND — the round-2 test-merge into fa3e861f5 was clean with the pin landing at 157 and both new label definitions surviving.

[JarryShaw]

resolve conflicts

[JarryShaw]

Conflicts resolved and rebased onto main 51100da7e. New head 39a4c4d8f, still one commit.

The conflict was semantic rather than textual, so the resolution is worth stating: #1020 merged in the meantime and deliberately narrowed these three guards from the *Base classes to the public Engine / Reassembly / TraceFlow, routing the built-ins through _register_internal_* instead. This branch had been written against the older, wider targets. Taking its side verbatim would have silently reverted #1020.

So in each of the three registrars I kept main's narrowed target and its #1016 note, and added only this branch's isinstance(x, type) test ahead of the issubclass. Verified by AST which function owns each remaining check:

function	guard
register_engine :454/:461	isinstance(…, type) then issubclass(…, Engine)
register_reassembly :527/:532	then issubclass(…, Reassembly)
register_traceflow :598/:603	then issubclass(…, TraceFlow)
_register_internal_engine :488	issubclass(…, EngineBase) — unchanged
_register_internal_reassembly :559	issubclass(…, ReassemblyBase) — unchanged
_register_internal_traceflow :630	issubclass(…, TraceFlowBase) — unchanged

main's docstrings already promised "not a class, or not an Engine subclass", so this supplies the half that was documented but unreachable.

The tests/foundation/test_extraction.py conflict was two different tests landing at the same spot — #1016's test_public_register_guards_take_public_class_and_internal_path_takes_base and this branch's test_register_helpers_reject_a_non_class_with_registry_error. Both kept.

Two corrections made while resolving:

• The changelog entry opened "a breaking change to", which contradicts both this PR's label set and the ruling on The four register_* registrars promise RegistryError for a non-class input and raise TypeError instead #1021 that it is not breaking — RegistryError subclasses TypeError, so except TypeError still catches it. Reworded to lead with what actually changed; the narrow caveat about matching the exact type or the old message is kept, since that part is true.
• The process.rst pin. It now reads the measured 159. Separately, main was red before this: fix(foundation): name the base class the registrar guard actually checks #1020 and docs(foundation): document the engine members on EngineBase, not Engine (#1024) #1027 each measured 157 against a 156-entry base — correct for each alone — and merging both made 158 while the pin stayed 157, so test_the_page_pins_its_own_measured_numbers was failing. Fixed directly on main in 51100da7e as a docs-only change.

Verified on 39a4c4d8f: tests/project 268 passed, 1 skipped, 864 subtests; tests/foundation 270 passed, 12 skipped, 464 subtests (up from main's 265, the new tests). 16 subtests of the new test fail when extraction.py and registry/protocols.py are reverted to main, and #1016's test passes either way.

Resetting review: to pending since the head moved; a fresh cross-review is dispatched on a model different from the one that resolved this.

[JarryShaw]

Cross-review verdict on 39a4c4d8f: GOOD TO GO (ran on Sonnet; I authored the conflict resolution myself, so the reviewing model differs from the resolving one).

It went after the resolution specifically — a semantic conflict resolved by hand is exactly where someone else's merged work gets silently reverted — and could not break it. What it derived independently rather than taking from me:

• fix(foundation): name the base class the registrar guard actually checks #1020 is intact. Public guards still check Engine / Reassembly / TraceFlow (:461, :532, :603); internal ones still check the *Base (:488, :559, :630). A class deriving only from EngineBase is still refused at the public door with the public-class message.
• The isinstance test is on the right side of the ModuleDescriptor unwrap. Probed with ModuleDescriptor('os', 'sep') — a descriptor naming a non-class attribute — and all five descriptor-accepting registrars raise RegistryError naming the unwrapped value, with raise sites at extraction.py:413/:455/:528/:599 and traceflow.py:238.
• Both tests survived the conflict, at :298 and :409.
• The entry's own census checks out. 16 not issubclass sites in pcapkit/ minus the 3 internal guards leaves 13; 6 fixed here, 7 on the protocol side. And the Transport subtlety holds: Transport.register(1, 5) raises UnsupportedCall at transport.py:110 because cls is Transport, while TCP.register and UDP.register reach the guard below and leak — which is why that gate makes the guard reachable rather than exempt.
• The old TypeError came from two different places, as the entry says: from the call site for the two register_dumper methods, because Dumper's metaclass is plain type; from <frozen abc>:123 for the four whose metaclass derives from ABCMeta.
• MRO for the non-breaking claim: RegistryError → BaseError → TypeError → Exception → BaseException → object, so except TypeError still catches it.
• Pin: 159 measured, 159 in the prose. tests/project + tests/foundation/registry: 290 passed, 1 skipped, 985 subtests.

It also found no seam in the changelog bullet I rewrote and rewrapped in three passes, which was the thing I most expected to be wrong.

One correction to my own briefing, not to this change: I have been telling agents the venv is Python 3.14.7. It is 3.14.8 (Python 3.14.8 (main, Sep 30 2026)), verified directly. No result anywhere depends on it, but I had the number wrong repeatedly.

Its one UNVERIFIED gap — the rest of tests/foundation beyond registry — is covered by my own run on this head: 270 passed, 12 skipped, 464 subtests, against main's 265.