Skip to content

design(const): get(value, default) on an out-of-range value returns the default but logs a CRITICAL error and installs pcapkit's excepthook #1845

Closed

Description

@JarryShaw

LLACode.get(999, 0) returns the default, but it logs CRITICAL pcapkit.utilities.exceptions: EnumValueError: 999 is not a valid LLACode and replaces sys.excepthook (measured on e5f7fb66b; the member name is the subject of #1848). On 24eb56105 the same call is silent.

How it got here:

What is already documented:

Options:

  1. Raise a quiet error on the get path, and keep register loud. _validate_value is shared, so this needs a flag or a signature change across the vendor templates and the regenerated const.
  2. Keep it loud, and document that get(value, default) logs for an out-of-range value.

Activity

  1. added
    designA design or decision issue: a pattern being decided rather than a defect or a request
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 16, 2026
  2. JarryShaw commented on Oct 16, 2026

    @JarryShaw
    OwnerAuthor

    take option 1 - get should be quiet as most of the time it's coming from Protocol parsing logic.

  3. moved this to Needs decision in PyPCAPKiton Oct 16, 2026
  4. JarryShaw commented on Oct 16, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: option 1. get takes the quiet path for an out-of-range value, because most calls come from protocol parsing. register stays loud. A worker is on it. It must not change the vendor templates or the regenerated const files, which another open fix is editing, so the quiet path will live in pcapkit/corekit. It also updates registry-protocol.rst and the EnumLookup.get docstring, which currently say the value miss stays loud.

  5. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 16, 2026
  6. moved this from Needs decision to WIP in PyPCAPKiton Oct 16, 2026
  7. JarryShaw commented on Oct 16, 2026

    @JarryShaw
    OwnerAuthor

    The fix for option 1 is up as #1873. get(value, default) now returns the default quietly for an out-of-range value, and register stays loud.

    One gap against the reason given. The protocol parsers never pass default: extracting http.pcap made thousands of Enum_X.get(...) calls, none of them with a default. So this quiet path changes nothing that parsing does. A no-default get with an out-of-range value still raises a loud EnumValueError. The #1823 follow-up note measured that case as unreachable from parsing, since each field's width fits its registry's range.

    Should a no-default get also raise quietly? The caller would still get the exception, but nothing would be logged and no hook installed. If not, the PR stands as is.

  8. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 16, 2026
  9. JarryShaw commented on Oct 17, 2026

    @JarryShaw
    OwnerAuthor

    Your recommendation on the behaviour?

  10. JarryShaw commented on Oct 17, 2026

    @JarryShaw
    OwnerAuthor

    My recommendation is to make both get paths quiet, and keep register loud.

    • get is a lookup. Its caller decides whether a miss matters. With a default, the miss is part of a successful call. Without one, the caller receives the EnumValueError and can catch it. Either way, a CRITICAL log and a swapped excepthook at construction are noise for a caller that handles the miss.
    • Nothing surfaces less. A quiet error still propagates. If nobody catches it, Python's default hook prints the traceback; only pcapkit's logging and hook install are skipped.
    • register stays loud. A rejected registration is a programming error in the caller's setup, and that is where loud errors are meant to fire.
    • It matches what get did before fix(const): make every registry's register reject what its get rejects (#1816) #1823. _missing_ raised a plain, silent ValueError, and fix(const): make every registry's register reject what its get rejects (#1816) #1823's own note suggested a quiet get path.

    In #1873 this is one more line: the no-default branch also runs under _CaughtByCaller(ValueError). The test asserting the no-default get stays loud would flip.

  11. JarryShaw commented on Oct 17, 2026

    @JarryShaw
    OwnerAuthor

    take your recommendations

  12. JarryShaw commented on Oct 17, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: both get paths are quiet for an out-of-range value, and register stays loud. With a default, get falls back to it. Without one, it raises a quiet EnumValueError, which still propagates but is not logged and installs no hook. A follow-up commit on #1873 extends the quiet block to the no-default branch and flips the test that pinned it loud.

  13. removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 17, 2026
  14. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    designA design or decision issue: a pattern being decided rather than a defect or a request

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions