gh-148427: Address review feedback

- Keep FIXME comment about error info
- Use mock.Mock with startElement.side_effect instead of inner classes
- Narrow assertRaises to only the triggering feed() call
- Update NEWS wording per reviewer suggestion

Author: WYSIATI

Lib/test/test_sax.py

@@ -1057,60 +1057,51 @@ def test_expat_entityresolver_default(self):
 
     def test_external_entity_ref_keyboard_interrupt(self):
         # gh-148427: KeyboardInterrupt must propagate, not be swallowed
-        class KBHandler(handler.ContentHandler):
-            def startElement(self, name, attrs):
-                if name == 'entity':
-                    raise KeyboardInterrupt('test')
+        eh = mock.Mock()
+        eh.startElement.side_effect = KeyboardInterrupt('test')
 
         parser = create_parser()
         parser.setFeature(feature_external_ges, True)
         parser.setEntityResolver(self.TestEntityResolver())
-        parser.setContentHandler(KBHandler())
+        parser.setContentHandler(eh)
 
+        parser.feed('<!DOCTYPE doc [\n')
+        parser.feed('  <!ENTITY test SYSTEM "whatever">\n')
+        parser.feed(']>\n')
         with self.assertRaises(KeyboardInterrupt):
-            parser.feed('<!DOCTYPE doc [\n')
-            parser.feed('  <!ENTITY test SYSTEM "whatever">\n')
-            parser.feed(']>\n')
             parser.feed('<doc>&test;</doc>')
-            parser.close()
 
     def test_external_entity_ref_system_exit(self):
         # gh-148427: SystemExit must propagate, not be swallowed
-        class ExitHandler(handler.ContentHandler):
-            def startElement(self, name, attrs):
-                if name == 'entity':
-                    raise SystemExit(42)
+        eh = mock.Mock()
+        eh.startElement.side_effect = SystemExit(42)
 
         parser = create_parser()
         parser.setFeature(feature_external_ges, True)
         parser.setEntityResolver(self.TestEntityResolver())
-        parser.setContentHandler(ExitHandler())
+        parser.setContentHandler(eh)
 
+        parser.feed('<!DOCTYPE doc [\n')
+        parser.feed('  <!ENTITY test SYSTEM "whatever">\n')
+        parser.feed(']>\n')
         with self.assertRaises(SystemExit):
-            parser.feed('<!DOCTYPE doc [\n')
-            parser.feed('  <!ENTITY test SYSTEM "whatever">\n')
-            parser.feed(']>\n')
             parser.feed('<doc>&test;</doc>')
-            parser.close()
 
     def test_external_entity_ref_stack_cleanup(self):
         # gh-148427: _entity_stack must be cleaned up after errors
-        class ErrorHandler(handler.ContentHandler):
-            def startElement(self, name, attrs):
-                if name == 'entity':
-                    raise ValueError('test error')
+        eh = mock.Mock()
+        eh.startElement.side_effect = ValueError('test error')
 
         parser = create_parser()
         parser.setFeature(feature_external_ges, True)
         parser.setEntityResolver(self.TestEntityResolver())
-        parser.setContentHandler(ErrorHandler())
+        parser.setContentHandler(eh)
 
+        parser.feed('<!DOCTYPE doc [\n')
+        parser.feed('  <!ENTITY test SYSTEM "whatever">\n')
+        parser.feed(']>\n')
         with self.assertRaises(SAXParseException):
-            parser.feed('<!DOCTYPE doc [\n')
-            parser.feed('  <!ENTITY test SYSTEM "whatever">\n')
-            parser.feed(']>\n')
             parser.feed('<doc>&test;</doc>')
-            parser.close()
 
         self.assertEqual(len(parser._entity_stack), 0)
 

Lib/xml/sax/expatreader.py

@@ -425,7 +425,7 @@ def external_entity_ref(self, context, base, sysid, pubid):
         try:
             xmlreader.IncrementalParser.parse(self, source)
         except Exception:
-            return 0
+            return 0  # FIXME: save error info here?
         finally:
             (self._parser, self._source) = self._entity_stack[-1]
             del self._entity_stack[-1]

Misc/NEWS.d/next/Library/2026-04-12-09-52-14.gh-issue-148427.xR7v3m.rst

@@ -1,4 +1,3 @@
-Fix bare ``except`` clause in :mod:`xml.sax` expatreader's external entity
-handler that silently swallowed :exc:`KeyboardInterrupt` and
-:exc:`SystemExit`. Also fix entity parser stack leak on errors by moving
-cleanup into a ``finally`` clause.
+Fix bare ``except`` clause in :mod:`xml.sax` Expat's external entity
+handler that silently swallowed :exc:`BaseException` instances such
+as :exc:`KeyboardInterrupt`.