Skip to content

Do not swallow non-TypeError exceptions in feature-detection fallbacks#1382

Open
devdanzin wants to merge 3 commits into
aio-libs:masterfrom
devdanzin:fix/broad-pyerr-clear
Open

Do not swallow non-TypeError exceptions in feature-detection fallbacks#1382
devdanzin wants to merge 3 commits into
aio-libs:masterfrom
devdanzin:fix/broad-pyerr-clear

Conversation

@devdanzin

Copy link
Copy Markdown

What do these changes do?

Several feature-detection fallbacks cleared every pending exception when a fallback path was taken:
view richcompare and items-view __contains__ measuring len(other), items-view __contains__
computing the identity, MultiDict.__eq__ probing keys(), and extend()/constructor probing
items(). A MemoryError or KeyboardInterrupt raised by the user object was silently discarded. Each
is now guarded with PyErr_ExceptionMatches() so only the expected TypeError/AttributeError is
cleared and anything else propagates — the same pattern the sibling sites in views.h already use.
C-extension-only; the pure-Python backend propagates naturally.

Are there changes in behavior for the user?

Yes (edge case): an exception other than TypeError/AttributeError (notably MemoryError,
KeyboardInterrupt) raised by an object being probed now propagates instead of being swallowed. The
normal fallbacks (a non-sized or non-mapping object) are unchanged.

Is it a substantial burden for the maintainers to support this?

No. It applies the module's existing guard pattern to the few sites that missed it.

Related issue number

Findings from the C-extension analysis report #1306.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist (test_non_typeerror_exceptions_are_not_swallowed)
  • Documentation reflects the changes (no public API change)
  • N/A — no CONTRIBUTORS.txt in this repo
  • Add a new news fragment into the CHANGES/ folder

Several probes cleared *every* pending exception when a fallback path was
taken: view richcompare and items-view __contains__ measuring len(other),
items-view __contains__ computing the identity, MultiDict.__eq__ probing
keys(), and extend()/ctor probing items().  A MemoryError or KeyboardInterrupt
raised by the user object was silently discarded.

Guard each with PyErr_ExceptionMatches() so only the expected
TypeError/AttributeError is cleared and anything else propagates -- the same
pattern the sibling sites in views.h already use.

Adds a C-extension regression test.
@devdanzin
devdanzin requested a review from asvetlov as a code owner July 24, 2026 14:06
@devdanzin
devdanzin requested a review from webknjaz as a code owner July 24, 2026 14:06
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Jul 24, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 242 untouched benchmarks


Comparing devdanzin:fix/broad-pyerr-clear (e5d1910) with master (e71a593)

Open in CodSpeed

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.80%. Comparing base (e71a593) to head (e5d1910).

Files with missing lines Patch % Lines
tests/test_multidict.py 90.00% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (90.00%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (99.92%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1382      +/-   ##
==========================================
- Coverage   99.86%   99.80%   -0.06%     
==========================================
  Files          28       28              
  Lines        3627     3647      +20     
  Branches      265      265              
==========================================
+ Hits         3622     3640      +18     
- Misses          3        5       +2     
  Partials        2        2              
Flag Coverage Δ
CI-GHA 99.80% <90.00%> (-0.06%) ⬇️
pytest 99.80% <90.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant