diff --git a/CHANGES.rst b/CHANGES.rst index 0f0b5b76..7bf67554 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -11,6 +11,17 @@ Unreleased - Bump minimum Werkzeug version to >= 0.15. - Drop support for Python 3.4. +- Error handlers for ``InternalServerError`` or ``500`` will always be + passed an instance of ``InternalServerError``. If they are invoked + due to an unhandled exception, that original exception is now + available as ``e.original_exception`` rather than being passed + directly to the handler. The same is true if the handler is for the + base ``HTTPException``. This makes error handler behavior more + consistent. :pr:`3266` + + - :meth:`Flask.finalize_request` is called for all unhandled + exceptions even if there is no ``500`` error handler. + - :meth:`flask.RequestContext.copy` includes the current session object in the request context copy. This prevents ``session`` pointing to an out-of-date object. (`#2935`_) diff --git a/docs/errorhandling.rst b/docs/errorhandling.rst index 92a88a07..9359acf8 100644 --- a/docs/errorhandling.rst +++ b/docs/errorhandling.rst @@ -139,10 +139,94 @@ raises the exception. However, the blueprint cannot handle 404 routing errors because the 404 occurs at the routing level before the blueprint can be determined. -.. versionchanged:: 0.11 - Handlers are prioritized by specificity of the exception classes they are - registered for instead of the order they are registered in. +Generic Exception Handlers +`````````````````````````` + +It is possible to register error handlers for very generic base classes +such as ``HTTPException`` or even ``Exception``. However, be aware that +these will catch more than you might expect. + +An error handler for ``HTTPException`` might be useful for turning +the default HTML errors pages into JSON, for example. However, this +handler will trigger for things you don't cause directly, such as 404 +and 405 errors during routing. Be sure to craft your handler carefully +so you don't lose information about the HTTP error. + +.. code-block:: python + + from flask import json + from werkzeug.exceptions import HTTPException + + @app.errorhandler(HTTPException) + def handle_exception(e): + """Return JSON instead of HTML for HTTP errors.""" + # start with the correct headers and status code from the error + response = e.get_response() + # replace the body with JSON + response.data = json.dumps({ + "code": e.code, + "name": e.name, + "description": e.description, + }) + response.content_type = "application/json" + return response + + +An error handler for ``Exception`` might seem useful for changing how +all errors, even unhandled ones, are presented to the user. However, +this is similar to doing ``except Exception:`` in Python, it will +capture *all* otherwise unhandled errors, including all HTTP status +codes. In most cases it will be safer to register handlers for more +specific exceptions. Since ``HTTPException`` instances are valid WSGI +responses, you could also pass them through directly. + +.. code-block:: python + + from werkzeug.exceptions import HTTPException + + @app.errorhandler(Exception) + def handle_exception(e): + # pass through HTTP errors + if isinstance(e, HTTPException): + return e + + # now you're handling non-HTTP exceptions only + return render_template("500_generic.html", e=e), 500 + +Error handlers still respect the exception class hierarchy. If you +register handlers for both ``HTTPException`` and ``Exception``, the +``Exception`` handler will not handle ``HTTPException`` subclasses +because it the ``HTTPException`` handler is more specific. + +Unhandled Exceptions +```````````````````` + +When there is no error handler registered for an exception, a 500 +Internal Server Error will be returned instead. See +:meth:`flask.Flask.handle_exception` for information about this +behavior. + +If there is an error handler registered for ``InternalServerError``, +this will be invoked. As of Flask 1.1.0, this error handler will always +be passed an instance of ``InternalServerError``, not the original +unhandled error. The original error is available as ``e.original_error``. +Until Werkzeug 1.0.0, this attribute will only exist during unhandled +errors, use ``getattr`` to get access it for compatibility. + +.. code-block:: python + + @app.errorhandler(InternalServerError) + def handle_500(e): + original = getattr(e, "original_exception", None) + + if original is None: + # direct 500 error, such as abort(500) + return render_template("500.html"), 500 + + # wrapped unhandled error + return render_template("500_unhandled.html", e=original), 500 + Logging ------- diff --git a/src/flask/app.py b/src/flask/app.py index 6e0647e3..503810e8 100644 --- a/src/flask/app.py +++ b/src/flask/app.py @@ -1811,11 +1811,36 @@ class Flask(_PackageBoundObject): return handler(e) def handle_exception(self, e): - """Default exception handling that kicks in when an exception - occurs that is not caught. In debug mode the exception will - be re-raised immediately, otherwise it is logged and the handler - for a 500 internal server error is used. If no such handler - exists, a default 500 internal server error message is displayed. + """Handle an exception that did not have an error handler + associated with it, or that was raised from an error handler. + This always causes a 500 ``InternalServerError``. + + Always sends the :data:`got_request_exception` signal. + + If :attr:`propagate_exceptions` is ``True``, such as in debug + mode, the error will be re-raised so that the debugger can + display it. Otherwise, the original exception is logged, and + an :exc:`~werkzeug.exceptions.InternalServerError` is returned. + + If an error handler is registered for ``InternalServerError`` or + ``500``, it will be used. For consistency, the handler will + always receive the ``InternalServerError``. The original + unhandled exception is available as ``e.original_exception``. + + .. note:: + Prior to Werkzeug 1.0.0, ``InternalServerError`` will not + always have an ``original_exception`` attribute. Use + ``getattr(e, "original_exception", None)`` to simulate the + behavior for compatibility. + + .. versionchanged:: 1.1.0 + Always passes the ``InternalServerError`` instance to the + handler, setting ``original_exception`` to the unhandled + error. + + .. versionchanged:: 1.1.0 + ``after_request`` functions and other finalization is done + even for the default 500 response when there is no handler. .. versionadded:: 0.3 """ @@ -1833,10 +1858,16 @@ class Flask(_PackageBoundObject): raise e self.log_exception((exc_type, exc_value, tb)) - handler = self._find_error_handler(InternalServerError()) - if handler is None: - return InternalServerError() - return self.finalize_request(handler(e), from_error_handler=True) + server_error = InternalServerError() + # TODO: pass as param when Werkzeug>=1.0.0 is required + # TODO: also remove note about this from docstring and docs + server_error.original_exception = e + handler = self._find_error_handler(server_error) + + if handler is not None: + server_error = handler(server_error) + + return self.finalize_request(server_error, from_error_handler=True) def log_exception(self, exc_info): """Logs an exception. This is called by :meth:`handle_exception` diff --git a/tests/test_user_error_handler.py b/tests/test_user_error_handler.py index f26826d2..51e99e43 100644 --- a/tests/test_user_error_handler.py +++ b/tests/test_user_error_handler.py @@ -6,6 +6,7 @@ tests.test_user_error_handler :copyright: © 2010 by the Pallets team. :license: BSD, see LICENSE for more details. """ +import pytest from werkzeug.exceptions import Forbidden from werkzeug.exceptions import HTTPException from werkzeug.exceptions import InternalServerError @@ -25,7 +26,13 @@ def test_error_handler_no_match(app, client): @app.errorhandler(500) def handle_500(e): - return type(e).__name__ + assert isinstance(e, InternalServerError) + original = getattr(e, "original_exception", None) + + if original is not None: + return "wrapped " + type(original).__name__ + + return "direct" @app.route("/custom") def custom_test(): @@ -35,9 +42,14 @@ def test_error_handler_no_match(app, client): def key_error(): raise KeyError() + @app.route("/abort") + def do_abort(): + flask.abort(500) + app.testing = False assert client.get("/custom").data == b"custom" - assert client.get("/keyerror").data == b"KeyError" + assert client.get("/keyerror").data == b"wrapped KeyError" + assert client.get("/abort").data == b"direct" def test_error_handler_subclass(app): @@ -194,3 +206,84 @@ def test_default_error_handler(): assert c.get("/forbidden").data == b"forbidden" # Don't handle RequestRedirect raised when adding slash. assert c.get("/slash", follow_redirects=True).data == b"slash" + + +class TestGenericHandlers(object): + """Test how very generic handlers are dispatched to.""" + + class Custom(Exception): + pass + + @pytest.fixture() + def app(self, app): + @app.route("/custom") + def do_custom(): + raise self.Custom() + + @app.route("/error") + def do_error(): + raise KeyError() + + @app.route("/abort") + def do_abort(): + flask.abort(500) + + @app.route("/raise") + def do_raise(): + raise InternalServerError() + + app.config["PROPAGATE_EXCEPTIONS"] = False + return app + + def report_error(self, e): + original = getattr(e, "original_exception", None) + + if original is not None: + return "wrapped " + type(original).__name__ + + return "direct " + type(e).__name__ + + @pytest.mark.parametrize("to_handle", (InternalServerError, 500)) + def test_handle_class_or_code(self, app, client, to_handle): + """``InternalServerError`` and ``500`` are aliases, they should + have the same behavior. Both should only receive + ``InternalServerError``, which might wrap another error. + """ + + @app.errorhandler(to_handle) + def handle_500(e): + assert isinstance(e, InternalServerError) + return self.report_error(e) + + assert client.get("/custom").data == b"wrapped Custom" + assert client.get("/error").data == b"wrapped KeyError" + assert client.get("/abort").data == b"direct InternalServerError" + assert client.get("/raise").data == b"direct InternalServerError" + + def test_handle_generic_http(self, app, client): + """``HTTPException`` should only receive ``HTTPException`` + subclasses. It will receive ``404`` routing exceptions. + """ + + @app.errorhandler(HTTPException) + def handle_http(e): + assert isinstance(e, HTTPException) + return str(e.code) + + assert client.get("/error").data == b"500" + assert client.get("/abort").data == b"500" + assert client.get("/not-found").data == b"404" + + def test_handle_generic(self, app, client): + """Generic ``Exception`` will handle all exceptions directly, + including ``HTTPExceptions``. + """ + + @app.errorhandler(Exception) + def handle_exception(e): + return self.report_error(e) + + assert client.get("/custom").data == b"direct Custom" + assert client.get("/error").data == b"direct KeyError" + assert client.get("/abort").data == b"direct InternalServerError" + assert client.get("/not-found").data == b"direct NotFound"