danielfm / pybreaker

Python implementation of the Circuit Breaker pattern.
BSD 3-Clause "New" or "Revised" License
528 stars 73 forks source link

Exceptions in generators have wrong location #47

Closed phillbaker closed 5 years ago

phillbaker commented 5 years ago

We're seeing unusual behavior when wrapping a generator (a contextmanager specifically) that has a try/except in a circuit breaker. Specifically, the try/except in the wrapped function isn't being executed.

A simplified example is:

from contextlib import contextmanager
from pybreaker import CircuitBreaker

class ExceptionA(Exception): pass

class ExceptionB(Exception): pass

breaker = CircuitBreaker()

class Foo:
  @contextmanager
  @breaker
  def wrapper(self):
    try:
      yield
    except ExceptionA as e:
      raise ExceptionB()

  def foo(self):
    with self.wrapper():
      raise ExceptionA()

try:
  Foo().foo()
except ExceptionB as e:
  print('caught ExceptionB', e)

This example should print "caught ExceptionB", however, it raises:

Traceback (most recent call last):
  File "context.py", line 67, in <module>
    Foo().foo()
  File "context.py", line 63, in foo
    raise ExceptionA()
__main__.ExceptionA

At the root, I believe that these lines:

https://github.com/danielfm/pybreaker/blob/42b05c7531dff69ded65f9651b2d3f244f104acc/src/pybreaker.py#L744-L745

should not have _handle_error invoke raise but call wrapped_generator.throw(e).

Would it make sense to submit a PR for this? Perhaps a new argument to _handle_error can make raising optional?