diff --git a/pysnooper/tracer.py b/pysnooper/tracer.py index f5e965b..222efdc 100644 --- a/pysnooper/tracer.py +++ b/pysnooper/tracer.py @@ -29,6 +29,23 @@ RETURN_OPCODES = { } +def call_ended_by_exception(frame, event, arg): + """Whether a 'return' event was caused by an exception, not a normal return. + + On an exception the interpreter still emits a 'return' event with ``arg`` of + ``None``, so the last executed opcode is what tells them apart. A frame's + ``f_lasti`` can be ``-1`` (e.g. before any instruction has run); indexing + ``co_code[-1]`` would read the wrong opcode, so treat that as a normal + return. See https://github.com/cool-RR/PySnooper/issues/260. + """ + if event != 'return' or arg is not None or frame.f_lasti < 0: + return False + code_byte = frame.f_code.co_code[frame.f_lasti] + if not isinstance(code_byte, int): + code_byte = ord(code_byte) + return opcode.opname[code_byte] not in RETURN_OPCODES + + def get_local_reprs(frame, watch=(), custom_repr=(), max_length=None, normalize=False): code = frame.f_code vars_order = (code.co_varnames + code.co_cellvars + code.co_freevars + @@ -536,14 +553,7 @@ class Tracer: # If a call ends due to an exception, we still get a 'return' event # with arg = None. This seems to be the only way to tell the difference # https://stackoverflow.com/a/12800909/2482744 - code_byte = frame.f_code.co_code[frame.f_lasti] - if not isinstance(code_byte, int): - code_byte = ord(code_byte) - ended_by_exception = ( - event == 'return' - and arg is None - and opcode.opname[code_byte] not in RETURN_OPCODES - ) + ended_by_exception = call_ended_by_exception(frame, event, arg) if ended_by_exception: self.write('{_FOREGROUND_RED}{indent}Call ended by exception{_STYLE_RESET_ALL}'. diff --git a/tests/test_ended_by_exception.py b/tests/test_ended_by_exception.py new file mode 100644 index 0000000..b8fc963 --- /dev/null +++ b/tests/test_ended_by_exception.py @@ -0,0 +1,41 @@ +"""Tests for call_ended_by_exception, the 'return'-via-exception detector. + +Regression test for https://github.com/cool-RR/PySnooper/issues/260: a frame's +f_lasti can be -1, and co_code[-1] would read the wrong opcode and misreport a +normal return as "Call ended by exception". +""" + +import opcode +from unittest.mock import Mock + +from pysnooper.tracer import call_ended_by_exception + + +def _frame(co_code, f_lasti): + frame = Mock() + frame.f_lasti = f_lasti + frame.f_code.co_code = co_code + return frame + + +def test_negative_f_lasti_is_not_an_exception(): + # Last byte is a non-return opcode, so reading co_code[-1] (the old bug) + # would misreport an exception. With f_lasti == -1 it must be a normal + # return. + co_code = bytes([0, opcode.opmap["NOP"]]) + assert call_ended_by_exception(_frame(co_code, -1), "return", None) is False + + +def test_last_opcode_return_is_normal_return(): + co_code = bytes([opcode.opmap["RETURN_VALUE"], 0]) + assert call_ended_by_exception(_frame(co_code, 0), "return", None) is False + + +def test_last_opcode_not_return_is_exception(): + co_code = bytes([opcode.opmap["NOP"], 0]) + assert call_ended_by_exception(_frame(co_code, 0), "return", None) is True + + +def test_non_return_event_is_never_exception(): + co_code = bytes([opcode.opmap["NOP"], 0]) + assert call_ended_by_exception(_frame(co_code, 0), "call", None) is False