From: Ben Darnell Date: Mon, 4 Jul 2011 19:16:50 +0000 (-0700) Subject: Revamp error handling: replace get_error_html with write_error. X-Git-Tag: v2.1.0~127 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5de868438adad3b8554f57b3427e1c4082cca9dd;p=thirdparty%2Ftornado.git Revamp error handling: replace get_error_html with write_error. --- diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index ae1955f79..333555fa4 100644 --- a/tornado/test/web_test.py +++ b/tornado/test/web_test.py @@ -3,12 +3,13 @@ from tornado.iostream import IOStream from tornado.template import DictLoader from tornado.testing import LogTrapTestCase, AsyncHTTPTestCase from tornado.util import b, bytes_type -from tornado.web import RequestHandler, _O, authenticated, Application, asynchronous, url +from tornado.web import RequestHandler, _O, authenticated, Application, asynchronous, url, HTTPError import binascii import logging import re import socket +import sys class CookieTestRequestHandler(RequestHandler): # stub out enough methods to make the secure_cookie functions work @@ -359,3 +360,87 @@ js_embed() {u"path": u"foo"}) self.assertEqual(self.fetch_json("/optional_path/"), {u"path": None}) + + +class ErrorResponseTest(AsyncHTTPTestCase, LogTrapTestCase): + def get_app(self): + class DefaultHandler(RequestHandler): + def get(self): + if self.get_argument("status", None): + raise HTTPError(int(self.get_argument("status"))) + 1/0 + + class WriteErrorHandler(RequestHandler): + def get(self): + if self.get_argument("status", None): + self.send_error(int(self.get_argument("status"))) + else: + 1/0 + + def write_error(self, status_code, **kwargs): + self.set_header("Content-Type", "text/plain") + if "exc_info" in kwargs: + self.write("Exception: %s" % kwargs["exc_info"][0].__name__) + else: + self.write("Status: %d" % status_code) + + class GetErrorHtmlHandler(RequestHandler): + def get(self): + if self.get_argument("status", None): + self.send_error(int(self.get_argument("status"))) + else: + 1/0 + + def get_error_html(self, status_code, **kwargs): + self.set_header("Content-Type", "text/plain") + if "exception" in kwargs: + self.write("Exception: %s" % sys.exc_info()[0].__name__) + else: + self.write("Status: %d" % status_code) + + class FailedWriteErrorHandler(RequestHandler): + def get(self): + 1/0 + + def write_error(self, status_code, **kwargs): + raise Exception("exception in write_error") + + + return Application([ + url("/default", DefaultHandler), + url("/write_error", WriteErrorHandler), + url("/get_error_html", GetErrorHtmlHandler), + url("/failed_write_error", FailedWriteErrorHandler), + ]) + + def test_default(self): + response = self.fetch("/default") + self.assertEqual(response.code, 500) + self.assertTrue(b("500: Internal Server Error") in response.body) + + response = self.fetch("/default?status=503") + self.assertEqual(response.code, 503) + self.assertTrue(b("503: Service Unavailable") in response.body) + + def test_write_error(self): + response = self.fetch("/write_error") + self.assertEqual(response.code, 500) + self.assertEqual(b("Exception: ZeroDivisionError"), response.body) + + response = self.fetch("/write_error?status=503") + self.assertEqual(response.code, 503) + self.assertEqual(b("Status: 503"), response.body) + + def test_get_error_html(self): + response = self.fetch("/get_error_html") + self.assertEqual(response.code, 500) + self.assertEqual(b("Exception: ZeroDivisionError"), response.body) + + response = self.fetch("/get_error_html?status=503") + self.assertEqual(response.code, 503) + self.assertEqual(b("Status: 503"), response.body) + + def test_failed_write_error(self): + response = self.fetch("/failed_write_error") + self.assertEqual(response.code, 500) + self.assertEqual(b(""), response.body) diff --git a/tornado/web.py b/tornado/web.py index 648c5cc79..fbffbdfa3 100644 --- a/tornado/web.py +++ b/tornado/web.py @@ -643,9 +643,13 @@ class RequestHandler(object): def send_error(self, status_code=500, **kwargs): """Sends the given HTTP error code to the browser. - We also send the error HTML for the given error code as returned by - get_error_html. Override that method if you want custom error pages - for your application. + If `flush()` has already been called, it is not possible to send + an error, so this method will simply terminate the response. + If output has been written but not yet flushed, it will be discarded + and replaced with the error page. + + Override `write_error()` to customize the error page that is returned. + Additional keyword arguments are passed through to `write_error`. """ if self._headers_written: logging.error("Cannot send error response after headers written") @@ -654,30 +658,55 @@ class RequestHandler(object): return self.clear() self.set_status(status_code) - message = self.get_error_html(status_code, **kwargs) - self.finish(message) + try: + self.write_error(status_code, **kwargs) + except Exception: + logging.error("Uncaught exception in write_error", exc_info=True) + if not self._finished: + self.finish() - def get_error_html(self, status_code, **kwargs): + def write_error(self, status_code, **kwargs): """Override to implement custom error pages. - get_error_html() should return a string containing the error page, - and should not produce output via self.write(). If you use a - Tornado template for the error page, you must use - "return self.render_string(...)" instead of "self.render()". + ``write_error`` may call `write`, `render`, `set_header`, etc + to produce output as usual. + + If this error was caused by an uncaught exception, an ``exc_info`` + triple will be available as ``kwargs["exc_info"]``. Note that this + exception may not be the "current" exception for purposes of + methods like ``sys.exc_info()`` or ``traceback.format_exc``. - If this error was caused by an uncaught exception, the - exception object can be found in kwargs e.g. kwargs['exception'] + For historical reasons, if a method ``get_error_html`` exists, + it will be used instead of the default ``write_error`` implementation. + ``get_error_html`` returned a string instead of producing output + normally, and had different semantics for exception handling. + Users of ``get_error_html`` are encouraged to convert their code + to override ``write_error`` instead. """ - if self.settings.get("debug") and "exception" in kwargs: + if hasattr(self, 'get_error_html'): + if 'exc_info' in kwargs: + exc_info = kwargs.pop('exc_info') + kwargs['exception'] = exc_info[1] + try: + # Put the traceback into sys.exc_info() + raise exc_info[0], exc_info[1], exc_info[2] + except Exception: + self.finish(self.get_error_html(status_code, **kwargs)) + else: + self.finish(self.get_error_html(status_code, **kwargs)) + return + if self.settings.get("debug") and "exc_info" in kwargs: # in debug mode, try to send a traceback self.set_header('Content-Type', 'text/plain') - return traceback.format_exc() + for line in traceback.format_exception(*kwargs["exc_info"]): + self.write(line) + self.finish() else: - return "%(code)d: %(message)s" \ - "%(code)d: %(message)s" % { - "code": status_code, - "message": httplib.responses[status_code], - } + self.finish("%(code)d: %(message)s" + "%(code)d: %(message)s" % { + "code": status_code, + "message": httplib.responses[status_code], + }) @property def locale(self): @@ -970,13 +999,13 @@ class RequestHandler(object): logging.warning(format, *args) if e.status_code not in httplib.responses: logging.error("Bad HTTP status code: %d", e.status_code) - self.send_error(500, exception=e) + self.send_error(500, exc_info=sys.exc_info()) else: - self.send_error(e.status_code, exception=e) + self.send_error(e.status_code, exc_info=sys.exc_info()) else: logging.error("Uncaught exception %s\n%r", self._request_summary(), self.request, exc_info=True) - self.send_error(500, exception=e) + self.send_error(500, exc_info=sys.exc_info()) def _ui_module(self, name, module): def render(*args, **kwargs): diff --git a/website/sphinx/overview.rst b/website/sphinx/overview.rst index 0efb453ab..19471dc62 100644 --- a/website/sphinx/overview.rst +++ b/website/sphinx/overview.rst @@ -1,3 +1,5 @@ +.. currentmodule:: tornado.web + Overview ======== @@ -156,8 +158,8 @@ Here is an example demonstrating the ``initialize()`` method: Other methods designed for overriding include: -- ``get_error_html(self, status_code, exception=None, **kwargs)`` - - returns HTML (as a string) for use on error pages. +- ``write_error(self, status_code, exc_info=None, **kwargs)`` - + outputs HTML for use on error pages. - ``get_current_user(self)`` - see `User Authentication <#user-authentication>`_ below - ``get_user_locale(self)`` - returns ``locale`` object to use for the @@ -169,6 +171,38 @@ Other methods designed for overriding include: - ``set_default_headers(self)`` - may be used to set additional headers on the response (such as a custom ``Server`` header) +Error Handling +~~~~~~~~~~~~~~ + +There are three ways to return an error from a `RequestHandler`: + +1. Manually call `~tornado.web.RequestHandler.set_status` and output the + response body normally. +2. Call `~RequestHandler.send_error`. This discards + any pending unflushed output and calls `~RequestHandler.write_error` to + generate an error page. +3. Raise an exception. `tornado.web.HTTPError` can be used to generate + a specified status code; all other exceptions return a 500 status. + The exception handler uses `~RequestHandler.send_error` and + `~RequestHandler.write_error` to generate the error page. + +The default error page includes a stack trace in debug mode and a one-line +description of the error (e.g. "500: Internal Server Error") otherwise. +To produce a custom error page, override `RequestHandler.write_error`. +This method may produce output normally via methods such as +`~RequestHandler.write` and `~RequestHandler.render`. If the error was +caused by an exception, an ``exc_info`` triple will be passed as a keyword +argument (note that this exception is not guaranteed to be the current +exception in ``sys.exc_info``, so ``write_error`` must use e.g. +`traceback.format_exception` instead of `traceback.format_exc`). + +In Tornado 2.0 and earlier, custom error pages were implemented by overriding +``RequestHandler.get_error_html``, which returned the error page as a string +instead of calling the normal output methods (and had slightly different +semantics for exceptions). This method is still supported, but it is +deprecated and applications are encouraged to switch to +`RequestHandler.write_error`. + Redirection ~~~~~~~~~~~ diff --git a/website/sphinx/releases/next.rst b/website/sphinx/releases/next.rst index 9b5166fd6..5ac6921d1 100644 --- a/website/sphinx/releases/next.rst +++ b/website/sphinx/releases/next.rst @@ -38,6 +38,9 @@ New features itself. * `tornado.web.RequestHandler.set_default_headers` may be overridden to set headers in a way that does not get reset during error handling. +* `tornado.web.RequestHandler.write_error` replaces ``get_error_html`` as the + preferred way to generate custom error pages (``get_error_html`` is still + supported, but deprecated) Bug fixes diff --git a/website/sphinx/web.rst b/website/sphinx/web.rst index 2840c240a..5e85f76f4 100644 --- a/website/sphinx/web.rst +++ b/website/sphinx/web.rst @@ -47,7 +47,7 @@ .. automethod:: RequestHandler.render_string .. automethod:: RequestHandler.redirect .. automethod:: RequestHandler.send_error - .. automethod:: RequestHandler.get_error_html + .. automethod:: RequestHandler.write_error .. automethod:: RequestHandler.clear