Skip to content

fix(corekit): quiet get's fallback over an out-of-range value (#1845) - #1873

Merged
JarryShaw merged 2 commits into
mainfrom
fix/1845-quiet-get
Oct 17, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
fix/1845-quiet-get

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

  • exceptions: add a _CaughtByCaller block, backed by a ContextVar, inside
    which a BaseError of a named class is constructed quiet; BaseError
    consults it alongside quiet=True.
  • enum: EnumLookup.get runs _validate_value and cls(key) in a
    _CaughtByCaller(ValueError) block when default is usable, so a generated
    override's loud rejection logs nothing and installs no hook there; with no
    usable default the miss stays loud.
  • enum: EnumRegistry.register validates in a block declaring nothing caught,
    so its rejection stays loud even when reached from inside get.
  • docs: get's docstring gains a 1.5.0 versionchanged and no longer says the
    value miss stays loud; registry-protocol.rst says what is reported now.
  • tests: cover quiet fallback, the loud paths, nesting and per-thread scope.

Closes #1845

Open scope question, asked on #1845: protocol parsers never pass default to get, so this quiet path does not change what parsing logs. Whether a no-default get should also raise quietly is pending the owner.

- exceptions: add a _CaughtByCaller block, backed by a ContextVar, inside
  which a BaseError of a named class is constructed quiet; BaseError
  consults it alongside quiet=True.
- enum: EnumLookup.get runs _validate_value and cls(key) in a
  _CaughtByCaller(ValueError) block when default is usable, so a generated
  override's loud rejection logs nothing and installs no hook there; with no
  usable default the miss stays loud.
- enum: EnumRegistry.register validates in a block declaring nothing caught,
  so its rejection stays loud even when reached from inside get.
- docs: get's docstring gains a 1.5.0 versionchanged and no longer says the
  value miss stays loud; registry-protocol.rst says what is reported now.
- tests: cover quiet fallback, the loud paths, nesting and per-thread scope.

Closes #1845
@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 16, 2026
…t a default (#1845)

- enum: EnumLookup.get always runs _validate_value and cls(key) in the
  _CaughtByCaller(ValueError) block, and converts aenum's own miss into a
  quiet EnumValueError, so a raised value miss is not logged and installs
  no hook; register stays loud, including from inside get.
- docs: get's docstring, versionchanged and registry-protocol.rst say every
  get miss is quiet and register is loud.
- tests: flip the no-default and unusable-default cases to quiet, add the
  converted miss, get_all, uncaught-traceback and raising-get-reaches-
  register cases; the #923 loud control now constructs its error directly.

Refs #1845
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 2da6b1be7 per the ruling on #1845. get is now quiet for an out-of-range value with or without a default. An uncaught miss still prints the interpreter's own traceback, with no CRITICAL log. register stays loud. The tests that pinned the no-default get as loud are flipped, including the #923 one, and a loud control is added in its place. The review is extended to this commit.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 17, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 2da6b1be7. A Sonnet cross-review covered the final head and the earlier d019b8368, and found no defects.

  • get is quiet. ToSPrecedence.get(8) raises EnumValueError with no log record and leaves the excepthooks and sys.tracebacklimit alone. I reproduced this myself. get(8, 0) returns the default quietly, and get_all behaves the same way.
  • An uncaught miss prints the interpreter's own traceback (exit 1, no [CRITICAL] line), with or without forced colour.
  • register stays loud, including when it is reached from inside a quiet get. Removing the _CaughtByCaller() block fails three new tests. A non-ValueError from the hook still propagates loudly.
  • Fail-before. Against d019b8368's enum.py, the new tests fail for the right reasons. The fix(corekit,utilities): raise pcapkit exceptions from EnumLookup.get, following stdlib Enum's shape #923 control fails if BaseError stops logging, so the quiet tests cannot pass vacuously.
  • Isolation. The quiet state is isolated per thread and per asyncio task, and swapping the ContextVar for a global fails the thread test.
  • Tests: tests/project gave 988 passed; tests/const, corekit, utilities, vendor and interface pass.
  • Unverified. End-to-end parsing of real captures, because the review tree had no generated samples.
  • Notes, not blockers:
    • The generated AppType.register and OptionType.register call _validate_value without the loud block. Inside a quiet get they would log nothing, but nothing reaches them that way today.
    • A non-str get now costs about 900 ns instead of about 405 ns.

@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
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 90.05% (unit tier, Python 3.15, 2da6b1be7, 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 2464 69 780 35 96.55%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2915 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 522 2 158 4 99.12%
pcapkit/vendor 4407 2342 1010 157 43.27%

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

@JarryShaw
JarryShaw merged commit 3bf2cbf into main Oct 17, 2026
55 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/1845-quiet-get branch October 17, 2026 13:20
@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.

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

1 participant