the generated enum guards raise a bare ValueError in 113 of 117 const modules, and tcp.flags.Flags has no guard at all

[JarryShaw]

The generated _missing_ range guards under pcapkit/const/ raise a bare ValueError, in 113 of the 117 modules. pcapkit.utilities.exceptions.EnumError exists for exactly this and is entirely unused — three references in the whole tree, none of them a raise. And one of the four modules that is exempt from the count is exempt for the worst possible reason: pcapkit/const/tcp/flags.py carries no guard at all, so Flags(-1) returns every defined bit instead of rejecting a value no wire field can hold.

Two defects in one census, filed together because the same sweep produces both and tcp/flags.py appears in each — as an exemption in the first and as the subject of the second.

Measured on 375e9d411, CPython 3.14.7, aenum 3.1.17, tree asserted against the worktree rather than the editable install (pcapkit.__file__ = …/pcapkit/__init__.py printed on every run). Every file named below is byte-identical on current origin/main (da381f259), verified with git diff --stat 375e9d411..origin/main -- <path>, and the census was re-run against both trees and returns the same 113/117 in each — so nothing here is stale against the tree as it stands.

Part 1 — 113 of 117 modules raise a bare ValueError

Counts derived here rather than taken on trust:

find pcapkit/const -name '*.py' -type f ! -name '__init__.py' | wc -l   # 117
grep -rl "raise ValueError" pcapkit/const/ | wc -l                      # 113

117 is 134 .py files less 17 __init__.py. All 117 carry the generated-provenance banner. An AST pass resolving every raise to its enclosing function gives:

modules (excl __init__.py): 117
files with >=1 ValueError raise: 113
total raise sites: 114
  _missing_    Call  113
  get          bare  1

113 is the number to quote, not 114. The 114th, pcapkit/const/reg/apptype.py:30605, is a bare raise ValueError used as internal control flow and caught two lines later, so it never escapes:

            try:
                ret = AppType._missing_(key)
                if ret is None:
                    raise ValueError
            except ValueError:
                ret = extend_enum(AppType, 'PORT_%d_%s' % (key, proto.name), key, 'unknown', proto)

The 113 real sites are byte-identical, one per module, all in _missing_. Verbatim, with the guard above it — pcapkit/const/arp/hardware.py:180-181:

        if not (isinstance(value, int) and 0 <= value <= 65535):
            raise ValueError('%r is not a valid %s' % (value, cls.__name__))

Same line at pcapkit/const/ipv4/option_number.py:143, pcapkit/const/mh/packet.py:125, pcapkit/const/reg/apptype.py:30623, and 109 others. The {FLAG} guard differs per registry — 90 distinct _missing_ shapes — while the raise line does not vary at all.

The four exempt modules, for two different reasons

Module	Why exempt	Bare ValueError still reaches a caller?
pcapkit/const/ftp/command.py	StrEnum; _missing_ auto-extends, no failure path	No
pcapkit/const/http/method.py	StrEnum; _missing_ auto-extends, no failure path	No
pcapkit/const/ipv6/extension_header.py	no _missing_ at all	Yes — raised by aenum
pcapkit/const/tcp/flags.py	no _missing_ at all	No — and that is Part 2

pcapkit/const/http/method.py:182-193 is the first shape: _missing_ ends in return extend_enum(cls, name, value), so there is nothing to raise.

The second shape complicates the headline. pcapkit/const/ipv6/extension_header.py authors no raise, but aenum does it for them:

ExtensionHeader.get(99999) -> RAISED ValueError: 99999 is not a valid ExtensionHeader
                              is BaseError? False   is bare builtin? True

So on observable behaviour 114 of 117 modules leak a bare ValueError, of which 113 are pcapkit's own text and one comes from the dependency and cannot be fixed by a template at all.

Part 2 — pcapkit/const/tcp/flags.py has no guard, and Flags(-1) returns every bit

class Flags(IntFlag) at pcapkit/const/tcp/flags.py:22 declares twelve members, bits 4 through 15 (Reserved_4 = 1 << 4 … FIN = 1 << 15). It defines no _missing_ — confirmed by execution, not by reading:

"_missing_" in vars(Flags): False
Flags._missing_ defined in : aenum._enum

so lookups fall to aenum's Flag machinery, which composes a pseudo-member for anything. Measured:

Flags(-1     ) -> value=65520   name='Reserved_4|Reserved_5|Reserved_6|AE|CWR|ECE|URG|ACK|PSH|RST|SYN|FIN'
Flags(-2     ) -> value=65520   name='Reserved_4|…|FIN'
Flags(-8     ) -> value=65520   name='Reserved_4|…|FIN'
Flags(-65536 ) -> value=0       name=None
Flags(65536  ) -> value=65536   name=None
Flags(1<<70  ) -> value=1180591620717411303424   name=None

Flags(-1) is exactly the OR of every declared member — 65520 == 0xfff0, equal to the fold of all twelve — so a negative value silently reads as every TCP flag set at once. Flags(-65536) is worse in the other direction: it silently reads as no flags set. The TCP flags field is sixteen bits off the wire and can never be negative, so every one of these is an input the type should reject.

Contrast the same generated family, which does carry the guard (pcapkit/const/mh/binding_ack_flag.py):

"_missing_" in vars(BindingACKFlag): True
BindingACKFlag(-1) -> ValueError : -1 is not a valid BindingACKFlag

The consequence is not only the wrong value — it disables get's fallback

pcapkit/const/tcp/flags.py:73-79:

        if isinstance(key, int):
            try:
                return Flags(key)
            except ValueError:
                if default == -1:
                    raise
                return Flags(default)

Because Flags(key) never raises for these inputs, the except ValueError is dead and the caller's default is silently ignored:

Flags.get(-1,     default=0) -> value=65520                  name='Reserved_4|…|FIN'
Flags.get(1<<70,  default=0) -> value=1180591620717411303424 name=None

A caller that asked for 0 on failure gets all twelve flags instead, with no error and nothing logged. That is the default contract issue #584 established for the integer path, not holding here.

Why EnumError is the wrong fix as stated, which is the part worth reading

EnumError is genuinely unused. An unrestricted grep over the tree returns exactly three hits, and grep -rn "raise EnumError\|except EnumError" returns none:

file:line	Kind
pcapkit/utilities/exceptions.py:276	its own definition
pcapkit/utilities/exceptions.py:38	__all__ entry, under the # TypeError comment
docs/source/pcapkit/utilities/exceptions.rst:121	autodoc stub

But pcapkit/utilities/exceptions.py:276-277 is:

class EnumError(BaseError, TypeError):
    """The argument(s) must be *enumeration protocol* type."""

It derives from TypeError, not ValueError, and its docstring is about an argument's type rather than a value's range. issubclass(EnumError, ValueError) is False. Three measured consequences of swapping it into the template, from throwaway replica enums (no library file touched):

1. aenum does propagate it, so the raise itself works: Swapped(99999) -> EnumError: 99999 is not a valid Swapped.

