fix(corekit,utilities): raise pcapkit exceptions from EnumLookup.get, following stdlib Enum's shape

The maintainer's ruling on #921, verbatim:

Either ValueError or KeyError, that's depending on how stdlib's Enum would raise on these circumstances. And we should raise one from pcapkit.utilities.exceptions rather builtin exceptions.

What stdlib does, measured on Python 3.14.7:

call	stdlib `Enum`
`E['nosuch']` — name miss	`KeyError: 'nosuch'`
`E(999)` — value miss	`ValueError: 999 is not a valid E`
`E(None)`	`ValueError: None is not a valid E`

So the shape the ruling selects is: a name miss is KeyError-derived, a value miss is ValueError-derived — and both must come from pcapkit.utilities.exceptions, not from builtins.

Where the library stands against that. EnumLookup.get (pcapkit/corekit/enum.py:395) surfaces a bare builtin KeyError on a name miss and a bare builtin ValueError on a value miss, so it has the right shape and the wrong provenance. Two classes work around it by converting: TransportProtocol.get and Criticality.get both catch the base's KeyError and re-raise ValueError naming the key, which under this ruling is the wrong direction — a name miss should stay KeyError-shaped.

A gap that has to be filled first. pcapkit/utilities/exceptions.py already has the value half: EnumValueError(BaseError, ValueError) at line 374. It has no KeyError-derived class at all — every member of that module derives from TypeError, AttributeError, ValueError, IOError, IndexError, FileExistsError or FileNotFoundError. So the name-miss half needs a new exception, something like EnumKeyError(BaseError, KeyError), before get can comply.

Scope

Add the missing KeyError-derived exception to pcapkit/utilities/exceptions.py and its __all__.
Make EnumLookup.get raise the two pcapkit.utilities.exceptions classes instead of builtins, keeping the stdlib shape — name miss KeyError-derived, value miss ValueError-derived. Both stay catchable as the builtins they derive from, so existing except KeyError / except ValueError call sites keep working.
Remove the now-wrong conversion from TransportProtocol.get and Criticality.get. Criticality.get disappears entirely at that point; TransportProtocol.get reduces to the .lower() call and nothing else.
Update the docstrings and docs/source/contributing/conventions.rst's registry-protocol section, which documents the current exception contract.

