Attached is a simple example; yes, a bit contrived, but
it's exactly what bit me in the ass for a week or two :)

nestled within abstract.c's recursive_isinstance, is
this lil nugget-

icls = PyObject_GetAttr(inst, __class__);
if (icls == NULL) {
  PyErr_Clear();
  retval = 0;
}
else {

No surrouding comments to indicate *why* it's
swallowing exceptions, but best explanation I've heard
was that it was attempting to swallow just
AttributeError... which would make sense.

So the question is, whats the purpose of it swallowing
exceptions there?  Bad form of AttributeError catching,
or some unstated reason?

Logged In: YES 
user_id=874085

addition note; this is both 2.5 and 2.4, probably stretches
bit further back also.

Logged In: YES 
user_id=874085

quicky patch for this; basically, wipe the exception only if
it's AttributeError, else let it bubble it's way up.

Neal, do you have time to look at this one?

Possibly in a week or so.  It's unlikely I'll get to it sooner.

Yeah, this seems like a bug and the patch seems correct.  The patch also
needs a test and add a space between if (.

The return value should be -1 in case of errors. There's also a second
code path swallowing all errors.
I've converted brian's test.py to a unit test testing both code paths
and added an updated patch for this one.
This patch is against trunk.
All tests in Lib/test/test_isinstance.py pass.

There are more locations in abstract.c where exceptions are
unconditionally masked after a GetAttr operation:

* line 1630, 1717 in PyNumber_{Long,Int} when looking for .__trunc__
* line 2932, in PyObject_IsInstance

I'd like to clarify the approach to fixing these types of problems. I
fixed a issue (#2196) like this by only propagating exceptions that
didn't descend from Exception because of this sentence in the hasattr docs:

This is implemented by calling getattr(object, name) and seeing whether
it raises an exception or not.)

However, I like constricting it to AttributeError only as that would
make it much less confusing. This might be something to bring up on
python-dev.

> I'd like to clarify the approach to fixing these types of problems.
> ...
> However, I like constricting it to AttributeError only as that would
> make it much less confusing. This might be something to bring up on
> python-dev.

I suspect that it might be on a case by case basis whether we want to
constrain more or less.  Bringing these cases up on python-dev should
lead to speedy decisions.

I can confirm that the patch applies cleanly to the 2.6 branch, that the new test fails before rebuilding, and that the test passes afterwards:

 $ hg summary
 parent: 41597:295c02a21979 tip
  [svn r80597] Merged revisions 80596 via svnmerge from
 branch: release26-maint
 commit: (clean)
 update: (current)
 $ ./configure && make
 ...
 $ ./python -E -tt Lib/test/regrtest.py test_isinstance
 test_isinstance
 1 test OK.
 $ patch -p1 < /tmp/issue1574217_dont_mask_errors.patch
 patching file Lib/test/test_isinstance.py
 patching file Objects/abstract.c
 Hunk #1 succeeded at 2854 (offset 619 lines).
 Hunk #2 succeeded at 2878 (offset 600 lines).
 test_isinstance
 test test_isinstance failed -- Traceback (most recent call last):
   File "/home/tseaver/projects/hg-repo/issue1574217/Lib/test/test_isinstance.py", line 94, in test_isinstance_dont_mask_non_attribute_error
    self.assertRaises(RuntimeError, isinstance, c, bool)
 AssertionError: RuntimeError not raised
 
 1 test failed:
     test_isinstance
 $ make
 ...
 $ ./python -E -tt Lib/test/regrtest.py test_isinstance
 test_isinstance
 1 test OK.

Can we get this closed as the patch is in 2.6 already, just needs to be applied to the later versions.  Or have I missed something?

I don't see it being applied.  Tres Seaver above reports that issue1574217_dont_mask_errors.txt applies cleanly to 2.6 branch, which means that it has not been applied yet.

Someone needs to check if the patch is current for 2.7 and py3k branches to move this forward.

This bug exists in Python 2.6 and 3.1, which are still being maintained, AFAIK.

This looks like a borderline case between a bug and a feature request.  If this is deemed to be a feature, it is not appropriate for 2.x or 3.1.  In any case, I think the first step should be to consider this for py3k branch.

I have verified that the issue is present in py3k.  The unit test portion of the patch applies cleanly in py3k an the added tests fail.  The code portion applies with patch -l and fixes the issue.  I'll upload a patch with fixed white space.

issue1574217.diff still applies against py3k as revision 86545  (offset -8 lines)

patching file Objects/abstract.c
Hunk #1 succeeded at 2500 (offset -8 lines).
Hunk #2 succeeded at 2523 (offset -8 lines).

And tests pass (./python ./Lib/test/test_isinstance.py)

I've committed this (with the whitespace fix) in r86577.  I've made myself a note to backport it when the maint branches unfreeze.

Upon reflection I think the risk of breaking apparently working programs is higher than the benefit to be obtained from backporting this.