Ticket #2803 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

findFailure fails to find the correct failure in a more complicated case

Reported by: Peaker Owned by: Peaker
Priority: highest Milestone:
Component: core Keywords:
Cc: therve Branch: branches/findfailure-2803
Author: peaker Launchpad Bug:

Description

The problem is that findFailure uses the fact that the caller of the frame in which except: was used is "throwExceptionIntoGenerator" but that doesn't necessarily mean that we are actually catching *that*. We may be catching something that was re-raised from a catcher of that exception.

I think a solution would be to verify that it was indeed a yield that raised it, and not only that throwExceptionIntoGenerator is the caller frame.

I attached a patch with an extra test in test_failure's generator_tests to demonstrate the problem.

Attachments

test_failure.patch Download (2.5 KB) - added by Peaker 3 years ago.
A test to demonstrate a problem with findFailure

Change History

Changed 3 years ago by Peaker

A test to demonstrate a problem with findFailure

  Changed 3 years ago by Peaker

I created a branch findfailure-2803 for this ticket, so ignore the patch file.

  Changed 3 years ago by peaker

(In [21923]) Naming conventions, spacing conventions, and a docstring. refs #2803

  Changed 3 years ago by Peaker

  • priority changed from normal to highest
  • keywords review added
  • owner glyph deleted
  • branch set to findfailure-2803
  • author set to peaker

Hairy situation requires hairy fixes!

I think its ready for review...

follow-up: ↓ 5   Changed 3 years ago by therve

  • keywords review removed
  • cc therve added
  • owner set to Peaker
  • branch changed from findfailure-2803 to branches/findfailure-2803
  • _yieldOpcode is unnecessary, opcode.opmap["YIELD_VALUE"] exists in python 2.3. I suggest making the value a private class variable of Failure (but with documentation).
  • several whitespaces missing around = operator in tests
  • instead of self.assertEqual(Failure().type, IndexError), I prefer self.assertIsInstance(Failure().value, IndexError)
  • in the docstrings of test_ambiguousFailureInGenerator and test_ambiguousFailureFromGenerator, please use L{Failure._findFailure} instead of findFailure
  • trailing whitespace in failure.py

Thanks!

in reply to: ↑ 4   Changed 3 years ago by Peaker

  • owner changed from Peaker to therve
  • keywords review added

Replying to therve:

Thanks for the review.

* _yieldOpcode is unnecessary, opcode.opmap["YIELD_VALUE"] exists in python 2.3. I suggest making the value a private class variable of Failure (but with documentation).

Ah, didn't know that. Done.

* several whitespaces missing around = operator in tests

Fixed.

* instead of self.assertEqual(Failure().type, IndexError), I prefer self.assertIsInstance(Failure().value, IndexError)

Done.

* in the docstrings of test_ambiguousFailureInGenerator and test_ambiguousFailureFromGenerator, please use L{Failure._findFailure} instead of findFailure

Done.

* trailing whitespace in failure.py

That wasn't mine, but fixed anyhow.

  Changed 3 years ago by therve

  • keywords review removed
  • owner changed from therve to Peaker

OK please merge.

  Changed 3 years ago by therve

  • status changed from new to closed
  • resolution set to fixed

(In [21945]) Merge findfailure-2803

Author: peaker Reviewer: therve Fixes #2803

Fix a corner case in Failure._findFailure where throwExceptionIntoGenerator is in the caller frame but not the real exception.

Note: See TracTickets for help on using tickets.