2. The get(key, default) fallback breaks, silently un-doing issue get()'s documented default is ignored on the integer path across the shared const/ enum template #584. The generated get() catches ValueError (pcapkit/vendor/default.py:91, and ~113 generated get() bodies, e.g. pcapkit/const/arp/hardware.py:164 catching what :181 raises). An EnumError sails past it:

   Canonical  get(99999, 0) -> <Canonical.A: 0>   FALLBACK WORKS
   Swapped    get(99999, 0) -> RAISED EnumError   FALLBACK BROKEN
   

   That is pinned by the suite. The full blast radius on origin/main is 8 assertRaises(ValueError) sites plus one template-text assertion:

   • tests/const/test_const_enum_get.py:158, :175, :179, :219 — :219 loops the sweep and pins covered == 110
   • tests/const/test_const_enum_lookup.py:199, :358, :360, :362 — :199 sweeps 111 registries; :358-362 are fix(mh): stop the four flag enums recursing on any non-member value (#623) #632's new test_the_range_guard_still_rejects, 12 assertions over 4 enums
   • tests/const/test_const_enum_get.py:287 — self.assertIn('except ValueError:', from_template.group(0)), which pins the template text directly and must change with it

   By contrast test_the_vendor_templates_still_emit_the_fix (tests/const/test_const_enum_lookup.py:367-405) would not need updating: it renders each template and asserts assertEqual(rendered, committed), so it is self-consistent and passes as long as the template change is followed by a regeneration and commit. It usefully constrains the fix to "template and regenerate together" and will catch a template-only or tree-only edit — exactly the failure mode here.

3. Every discarded fallback would log CRITICAL and zero sys.tracebacklimit. BaseError.__init__ (pcapkit/utilities/exceptions.py:191-205) logs at construction, so it fires even when the exception is caught and thrown away — measured: 1 CRITICAL record and sys.tracebacklimit == 0 after a fallback that succeeded. The library's own answer is quiet=True (pcapkit/utilities/exceptions.py:165, used at pcapkit/corekit/multidict.py:183).

The repo's own precedent points the other way. tests/corekit/test_fields_ipaddress.py:454-464:

That inconsistency is deliberate and safe only because both classes derive from BaseError and from ValueError, so neither documented handler can tell them apart. […] used to raise a bare ValueError, so they were catchable by except ValueError and not by except BaseError. They are now catchable by both -- a widening, not a break.

That is the established pattern: an in-library class deriving from BaseError and the builtin it replaces — ProtocolError(BaseError, ValueError) at :357, FieldValueError(BaseError, ValueError) at :373. Ten such ValueError-category exceptions exist at pcapkit/utilities/exceptions.py:349-385; EnumError is not among them.

So a correct fix is a coordinated change, not a string swap: either give the enum guard an exception deriving from ValueError as well as BaseError (the widening precedent above), or change the except ValueError in every template in lockstep — and in both cases pass quiet=True and update tests/const/test_const_enum_get.py:287, which pins the current template text.

One honesty note: no written rule in the tree says these guards must raise EnumError. docs/source/pcapkit/utilities/exceptions.rst documents the loud/quiet semantics and the category groupings, and CONTRIBUTING.md does not cover it. Part 1 is therefore a proposal grounded in the module's evident purpose, not a quotable convention being violated. Part 2 stands on its own regardless — a missing range guard is a defect under any exception policy.

The fix is a template change plus a regeneration, not a 113-file hand-edit

Everything under pcapkit/const/ is generated. The provenance line is in all 117 modules — pcapkit/const/arp/hardware.py:8-9:

This module contains the constant enumeration for **Hardware Types**,
which is automatically generated from :class:`pcapkit.vendor.arp.hardware.Hardware`.

and that banner is itself template text at pcapkit/vendor/default.py:60-61. There is no literal "do not edit" anywhere under pcapkit/const/; the generated-ness is established by the provenance line plus the regeneration tests.

The raise lives in 9 template files, not 1. There are 13 LINE templates under pcapkit/vendor/; these carry the canonical message:

pcapkit/vendor/default.py:108
pcapkit/vendor/ftp/return_code.py:171
pcapkit/vendor/http/status_code.py:104
pcapkit/vendor/mh/binding_ack_flag.py:78
pcapkit/vendor/mh/binding_update_flag.py:77
pcapkit/vendor/mh/handover_ack_flag.py:78
pcapkit/vendor/mh/handover_initiate_flag.py:78
pcapkit/vendor/pcapng/option_type.py:140
pcapkit/vendor/reg/apptype.py:188

plus the bare control-flow raise at pcapkit/vendor/reg/apptype.py:170. The canonical one, pcapkit/vendor/default.py:99-108:

    @classmethod
    def _missing_(cls, value: 'int') -> '{NAME}':
        """Lookup function used when value is not found.

        Args:
            value: Value to get enum item.

        """
        if not ({FLAG}):
            raise ValueError('%r is not a valid %s' % (value, cls.__name__))

Nothing under const/ is hand-written, so regeneration does fix all 113 provided all 9 templates are edited: clustering every get() and _missing_ body by AST skeleton traces all 8 distinct get() shapes to a vendor template — 106 to default.py, and the bespoke ones (pcapkit/const/pcapng/option_type.py:201 taking namespace, pcapkit/const/reg/apptype.py:30585 taking proto, TransportProtocol at :41 with no default) to per-registry overrides that carry their own template text.

Part 2 is different in shape: tcp/flags.py's guard is absent, so the fix is to add one, in pcapkit/vendor/tcp/flags.py and then regenerate. Note the fix cannot simply copy the mh flag shape — issue #623 (fixed in #632) established that an overriding _missing_ on an IntFlag shadows the aenum Flag machinery that legitimately resolves composites of defined bits, which is precisely why Flags, defining no _missing_, never recursed. A guard for Flags has to reject out-of-range values while still delegating in-range composites to super()._missing_, as #632 landed for the four mh registries.

Regeneration routes, all evidenced, none run here:

• make vendor — Makefile:69-70, pipenv run pcapkit-vendor
• the pcapkit-vendor console script — pyproject.toml:107, backed by pcapkit/vendor/__main__.py
• per-module python -m pcapkit.vendor.<pkg>.<module>, which is what the suite tells you to run (tests/vendor/test_ipx_socket_unit.py:230)
• .github/workflows/cron-vendor.yml:59-67, weekly and on every push to main, which runs pcapkit-vendor then isort -l100 -ppcapkit pcapkit/const/*/*.py — note make vendor omits that isort pass, so it is not byte-identical to CI output

Notes

• Both parts are side-findings from the worker on _missing_ in the four mh flag enums ends in return cls(value), so any non-member value recurses — including 0 #623 (PR fix(mh): stop the four flag enums recursing on any non-member value (#623) #632, now merged) while it was fixing the four mh flag registries. Neither was in that change's scope, so they were reported rather than fixed. Filing them so they are not rediscovered at full cost.
• fix(mh): stop the four flag enums recursing on any non-member value (#623) #632 changed only the last line of those four _missing_ bodies (return cls(value) → return super()._missing_(value)); the raise ValueError guard above it is untouched, so Part 1's count is the same before and after it landed. Verified by reading origin/main directly.
• fix(mh): stop the four flag enums recursing on any non-member value (#623) #632 also added ConstFlagMissingRecursionTests to tests/const/test_const_enum_lookup.py, whose test_the_range_guard_still_rejects (around :346) asserts assertRaises(ValueError) for obj(-1) over the four mh registries. Two things follow: a change to the exception type has to update that test too, and the same test's scope is the four mh registries rather than the full seven-class IntFlag sweep it sits beside — which is why Part 2's missing guard in Flags is not caught there.
• The counts are of modules, not registries. A module may define several: pcapkit/const/ftp/command.py defines four. The repo's own class-level census is 118 (tests/const/test_const_enum_get.py:194). 117 modules and 118 registries are both right, about different things.
• Part 2's Flags also turns up in dumpkit's object_hook interpolates o.name unguarded, so a nameless flag member renders as Type::None [0] #648, from the other direction: pcapkit.const.tcp.flags.Flags is the only registry in the whole of pcapkit/const/ that is nameless at zero, which is what makes Flags(0) render as Flags::None [0] there. The two are independent — dumpkit's object_hook interpolates o.name unguarded, so a nameless flag member renders as Type::None [0] #648 is the dumper's unguarded o.name, this is the registry's missing range guard — but both are about the same class and are worth reading together.
• Related by area only: get()'s documented default is ignored on the integer path across the shared const/ enum template #584 (the get() integer path, fixed across 114 modules in fix(const): normalise the key get() looks up, and honour its documented default (#582, #583, #584) #596) and const enum lookups reject values that appear on the wire: RouterAlert(0) is RFC 2113's only defined value #492 (the registry-wide lookup sweep). This is the exception type and a missing guard, neither of which those touched.

[JarryShaw]

Owner decision: keep the bare ValueError — it is correct, and the fix inverts

In the owner's words:

my thoughts was to keep const enums mock the actual default built-in enum's behaviour, with one exception: they contain the missing then register fallback (mutable enums)

So the 113 modules raising a bare ValueError are right, because that is exactly what the built-in does. Measured on the repo venv, CPython 3.14.7, aenum 3.1.17:

stdlib IntEnum(99) -> ValueError | 99 is not a valid C | isinstance ValueError: True
aenum  IntEnum(99) -> ValueError | 99 is not a valid D | isinstance ValueError: True

This issue's implied fix is therefore reversed: the 4 outliers come to the 113, not the other way round.

Recording why converting to EnumError would have been actively wrong, so nobody reopens it: EnumError is class EnumError(BaseError, TypeError) — not a ValueError. Swapping would diverge from the built-in the owner wants mirrored, break 8 existing assertRaises(ValueError) sites plus a template pin at tests/const/test_const_enum_get.py:287, and regress #584's get(key, default) fallback into emitting CRITICAL on every discarded default.

The one deliberate divergence from the built-in stays: the _missing_ → extend_enum register fallback on the mutable enums.

A worker is on this now — recounting the modules rather than trusting the 113/117 figure, identifying every outlier by file:line, and fixing it in the generator template under pcapkit/vendor/ rather than hand-editing generated files.

Out of scope and deliberately untouched: the default: 'int' = -1 on the generated get() methods, which uses == rather than is and is dispatched on across module boundaries at pcapkit/const/l2tpv2.py:237 and pcapkit/const/ospf.py:191. That is a behaviour change and wants its own issue.