From ec0d208bc14c49c611ae05d391b1edb35af854c7 Mon Sep 17 00:00:00 2001 From: Martijn Pieters Date: Mon, 23 Mar 2015 15:17:19 +0000 Subject: [PATCH] Switch away from using None as default value for the exception when tearing down a context. When an exception has been handled when using the request / app context in a with statement, `sys.exc_info()` will still contain the exception information even though it has been handled already. The `__exit__` methods pass in `None` for the exception value in that case, which needs to be distinguisable from the default value for the `exc` parameter. Use a dedicated singleton sentinel value instead. --- flask/app.py | 11 +++++++---- flask/ctx.py | 12 ++++++++---- tests/test_appctx.py | 15 +++++++++++++++ tests/test_reqctx.py | 15 +++++++++++++++ 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/flask/app.py b/flask/app.py index 467725c1..1f7df2e8 100644 --- a/flask/app.py +++ b/flask/app.py @@ -38,6 +38,9 @@ from ._compat import reraise, string_types, text_type, integer_types # a lock used for logger initialization _logger_lock = Lock() +# a singleton sentinel value for parameter defaults +_sentinel = object() + def _make_timedelta(value): if not isinstance(value, timedelta): @@ -1774,7 +1777,7 @@ class Flask(_PackageBoundObject): self.save_session(ctx.session, response) return response - def do_teardown_request(self, exc=None): + def do_teardown_request(self, exc=_sentinel): """Called after the actual request dispatching and will call every as :meth:`teardown_request` decorated function. This is not actually called by the :class:`Flask` object itself but is always @@ -1785,7 +1788,7 @@ class Flask(_PackageBoundObject): Added the `exc` argument. Previously this was always using the current exception information. """ - if exc is None: + if exc is _sentinel: exc = sys.exc_info()[1] funcs = reversed(self.teardown_request_funcs.get(None, ())) bp = _request_ctx_stack.top.request.blueprint @@ -1795,14 +1798,14 @@ class Flask(_PackageBoundObject): func(exc) request_tearing_down.send(self, exc=exc) - def do_teardown_appcontext(self, exc=None): + def do_teardown_appcontext(self, exc=_sentinel): """Called when an application context is popped. This works pretty much the same as :meth:`do_teardown_request` but for the application context. .. versionadded:: 0.9 """ - if exc is None: + if exc is _sentinel: exc = sys.exc_info()[1] for func in reversed(self.teardown_appcontext_funcs): func(exc) diff --git a/flask/ctx.py b/flask/ctx.py index 8c574c2e..24d0612c 100644 --- a/flask/ctx.py +++ b/flask/ctx.py @@ -21,6 +21,10 @@ from .signals import appcontext_pushed, appcontext_popped from ._compat import BROKEN_PYPY_CTXMGR_EXIT, reraise +# a singleton sentinel value for parameter defaults +_sentinel = object() + + class _AppCtxGlobals(object): """A plain object.""" @@ -168,11 +172,11 @@ class AppContext(object): _app_ctx_stack.push(self) appcontext_pushed.send(self.app) - def pop(self, exc=None): + def pop(self, exc=_sentinel): """Pops the app context.""" self._refcnt -= 1 if self._refcnt <= 0: - if exc is None: + if exc is _sentinel: exc = sys.exc_info()[1] self.app.do_teardown_appcontext(exc) rv = _app_ctx_stack.pop() @@ -320,7 +324,7 @@ class RequestContext(object): if self.session is None: self.session = self.app.make_null_session() - def pop(self, exc=None): + def pop(self, exc=_sentinel): """Pops the request context and unbinds it by doing that. This will also trigger the execution of functions registered by the :meth:`~flask.Flask.teardown_request` decorator. @@ -334,7 +338,7 @@ class RequestContext(object): if not self._implicit_app_ctx_stack: self.preserved = False self._preserved_exc = None - if exc is None: + if exc is _sentinel: exc = sys.exc_info()[1] self.app.do_teardown_request(exc) diff --git a/tests/test_appctx.py b/tests/test_appctx.py index 28836ec5..2b0f5f94 100644 --- a/tests/test_appctx.py +++ b/tests/test_appctx.py @@ -78,6 +78,21 @@ def test_app_tearing_down_with_previous_exception(): assert cleanup_stuff == [None] +def test_app_tearing_down_with_handled_exception(): + cleanup_stuff = [] + app = flask.Flask(__name__) + @app.teardown_appcontext + def cleanup(exception): + cleanup_stuff.append(exception) + + with app.app_context(): + try: + raise Exception('dummy') + except Exception: + pass + + assert cleanup_stuff == [None] + def test_custom_app_ctx_globals_class(): class CustomRequestGlobals(object): def __init__(self): diff --git a/tests/test_reqctx.py b/tests/test_reqctx.py index b4f704a6..558958dc 100644 --- a/tests/test_reqctx.py +++ b/tests/test_reqctx.py @@ -48,6 +48,21 @@ def test_teardown_with_previous_exception(): assert buffer == [] assert buffer == [None] +def test_teardown_with_handled_exception(): + buffer = [] + app = flask.Flask(__name__) + @app.teardown_request + def end_of_request(exception): + buffer.append(exception) + + with app.test_request_context(): + assert buffer == [] + try: + raise Exception('dummy') + except Exception: + pass + assert buffer == [None] + def test_proper_test_request_context(): app = flask.Flask(__name__) app.config.update(