Blast radius is real and not yet fully measured. This changes the contract for every concrete EnumLookup subclass. A census is running to establish how many already surface ValueError versus KeyError on a name miss and what in pcapkit/ and tests/ catches KeyError around a .get( call; I will post the counts here when it lands.

Blocked on #921 and #922 merging. Both touch pcapkit/corekit/enum.py or its subclasses and both are review: good-to-go; starting here first would collide with them.

Census landed, and it settles the direction: the base already has the right shape. Only its provenance is wrong.

Re-derived by me independently of the agent that ran it, on 3766c3c09. Both counts agree exactly.

127 concrete EnumLookup subclasses (vendor codegen templates and the two memberless bases excluded — the figure matches pcapkit/corekit/enum.py's own supporting comment). On a string name miss:

outcome	count
`KeyError`	119
`ValueError`	5
returns (mints)	3

The 5 are AppType and its four transport subclasses TCP/UDP/SCTP/DCCP in pcapkit/const/reg/apptype/, and they land there only because their own get() takes an int port and never accepts a string at all — not from a name-miss policy. The 3 that return are Command, Method and OptionType, all of which mint.

So ValueError-on-name-miss is a 5-of-127 minority. EnumLookup.get must not change its exception shape — it already matches stdlib, and converting would overturn 119 classes to match 5. What changes is only that the exceptions come from pcapkit.utilities.exceptions. That narrows this issue's scope, it does not widen it.

The one design point that decides the cost. If EnumKeyError derives from KeyError — as MissingKeyError already does at pcapkit/utilities/exceptions.py:459 — every existing except KeyError keeps catching and the blast radius collapses to nearly nothing. Measured, the sites that would otherwise break: pcapkit/toolkit/scapy.py:342, hopopt.py:806 and :1040, ipv6_opts.py:809 and :1043, and const/http/method.py:297. That last one matters most — Method.get catches the base's KeyError in order to mint, so it is wired directly to the current contract. There are also 28 assertRaises(KeyError) assertions across 8 test files, about 20 of them against .get( on a real registry, including tests/corekit/test_enum_lookup_base_unit.py which pins the base's contract directly and tests/const/test_const_registry_protocol.py:1258-1261 which states it in prose.

One more item for the scope, found by the census. pcapkit/protocols/internet/mh.py:648 and :740 already do except KeyError: raise EnumValueError(...) on a name miss — the right provenance but the minority shape, and the same conversion this ruling rejects. Those two should become KeyError-derived along with the base.

A defect worth its own note: Command.get (pcapkit/const/ftp/command.py:485) unconditionally calls key.upper(), so a non-str key raises AttributeError rather than any enum exception. On the value-miss path that is simply wrong, and it is in neither shape.

Scope caveat: the census measured origin/main, where TransportProtocol and Criticality are not yet EnumLookup subclasses — #921 is what reparents them. Their two overrides' KeyError → ValueError conversion is confirmed as the minority behaviour by these numbers, so deleting it stays in scope here.

Recording the precise end state of the two overrides, measured on #921's branch, so it is not rediscovered here.

TransportProtocol.get survives this issue, reduced to the .lower() call and nothing else. Case folding is not in the base and is live public behaviour — get('TCP') resolves today and raises KeyError without the override.
Criticality.get can be deleted outright. Once the KeyError → ValueError conversion goes, its body is a pure pass-through. I checked the one thing that could make deletion unsafe — default handling — and the inherited base is identical to the override's forwarding:

inherited base:  get('nosuch')           -> KeyError: 'nosuch'
                 get('nosuch', <member>) -> <Criticality.reject: 0>
                 get('nosuch', 99)       -> KeyError   (a non-member default is not honoured)
override today:  get('nosuch', <member>) -> <Criticality.reject: 0>   (identical)

The only loss is the narrower int | str | Criticality key annotation, a typing nicety rather than behaviour. Deleting it is therefore safe on a closed ASN.1 ENUMERATED: a member-valued default resolves through _value2member_map_ and never through the constructor, so it still cannot mint a fourth value.

Half-unblocked: #921 merged as 5a4f8ca59. Still blocked on #922.

This issue was gated on both open PRs that touch pcapkit/corekit/enum.py or its subclasses. #921 has landed, so the TransportProtocol and Criticality re-parenting this work builds on is now on main — verified, all four spot-checked classes have EnumLookup in their MRO on 5a4f8ca59.

#922 remains the blocker. It touches pcapkit/corekit/enum.py directly (the NO_DEFAULT/NoDefaultType definitions move out to pcapkit/corekit/sentinels.py), which is the same file this issue changes. It is review: good-to-go at 16f3a05fb and held only by a reclaimed CI runner needing a re-run.

Worth noting for whoever picks this up: once #922 lands, pcapkit/corekit/sentinels.py is where NO_DEFAULT lives, so the new EnumKeyError work here should import it from there rather than from enum.py — and docs/source/pcapkit/corekit/enum.rst, added by #919, will need its autoclass/autodata targets repointed for NoDefaultType and NO_DEFAULT. That doc fix belongs to neither PR and is currently unowned.

Unblocked — both blockers merged. #921 landed as 5a4f8ca59 and #922 as c7342cac1.

Two things this issue must now account for, both consequences of #922 rather than new work:

NO_DEFAULT and NoDefaultType now live in pcapkit/corekit/sentinels.py, re-exported from pcapkit/corekit/enum.py. The new EnumKeyError work should import from the canonical module, not the shim.
docs/source/pcapkit/corekit/enum.rst — added by docs: add pcapkit.__version__ entry and the missing corekit.enum page #919 — still points its autoclass/autodata at pcapkit.corekit.enum for those two names. That page needs repointing, it belongs to neither merged PR, and it is currently unowned. Doing it here is the natural home since this issue already changes that module's contract.

The census recorded above still stands: 119 of 127 concrete EnumLookup subclasses raise KeyError on a name miss, so the base keeps its stdlib shape and only its provenance changes. Removing blocked.

docs(conventions): harvest the settled rulings, correct the stale pro…

refactor(const,protocols): reparent the last seven non-registry enums…

docs(conventions,const): resolve part B's unresolved cross-references (…

docs(pcapkit,ci): cite the issue a defect belongs to, not the pull re…

docs(corekit): state the enum.py rulings as statements, not quotations

docs(protocols): state the rulings behind mh, vendor and exceptions i…

docs(corekit): state the enum.py rulings as statements, not quotations

docs(const,vendor): state the generated files' rulings instead of quo…

Navigation Menu

Repository navigation

fix(corekit,utilities): raise pcapkit exceptions from EnumLookup.get, following stdlib Enum's shape #923

Scope

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

fix(corekit,utilities): raise pcapkit exceptions from EnumLookup.get, following stdlib Enum's shape #923

Description

Scope

Activity

JarryShaw commented on Sep 29, 2026

JarryShaw commented on Sep 29, 2026

JarryShaw commented on Sep 29, 2026

JarryShaw commented on Sep 29, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions