GeezerVison is showing github.com — gh-155944: narrow xml.dom fallback discovery to ImportError and AttributeError by shaneraphel · Pull Request #157980 · python/cpython
Conversation
shaneraphel
commented
Sep 23, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary
xml.dom.domreg.getDOMImplementation() tried each well-known implementation under a bare except Exception, so a genuine bug in a candidate getDOMImplementation() factory was silently skipped exactly like a missing module.- The loop now catches
(ImportError, AttributeError) only: a missing module or a missing factory still means "try the next one", while any other exception propagates.
- Adds
Lib/test/test_xml_domreg.py (missing module skipped, missing factory skipped, RuntimeError from a broken factory propagates, default discovery still works) and a NEWS entry.
Fixes #155944.
Test plan
- New tests pass against the patched module (4/4), loaded directly from this tree
- Against the unpatched module, the genuine-bug test fails with
ImportError: no suitable DOM implementation found, reproducing the reported masking
- Full
test_xml_dom / CI run
Prepared with an AI assistant. I verified the 4/4 pass and the before/after difference by running the new tests against the patched and original module before opening this pull request. The author of the commit is shanyu.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
… AttributeError.
The well-known-implementation loop caught bare Exception, so a genuine bug in a candidate factory was skipped exactly like a missing module. Only a missing module (ImportError) or a missing factory (AttributeError) now means "not available"; anything else propagates. Adds Lib/test/test_xml_domreg.py and a NEWS entry.
|
All commit authors signed the Contributor License Agreement.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
You need to sign the CLA before we can review.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks for flagging this. The python-cla-bot comment above shows all commit authors have signed the CLA, so I believe that gate is green now. Let me know if anything else is needed on my side.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I said on the issue that a buggy implementation should rather be skipped because it may be in a state that it is actually not a good enough implementation. Now any bad implementation blocks the discovery of other good ones. This is not what I would expect.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…one.
A factory that raises is not a good enough implementation, but it must
not stop discovery. Import and attribute failures stay "unavailable".
If nothing usable remains, the first factory failure is re-raised.
|
A failing factory is not a good enough implementation, and it should not stop the search. The loop now treats two cases differently:
ImportError and AttributeError still mean the candidate is not available, and the search continues.- Any other exception from that candidate is remembered and the search continues, so a later well-known implementation can still be returned.
- If nothing usable is left, the first factory failure is re-raised. A lone broken candidate does not turn into a generic
ImportError.
test_buggy_candidate_does_not_block_a_later_one puts a factory that raises RuntimeError ahead of minidom and checks that minidom is still returned. The five tests in test_xml_domreg passed locally.
Prepared with an AI assistant. I reviewed the change and ran the tests.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|