Improved error handling

Invalid SQL now shows a special error.html template, and is covered by tests.
This commit is contained in:
Simon Willison 2017-11-12 13:16:15 -08:00
commit 666aa03253
4 changed files with 53 additions and 20 deletions

View file

@ -140,6 +140,8 @@ class BaseView(HTTPMethodView):
as_json = False as_json = False
extra_template_data = {} extra_template_data = {}
start = time.time() start = time.time()
template = self.template
status_code = 200
try: try:
data, extra_template_data = await self.data( data, extra_template_data = await self.data(
request, name, hash, **kwargs request, name, hash, **kwargs
@ -148,7 +150,11 @@ class BaseView(HTTPMethodView):
data = { data = {
'ok': False, 'ok': False,
'error': str(e), 'error': str(e),
'database': name,
'database_hash': hash,
} }
template = 'error.html'
status_code = 400
end = time.time() end = time.time()
data['query_ms'] = (end - start) * 1000 data['query_ms'] = (end - start) * 1000
if as_json: if as_json:
@ -165,6 +171,7 @@ class BaseView(HTTPMethodView):
json.dumps( json.dumps(
data, cls=CustomJSONEncoder data, cls=CustomJSONEncoder
), ),
status=status_code,
content_type='application/json', content_type='application/json',
headers={ headers={
'Access-Control-Allow-Origin': '*' 'Access-Control-Allow-Origin': '*'
@ -180,10 +187,11 @@ class BaseView(HTTPMethodView):
'url_jsono': path_with_ext(request, '.jsono'), 'url_jsono': path_with_ext(request, '.jsono'),
}} }}
r = self.jinja.render( r = self.jinja.render(
self.template, template,
request, request,
**context, **context,
) )
r.status = status_code
# Set far-future cache expiry # Set far-future cache expiry
if self.cache_headers: if self.cache_headers:
r.headers['Cache-Control'] = 'max-age={}'.format( r.headers['Cache-Control'] = 'max-age={}'.format(

View file

@ -8,9 +8,6 @@
</head> </head>
<body> <body>
{% if error %}
<div style="padding: 1em; margin: 1em; border: 3px solid red;">{{ error }}</div>
{% endif %}
{% block content %} {% block content %}
{% endblock %} {% endblock %}

View file

@ -0,0 +1,14 @@
{% extends "base.html" %}
{% block title %}{{ database }}{% endblock %}
{% block content %}
<div class="hd"><a href="/">home</a> / <a href="/{{ database }}-{{ database_hash }}">{{ database }}</a></div>
<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_hash and database_hash[:6] }}">{{ database }}</h1>
{% if error %}
<div style="padding: 1em; margin: 1em; border: 3px solid red;">{{ error }}</div>
{% endif %}
{% endblock %}

View file

@ -6,7 +6,7 @@ import tempfile
@pytest.fixture(scope='module') @pytest.fixture(scope='module')
def three_table_app_client(): def app_client():
with tempfile.TemporaryDirectory() as tmpdir: with tempfile.TemporaryDirectory() as tmpdir:
filepath = os.path.join(tmpdir, 'four_tables.db') filepath = os.path.join(tmpdir, 'four_tables.db')
conn = sqlite3.connect(filepath) conn = sqlite3.connect(filepath)
@ -15,13 +15,13 @@ def three_table_app_client():
yield Datasette([filepath]).app().test_client yield Datasette([filepath]).app().test_client
def test_homepage(three_table_app_client): def test_homepage(app_client):
_, response = three_table_app_client.get('/') _, response = app_client.get('/')
assert response.status == 200 assert response.status == 200
assert 'four_tables' in response.text assert 'four_tables' in response.text
# Now try the JSON # Now try the JSON
_, response = three_table_app_client.get('/.json') _, response = app_client.get('/.json')
assert response.status == 200 assert response.status == 200
assert response.json.keys() == {'four_tables': 0}.keys() assert response.json.keys() == {'four_tables': 0}.keys()
d = response.json['four_tables'] d = response.json['four_tables']
@ -29,13 +29,13 @@ def test_homepage(three_table_app_client):
assert d['tables_count'] == 4 assert d['tables_count'] == 4
def test_database_page(three_table_app_client): def test_database_page(app_client):
_, response = three_table_app_client.get('/four_tables', allow_redirects=False) _, response = app_client.get('/four_tables', allow_redirects=False)
assert response.status == 302 assert response.status == 302
_, response = three_table_app_client.get('/four_tables') _, response = app_client.get('/four_tables')
assert 'four_tables' in response.text assert 'four_tables' in response.text
# Test JSON list of tables # Test JSON list of tables
_, response = three_table_app_client.get('/four_tables.json') _, response = app_client.get('/four_tables.json')
data = response.json data = response.json
assert 'four_tables' == data['database'] assert 'four_tables' == data['database']
assert [{ assert [{
@ -57,8 +57,8 @@ def test_database_page(three_table_app_client):
}] == data['tables'] }] == data['tables']
def test_custom_sql(three_table_app_client): def test_custom_sql(app_client):
_, response = three_table_app_client.get( _, response = app_client.get(
'/four_tables.jsono?sql=select+content+from+simple_primary_key' '/four_tables.jsono?sql=select+content+from+simple_primary_key'
) )
data = response.json data = response.json
@ -74,10 +74,24 @@ def test_custom_sql(three_table_app_client):
assert 'four_tables' == data['database'] assert 'four_tables' == data['database']
def test_table_page(three_table_app_client): def test_invalid_custom_sql(app_client):
_, response = three_table_app_client.get('/four_tables/simple_primary_key') _, response = app_client.get(
'/four_tables?sql=.schema'
)
assert response.status == 400
assert 'Statement must begin with SELECT' in response.text
_, response = app_client.get(
'/four_tables.json?sql=.schema'
)
assert response.status == 400
assert response.json['ok'] is False
assert 'Statement must begin with SELECT' == response.json['error']
def test_table_page(app_client):
_, response = app_client.get('/four_tables/simple_primary_key')
assert response.status == 200 assert response.status == 200
_, response = three_table_app_client.get('/four_tables/simple_primary_key.jsono') _, response = app_client.get('/four_tables/simple_primary_key.jsono')
assert response.status == 200 assert response.status == 200
data = response.json data = response.json
assert data['query']['sql'] == 'select * from "simple_primary_key" order by pk limit 51' assert data['query']['sql'] == 'select * from "simple_primary_key" order by pk limit 51'
@ -91,10 +105,10 @@ def test_table_page(three_table_app_client):
}] }]
def test_view(three_table_app_client): def test_view(app_client):
_, response = three_table_app_client.get('/four_tables/simple_view') _, response = app_client.get('/four_tables/simple_view')
assert response.status == 200 assert response.status == 200
_, response = three_table_app_client.get('/four_tables/simple_view.jsono') _, response = app_client.get('/four_tables/simple_view.jsono')
assert response.status == 200 assert response.status == 200
data = response.json data = response.json
assert data['rows'] == [{ assert data['rows'] == [{