Monkey-patch catchall `except:` statements so they at least don't catch `KeyboardInterrupt` errors

[Jeroen Demeyer]

There are a lot places in the Sage library which use

except:

This is bad because it catches non-errors like KeyboardInterrupt.

---

Then the patch 11310_auto.patch​ has ben auto-generated to replace all instances of

except:

by

except StandardError:

and all instances of

raise Exception

by

raise RuntimeError

---

Apply trac_11310.patch​ and 11310_auto.patch​ and 11310_psage.patch​.

[Punarbasu Purkayastha]

What's the problem with this ticket? I think it is a small enough change that it can be at least set for review.

[Keshav Kini]

Tests pass on 5.0. Jeroen, is this ready for review, or do you want to add more stuff?

[Jeroen Demeyer]

I would like to fix the following from devel/sage/sage/structure/parent.pyx, starting at line 2577:

        try:
            return super(Parent, self)._an_element_()
        except EmptySetError:
            raise
        except:
            _record_exception()
            pass

        try:
            return self.gen(0)
        except:
            _record_exception()
            pass

        try:
            return self.gen()
        except:
            _record_exception()
            pass

[Keshav Kini]

Changing these bare except statements to except StandardError seems like not enough, don't you think? Except statements should always be written to accept the narrowest possible expected exception class only.

[Punarbasu Purkayastha]

Replying to kini:

Changing these bare except statements to except StandardError seems like not enough, don't you think? Except statements should always be written to accept the narrowest possible expected exception class only.

Then what should it be? According to the ​hierarchy of exceptions I think StandardError encapsulates most of what can go wrong. It is at least better than a blank except:.

About the RuntimeErrors I think some of them should be ValueError and I spotted one which should be an AttributeError.

[Keshav Kini]

I mean that the code should be inspected and the appropriate exception class should be used in each case. Of course, this is not easy, considering that the autogenerated patch is more than 100 KB. But still, it should be done.

[Jeroen Demeyer]

I'm currently going though the auto-generated patch and fixing the obvious things by hand. Note that the title of this ticket isn't "totally fix all exceptions inside Sage".

The main point is to not catch KeyboardInterrupt errors.

[Punarbasu Purkayastha]

Replying to jdemeyer:

The main point is to not catch KeyboardInterrupt errors.

Other than some of the RuntimeErrors, I think the patch is fine, if the objective was this.

[Keshav Kini]

Replying to jdemeyer:

I'm currently going though the auto-generated patch and fixing the obvious things by hand. Note that the title of this ticket isn't "totally fix all exceptions inside Sage".

The main point is to not catch KeyboardInterrupt errors.

Well, the title is a bit misleading I guess. Sure, it's not "totally fix all exceptions inside Sage", but it does seem like "totally fix some exceptions inside Sage" :)

[Jeroen Demeyer]

Okay, I changed a lot of things by hand, needs_review.

[Keshav Kini]

Patchbot says trailing whitespace :P

[Punarbasu Purkayastha]

Hmm.. a lot of doctests are failing:

The following tests failed:
                           
	sage -t  --long -force_lib devel/sage/doc/en/thematic_tutorials/lie/weight_ring.rst # 2 doctests failed
	sage -t  --long -force_lib devel/sage/sage/rings/polynomial/pbori.pyx # 3 doctests failed
	sage -t  --long -force_lib devel/sage/sage/combinat/free_module.py # 3 doctests failed
	sage -t  --long -force_lib devel/sage/sage/combinat/posets/poset_examples.py # 1 doctests failed
	sage -t  --long -force_lib devel/sage/sage/combinat/root_system/weyl_characters.py # 11 doctests failed
	sage -t  --long -force_lib devel/sage/sage/matrix/matrix_double_dense.pyx # 1 doctests failed
	sage -t  --long -force_lib devel/sage/sage/modules/vector_double_dense.pyx # 1 doctests failed
	sage -t  --long -force_lib devel/sage/sage/graphs/isgci.py # 2 doctests failed
	sage -t  --long -force_lib devel/sage/sage/graphs/generic_graph.py # 3 doctests failed
	sage -t  --long -force_lib devel/sage/sage/graphs/graph_decompositions/rankwidth.pyx # 1 doctests failed
	sage -t  --long -force_lib devel/sage/sage/graphs/graph_generators.py # 2 doctests failed
	sage -t  --long -force_lib devel/sage/sage/interfaces/chomp.py # 3 doctests failed
	sage -t  --long -force_lib devel/sage/sage/interfaces/expect.py # 1 doctests failed
	sage -t  --long -force_lib devel/sage/sage/interfaces/psage.py # Time out
	sage -t  --long -force_lib devel/sage/sage/homology/simplicial_complex_morphism.py # 8 doctests failed
	sage -t  --long -force_lib devel/sage/sage/homology/delta_complex.py # 5 doctests failed
	sage -t  --long -force_lib devel/sage/sage/homology/cell_complex.py # 5 doctests failed
	sage -t  --long -force_lib devel/sage/sage/homology/simplicial_complex_homset.py # 7 doctests failed
	sage -t  --long -force_lib devel/sage/sage/homology/simplicial_complex.py # 33 doctests failed
	sage -t  --long -force_lib devel/sage/sage/homology/examples.py # 8 doctests failed

[Punarbasu Purkayastha]

I think something changed in the patches between the last time I ran make ptestlong (I had to leave it running and go home) and the new patches uploaded 12h ago. At present, running the doctest on only these few files gives no errors.

[Keshav Kini]

Patchbot says it's fine over the whole library. Looks good to me.

[Jeroen Demeyer]

Rebased to #11143.

[Jeroen Demeyer]

Not really, I just removed a hunk to resolve the conflict.

[Volker Braun]

Rediffed patch

[Volker Braun]

The new patch just fixes some fuzz 2 with #13109, back to positive review.

[Jeroen Demeyer]

Additional patch

[Jeroen Demeyer]

Additional patch 11310_psage.patch​ needs review (there were sporadic doctest failures in psage.py).

[Volker Braun]

Looks good!

In a perfect world ExceptionPexpect would derive from StandardError and not the raw exceptions, but oh well.

[Punarbasu Purkayastha]

psage is a weird beast. I have seen doctest errors in psage which change from version to version. I couldn't explain it, nor do I know the fix. See #12061.

[R. Andrew Ohana]

This causes issues with Python 3. See #15620.