No tracebacks on normal errors and prettier error pages (#1768)

* Default error handler now only logs traceback on 500 errors and all responses are HTML formatted.

* Tests passing.

* Ability to flag any exception object with self.quiet = True following @ashleysommer suggestion.

* Refactor HTML formatting into errorpages.py. String escapes for debug tracebacks.

* Remove extra includes

* Auto-set quiet flag also when decorator is used.

* Cleanup, make error pages (probably) HTML5-compliant and similar for debug and non-debug modes.

* Fix lookup of non-existant status codes

* No logging of 503 errors after all.

* lint
This commit is contained in:
L. Kärkkäinen 2020-01-20 16:58:14 +02:00 committed by Stephen Sadowski
parent b565072ed9
commit ba9b432993
14 changed files with 164 additions and 213 deletions

117
sanic/errorpages.py Normal file
View File

@ -0,0 +1,117 @@
import sys
from traceback import extract_tb
from sanic.exceptions import SanicException
from sanic.helpers import STATUS_CODES
from sanic.response import html
# Here, There Be Dragons (custom HTML formatting to follow)
def escape(text):
"""Minimal HTML escaping, not for attribute values (unlike html.escape)."""
return f"{text}".replace("&", "&amp;").replace("<", "&lt;")
def exception_response(request, exception, debug):
status = 500
text = (
"The server encountered an internal error "
"and cannot complete your request."
)
headers = {}
if isinstance(exception, SanicException):
text = f"{exception}"
status = getattr(exception, "status_code", status)
headers = getattr(exception, "headers", headers)
elif debug:
text = f"{exception}"
status_text = STATUS_CODES.get(status, b"Error Occurred").decode()
title = escape(f"{status}{status_text}")
text = escape(text)
if debug and not getattr(exception, "quiet", False):
return html(
f"<!DOCTYPE html><meta charset=UTF-8><title>{title}</title>"
f"<style>{TRACEBACK_STYLE}</style>\n"
f"<h1>⚠️ {title}</h1><p>{text}\n"
f"{_render_traceback_html(request, exception)}",
status=status,
)
# Keeping it minimal with trailing newline for pretty curl/console output
return html(
f"<!DOCTYPE html><meta charset=UTF-8><title>{title}</title>"
"<style>html { font-family: sans-serif }</style>\n"
f"<h1>⚠️ {title}</h1><p>{text}\n",
status=status,
headers=headers,
)
def _render_exception(exception):
frames = extract_tb(exception.__traceback__)
frame_html = "".join(TRACEBACK_LINE_HTML.format(frame) for frame in frames)
return TRACEBACK_WRAPPER_HTML.format(
exc_name=escape(exception.__class__.__name__),
exc_value=escape(exception),
frame_html=frame_html,
)
def _render_traceback_html(request, exception):
exc_type, exc_value, tb = sys.exc_info()
exceptions = []
while exc_value:
exceptions.append(_render_exception(exc_value))
exc_value = exc_value.__cause__
traceback_html = TRACEBACK_BORDER.join(reversed(exceptions))
appname = escape(request.app.name)
name = escape(exception.__class__.__name__)
value = escape(exception)
path = escape(request.path)
return (
f"<h2>Traceback of {appname} (most recent call last):</h2>"
f"{traceback_html}"
"<div class=summary><p>"
f"<b>{name}: {value}</b> while handling path <code>{path}</code>"
)
TRACEBACK_STYLE = """
html { font-family: sans-serif }
h2 { color: #888; }
.tb-wrapper p { margin: 0 }
.frame-border { margin: 1rem }
.frame-line > * { padding: 0.3rem 0.6rem }
.frame-line { margin-bottom: 0.3rem }
.frame-code { font-size: 16px; padding-left: 4ch }
.tb-wrapper { border: 1px solid #eee }
.tb-header { background: #eee; padding: 0.3rem; font-weight: bold }
.frame-descriptor { background: #e2eafb; font-size: 14px }
"""
TRACEBACK_WRAPPER_HTML = (
"<div class=tb-header>{exc_name}: {exc_value}</div>"
"<div class=tb-wrapper>{frame_html}</div>"
)
TRACEBACK_BORDER = (
"<div class=frame-border>"
"The above exception was the direct cause of the following exception:"
"</div>"
)
TRACEBACK_LINE_HTML = (
"<div class=frame-line>"
"<p class=frame-descriptor>"
"File {0.filename}, line <i>{0.lineno}</i>, "
"in <code><b>{0.name}</b></code>"
"<p class=frame-code><code>{0.line}</code>"
"</div>"
)

View File

@ -1,133 +1,18 @@
from sanic.helpers import STATUS_CODES from sanic.helpers import STATUS_CODES
TRACEBACK_STYLE = """
<style>
body {
padding: 20px;
font-family: Arial, sans-serif;
}
p {
margin: 0;
}
.summary {
padding: 10px;
}
h1 {
margin-bottom: 0;
}
h3 {
margin-top: 10px;
}
h3 code {
font-size: 24px;
}
.frame-line > * {
padding: 5px 10px;
}
.frame-line {
margin-bottom: 5px;
}
.frame-code {
font-size: 16px;
padding-left: 30px;
}
.tb-wrapper {
border: 1px solid #f3f3f3;
}
.tb-header {
background-color: #f3f3f3;
padding: 5px 10px;
}
.tb-border {
padding-top: 20px;
}
.frame-descriptor {
background-color: #e2eafb;
}
.frame-descriptor {
font-size: 14px;
}
</style>
"""
TRACEBACK_WRAPPER_HTML = """
<html>
<head>
{style}
</head>
<body>
{inner_html}
<div class="summary">
<p>
<b>{exc_name}: {exc_value}</b>
while handling path <code>{path}</code>
</p>
</div>
</body>
</html>
"""
TRACEBACK_WRAPPER_INNER_HTML = """
<h1>{exc_name}</h1>
<h3><code>{exc_value}</code></h3>
<div class="tb-wrapper">
<p class="tb-header">Traceback (most recent call last):</p>
{frame_html}
</div>
"""
TRACEBACK_BORDER = """
<div class="tb-border">
<b><i>
The above exception was the direct cause of the
following exception:
</i></b>
</div>
"""
TRACEBACK_LINE_HTML = """
<div class="frame-line">
<p class="frame-descriptor">
File {0.filename}, line <i>{0.lineno}</i>,
in <code><b>{0.name}</b></code>
</p>
<p class="frame-code"><code>{0.line}</code></p>
</div>
"""
INTERNAL_SERVER_ERROR_HTML = """
<h1>Internal Server Error</h1>
<p>
The server encountered an internal error and cannot complete
your request.
</p>
"""
_sanic_exceptions = {} _sanic_exceptions = {}
def add_status_code(code): def add_status_code(code, quiet=None):
""" """
Decorator used for adding exceptions to :class:`SanicException`. Decorator used for adding exceptions to :class:`SanicException`.
""" """
def class_decorator(cls): def class_decorator(cls):
cls.status_code = code cls.status_code = code
if quiet or quiet is None and code != 500:
cls.quiet = True
_sanic_exceptions[code] = cls _sanic_exceptions[code] = cls
return cls return cls
@ -135,12 +20,16 @@ def add_status_code(code):
class SanicException(Exception): class SanicException(Exception):
def __init__(self, message, status_code=None): def __init__(self, message, status_code=None, quiet=None):
super().__init__(message) super().__init__(message)
if status_code is not None: if status_code is not None:
self.status_code = status_code self.status_code = status_code
# quiet=None/False/True with None meaning choose by status
if quiet or quiet is None and status_code not in (None, 500):
self.quiet = True
@add_status_code(404) @add_status_code(404)
class NotFound(SanicException): class NotFound(SanicException):

View File

@ -1,21 +1,13 @@
import sys from traceback import format_exc
from traceback import extract_tb, format_exc
from sanic.errorpages import exception_response
from sanic.exceptions import ( from sanic.exceptions import (
INTERNAL_SERVER_ERROR_HTML,
TRACEBACK_BORDER,
TRACEBACK_LINE_HTML,
TRACEBACK_STYLE,
TRACEBACK_WRAPPER_HTML,
TRACEBACK_WRAPPER_INNER_HTML,
ContentRangeError, ContentRangeError,
HeaderNotFound, HeaderNotFound,
InvalidRangeType, InvalidRangeType,
SanicException,
) )
from sanic.log import logger from sanic.log import logger
from sanic.response import html, text from sanic.response import text
class ErrorHandler: class ErrorHandler:
@ -40,35 +32,6 @@ class ErrorHandler:
self.cached_handlers = {} self.cached_handlers = {}
self.debug = False self.debug = False
def _render_exception(self, exception):
frames = extract_tb(exception.__traceback__)
frame_html = []
for frame in frames:
frame_html.append(TRACEBACK_LINE_HTML.format(frame))
return TRACEBACK_WRAPPER_INNER_HTML.format(
exc_name=exception.__class__.__name__,
exc_value=exception,
frame_html="".join(frame_html),
)
def _render_traceback_html(self, exception, request):
exc_type, exc_value, tb = sys.exc_info()
exceptions = []
while exc_value:
exceptions.append(self._render_exception(exc_value))
exc_value = exc_value.__cause__
return TRACEBACK_WRAPPER_HTML.format(
style=TRACEBACK_STYLE,
exc_name=exception.__class__.__name__,
exc_value=exception,
inner_html=TRACEBACK_BORDER.join(reversed(exceptions)),
path=request.path,
)
def add(self, exception, handler): def add(self, exception, handler):
""" """
Add a new exception handler to an already existing handler object. Add a new exception handler to an already existing handler object.
@ -166,27 +129,17 @@ class ErrorHandler:
:class:`Exception` :class:`Exception`
:return: :return:
""" """
self.log(format_exc()) quiet = getattr(exception, "quiet", False)
try: if quiet is False:
url = repr(request.url) try:
except AttributeError: url = repr(request.url)
url = "unknown" except AttributeError:
url = "unknown"
response_message = "Exception occurred while handling uri: %s" self.log(format_exc())
logger.exception(response_message, url) logger.exception("Exception occurred while handling uri: %s", url)
if issubclass(type(exception), SanicException): return exception_response(request, exception, self.debug)
return text(
"Error: {}".format(exception),
status=getattr(exception, "status_code", 500),
headers=getattr(exception, "headers", dict()),
)
elif self.debug:
html_output = self._render_traceback_html(exception, request)
return html(html_output, status=500)
else:
return html(INTERNAL_SERVER_ERROR_HTML, status=500)
class ContentRangeHandler: class ContentRangeHandler:

View File

@ -105,10 +105,7 @@ def test_app_handle_request_handler_is_none(app, monkeypatch):
request, response = app.test_client.get("/test") request, response = app.test_client.get("/test")
assert ( assert "'None' was returned while requesting a handler from the router" in response.text
response.text
== "Error: 'None' was returned while requesting a handler from the router"
)
@pytest.mark.parametrize("websocket_enabled", [True, False]) @pytest.mark.parametrize("websocket_enabled", [True, False])
@ -189,7 +186,7 @@ def test_handle_request_with_nested_sanic_exception(app, monkeypatch, caplog):
with caplog.at_level(logging.ERROR): with caplog.at_level(logging.ERROR):
request, response = app.test_client.get("/") request, response = app.test_client.get("/")
assert response.status == 500 assert response.status == 500
assert response.text == "Error: Mock SanicException" assert "Mock SanicException" in response.text
assert ( assert (
"sanic.root", "sanic.root",
logging.ERROR, logging.ERROR,

View File

@ -18,4 +18,4 @@ def test_bad_request_response(app):
app.run(host="127.0.0.1", port=42101, debug=False) app.run(host="127.0.0.1", port=42101, debug=False)
assert lines[0] == b"HTTP/1.1 400 Bad Request\r\n" assert lines[0] == b"HTTP/1.1 400 Bad Request\r\n"
assert lines[-1] == b"Error: Bad Request" assert b"Bad Request" in lines[-1]

View File

@ -172,7 +172,7 @@ def test_handled_unhandled_exception(exception_app):
request, response = exception_app.test_client.get("/divide_by_zero") request, response = exception_app.test_client.get("/divide_by_zero")
assert response.status == 500 assert response.status == 500
soup = BeautifulSoup(response.body, "html.parser") soup = BeautifulSoup(response.body, "html.parser")
assert soup.h1.text == "Internal Server Error" assert "Internal Server Error" in soup.h1.text
message = " ".join(soup.p.text.split()) message = " ".join(soup.p.text.split())
assert message == ( assert message == (
@ -218,4 +218,4 @@ def test_abort(exception_app):
request, response = exception_app.test_client.get("/abort/message") request, response = exception_app.test_client.get("/abort/message")
assert response.status == 500 assert response.status == 500
assert response.text == "Error: Abort" assert "Abort" in response.text

View File

@ -86,7 +86,7 @@ def test_html_traceback_output_in_debug_mode():
summary_text = " ".join(soup.select(".summary")[0].text.split()) summary_text = " ".join(soup.select(".summary")[0].text.split())
assert ( assert (
"NameError: name 'bar' " "is not defined while handling path /4" "NameError: name 'bar' is not defined while handling path /4"
) == summary_text ) == summary_text
@ -112,7 +112,7 @@ def test_chained_exception_handler():
summary_text = " ".join(soup.select(".summary")[0].text.split()) summary_text = " ".join(soup.select(".summary")[0].text.split())
assert ( assert (
"ZeroDivisionError: division by zero " "while handling path /6/0" "ZeroDivisionError: division by zero while handling path /6/0"
) == summary_text ) == summary_text

View File

@ -2,7 +2,7 @@ import logging
from asyncio import CancelledError from asyncio import CancelledError
from sanic.exceptions import NotFound from sanic.exceptions import NotFound, SanicException
from sanic.request import Request from sanic.request import Request
from sanic.response import HTTPResponse, text from sanic.response import HTTPResponse, text
@ -93,7 +93,7 @@ def test_middleware_response_raise_cancelled_error(app, caplog):
"sanic.root", "sanic.root",
logging.ERROR, logging.ERROR,
"Exception occurred while handling uri: 'http://127.0.0.1:42101/'", "Exception occurred while handling uri: 'http://127.0.0.1:42101/'",
) in caplog.record_tuples ) not in caplog.record_tuples
def test_middleware_response_raise_exception(app, caplog): def test_middleware_response_raise_exception(app, caplog):
@ -102,14 +102,16 @@ def test_middleware_response_raise_exception(app, caplog):
raise Exception("Exception at response middleware") raise Exception("Exception at response middleware")
with caplog.at_level(logging.ERROR): with caplog.at_level(logging.ERROR):
reqrequest, response = app.test_client.get("/") reqrequest, response = app.test_client.get("/fail")
assert response.status == 404 assert response.status == 404
# 404 errors are not logged
assert ( assert (
"sanic.root", "sanic.root",
logging.ERROR, logging.ERROR,
"Exception occurred while handling uri: 'http://127.0.0.1:42101/'", "Exception occurred while handling uri: 'http://127.0.0.1:42101/'",
) in caplog.record_tuples ) not in caplog.record_tuples
# Middleware exception ignored but logged
assert ( assert (
"sanic.error", "sanic.error",
logging.ERROR, logging.ERROR,

View File

@ -27,7 +27,7 @@ def test_payload_too_large_at_data_received_default(app):
response = app.test_client.get("/1", gather_request=False) response = app.test_client.get("/1", gather_request=False)
assert response.status == 413 assert response.status == 413
assert response.text == "Error: Payload Too Large" assert "Payload Too Large" in response.text
def test_payload_too_large_at_on_header_default(app): def test_payload_too_large_at_on_header_default(app):
@ -40,4 +40,4 @@ def test_payload_too_large_at_on_header_default(app):
data = "a" * 1000 data = "a" * 1000
response = app.test_client.post("/1", gather_request=False, data=data) response = app.test_client.post("/1", gather_request=False, data=data)
assert response.status == 413 assert response.status == 413
assert response.text == "Error: Payload Too Large" assert "Payload Too Large" in response.text

View File

@ -329,15 +329,12 @@ def test_request_stream_handle_exception(app):
# 404 # 404
request, response = app.test_client.post("/in_valid_post", data=data) request, response = app.test_client.post("/in_valid_post", data=data)
assert response.status == 404 assert response.status == 404
assert response.text == "Error: Requested URL /in_valid_post not found" assert "Requested URL /in_valid_post not found" in response.text
# 405 # 405
request, response = app.test_client.get("/post/random_id") request, response = app.test_client.get("/post/random_id")
assert response.status == 405 assert response.status == 405
assert ( assert "Method GET not allowed for URL /post/random_id" in response.text
response.text == "Error: Method GET not allowed for URL"
" /post/random_id"
)
def test_request_stream_blueprint(app): def test_request_stream_blueprint(app):

View File

@ -102,7 +102,7 @@ def test_default_server_error_request_timeout():
client = DelayableSanicTestClient(request_timeout_default_app, 2) client = DelayableSanicTestClient(request_timeout_default_app, 2)
request, response = client.get("/1") request, response = client.get("/1")
assert response.status == 408 assert response.status == 408
assert response.text == "Error: Request Timeout" assert "Request Timeout" in response.text
def test_default_server_error_request_dont_timeout(): def test_default_server_error_request_dont_timeout():
@ -125,4 +125,4 @@ def test_default_server_error_websocket_request_timeout():
request, response = client.get("/ws1", headers=headers) request, response = client.get("/ws1", headers=headers)
assert response.status == 408 assert response.status == 408
assert response.text == "Error: Request Timeout" assert "Request Timeout" in response.text

View File

@ -40,7 +40,7 @@ async def handler_2(request):
def test_default_server_error_response_timeout(): def test_default_server_error_response_timeout():
request, response = response_timeout_default_app.test_client.get("/1") request, response = response_timeout_default_app.test_client.get("/1")
assert response.status == 503 assert response.status == 503
assert response.text == "Error: Response Timeout" assert "Response Timeout" in response.text
response_handler_cancelled_app.flag = False response_handler_cancelled_app.flag = False
@ -65,5 +65,5 @@ async def handler_3(request):
def test_response_handler_cancelled(): def test_response_handler_cancelled():
request, response = response_handler_cancelled_app.test_client.get("/1") request, response = response_handler_cancelled_app.test_client.get("/1")
assert response.status == 503 assert response.status == 503
assert response.text == "Error: Response Timeout" assert "Response Timeout" in response.text
assert response_handler_cancelled_app.flag is False assert response_handler_cancelled_app.flag is False

View File

@ -238,7 +238,7 @@ def test_static_content_range_invalid_unit(
request, response = app.test_client.get("/testing.file", headers=headers) request, response = app.test_client.get("/testing.file", headers=headers)
assert response.status == 416 assert response.status == 416
assert response.text == "Error: {} is not a valid Range Type".format(unit) assert f"{unit} is not a valid Range Type" in response.text
@pytest.mark.parametrize("file_name", ["test.file", "decode me.txt"]) @pytest.mark.parametrize("file_name", ["test.file", "decode me.txt"])
@ -256,9 +256,7 @@ def test_static_content_range_invalid_start(
request, response = app.test_client.get("/testing.file", headers=headers) request, response = app.test_client.get("/testing.file", headers=headers)
assert response.status == 416 assert response.status == 416
assert response.text == "Error: '{}' is invalid for Content Range".format( assert f"'{start}' is invalid for Content Range" in response.text
start
)
@pytest.mark.parametrize("file_name", ["test.file", "decode me.txt"]) @pytest.mark.parametrize("file_name", ["test.file", "decode me.txt"])
@ -276,9 +274,7 @@ def test_static_content_range_invalid_end(
request, response = app.test_client.get("/testing.file", headers=headers) request, response = app.test_client.get("/testing.file", headers=headers)
assert response.status == 416 assert response.status == 416
assert response.text == "Error: '{}' is invalid for Content Range".format( assert f"'{end}' is invalid for Content Range" in response.text
end
)
@pytest.mark.parametrize("file_name", ["test.file", "decode me.txt"]) @pytest.mark.parametrize("file_name", ["test.file", "decode me.txt"])
@ -295,7 +291,7 @@ def test_static_content_range_invalid_parameters(
request, response = app.test_client.get("/testing.file", headers=headers) request, response = app.test_client.get("/testing.file", headers=headers)
assert response.status == 416 assert response.status == 416
assert response.text == "Error: Invalid for Content Range parameters" assert "Invalid for Content Range parameters" in response.text
@pytest.mark.parametrize( @pytest.mark.parametrize(
@ -369,7 +365,7 @@ def test_file_not_found(app, static_file_directory):
request, response = app.test_client.get("/static/not_found") request, response = app.test_client.get("/static/not_found")
assert response.status == 404 assert response.status == 404
assert response.text == "Error: File not found" assert "File not found" in response.text
@pytest.mark.parametrize("static_name", ["_static_name", "static"]) @pytest.mark.parametrize("static_name", ["_static_name", "static"])

View File

@ -49,7 +49,7 @@ def test_unexisting_methods(app):
request, response = app.test_client.get("/") request, response = app.test_client.get("/")
assert response.text == "I am get method" assert response.text == "I am get method"
request, response = app.test_client.post("/") request, response = app.test_client.post("/")
assert response.text == "Error: Method POST not allowed for URL /" assert "Method POST not allowed for URL /" in response.text
def test_argument_methods(app): def test_argument_methods(app):