Better mechanism for handling errors; 404s for missing table/database

New error mechanism closes #193

404s for missing tables/databesse closes #184

Makes pull request #202 unnecessary.

datasette/app.py

@@ -47,7 +47,11 @@ connections = threading.local()
 
 
 class DatasetteError(Exception):
-    pass
+    def __init__(self, message, title=None, error_dict=None, status=500, template=None):
+        self.message = message
+        self.title = title
+        self.error_dict = error_dict or {}
+        self.status = status
 
 
 class RenderMixin(HTTPMethodView):
@@ -200,14 +204,11 @@ class BaseView(RenderMixin):
             else:
                 data, extra_template_data, templates = response_or_template_contexts
         except (sqlite3.OperationalError, InvalidSql, DatasetteError) as e:
-            data = {
+            raise DatasetteError(str(e), title='Invalid SQL', status=400)
-                'ok': False,
+        except (sqlite3.OperationalError) as e:
-                'error': str(e),
+            raise DatasetteError(str(e))
-                'database': name,
+        except DatasetteError:
-                'database_hash': hash,
+            raise
-            }
-            status_code = 400
-            templates = ['error.html']
         end = time.time()
         data['query_ms'] = (end - start) * 1000
         for key in ('source', 'source_url', 'license', 'license_url'):
@@ -554,19 +555,28 @@ class TableView(RowTableShared):
         canned_query = self.ds.get_canned_query(name, table)
         if canned_query is not None:
             return await self.custom_sql(request, name, hash, canned_query['sql'], editable=False, canned_query=table)
-        is_view = bool(list(await self.execute(name, "SELECT count(*) from sqlite_master WHERE type = 'view' and name=:n", {
+        is_view = bool(list(await self.execute(
-            'n': table,
+            name,
-        }))[0][0])
+            "SELECT count(*) from sqlite_master WHERE type = 'view' and name=:n",
+            {'n': table}
+        ))[0][0])
         view_definition = None
         table_definition = None
         if is_view:
-            view_definition = list(await self.execute(name, 'select sql from sqlite_master where name = :n and type="view"', {
+            view_definition = list(await self.execute(
-                'n': table,
+                name,
-            }))[0][0]
+                'select sql from sqlite_master where name = :n and type="view"',
+                {'n': table}
+            ))[0][0]
         else:
-            table_definition = list(await self.execute(name, 'select sql from sqlite_master where name = :n and type="table"', {
+            table_definition_rows = list(await self.execute(
-                'n': table,
+                name,
-            }))[0][0]
+                'select sql from sqlite_master where name = :n and type="table"',
+                {'n': table}
+            ))
+            if not table_definition_rows:
+                raise NotFound('Table not found: {}'.format(table))
+            table_definition = table_definition_rows[0][0]
         info = self.ds.inspect()
         table_info = info[name]['tables'].get(table) or {}
         pks = table_info.get('primary_keys') or []
@@ -1199,4 +1209,36 @@ class Datasette:
             RowView.as_view(self),
             '/<db_name:[^/]+>/<table:[^/]+?>/<pk_path:[^/]+?><as_json:(\.jsono?)?$>'
         )
+
+        @app.exception(Exception)
+        def on_exception(request, exception):
+            title = None
+            if isinstance(exception, NotFound):
+                status = 404
+                info = {}
+                message = exception.args[0]
+            elif isinstance(exception, DatasetteError):
+                status = exception.status
+                info = exception.error_dict
+                message = exception.message
+                title = exception.title
+            else:
+                status = 500
+                info = {}
+                message = str(exception)
+            templates = ['500.html']
+            if status != 500:
+                templates = ['{}.html'.format(status)] + templates
+            info.update({
+                'ok': False,
+                'error': message,
+                'status': status,
+                'title': title,
+            })
+            if (request.path.split('?')[0].endswith('.json')):
+                return response.json(info, status=status)
+            else:
+                template = self.jinja_env.select_template(templates)
+                return response.html(template.render(info), status=status)
+
         return app

datasette/templates/500.html

@@ -0,0 +1,12 @@
+{% extends "base.html" %}
+
+{% block title %}{% if title %}{{ title }}{% else %}Error {{ status }}{% endif %}{% endblock %}
+
+{% block content %}
+<div class="hd"><a href="/">home</a></div>
+
+<h1>{% if title %}{{ title }}{% else %}Error {{ status }}{% endif %}</h1>
+
+<div style="padding: 1em; margin: 1em 0; border: 3px solid red;">{{ error }}</div>
+
+{% endblock %}

datasette/templates/base.html

@@ -20,17 +20,19 @@
 <div class="ft">
     Powered by <a href="https://github.com/simonw/datasette" title="Datasette v{{ datasette_version }}">Datasette</a>
     {% if query_ms %}&middot; Query took {{ query_ms|round(3) }}ms{% endif %}
-    {% if metadata.license or metadata.license_url %}&middot; Data license:
+    {% if metadata %}
-        {% if metadata.license_url %}
+        {% if metadata.license or metadata.license_url %}&middot; Data license:
-            <a href="{{ metadata.license_url }}">{{ metadata.license or metadata.license_url }}</a>
+            {% if metadata.license_url %}
-        {% else %}
+                <a href="{{ metadata.license_url }}">{{ metadata.license or metadata.license_url }}</a>
-            {{ metadata.license }}
+            {% else %}
+                {{ metadata.license }}
+            {% endif %}
+        {% endif %}
+        {% if metadata.source or metadata.source_url %}&middot;
+            Data source: {% if metadata.source_url %}
+                <a href="{{ metadata.source_url }}">
+            {% endif %}{{ metadata.source or metadata.source_url }}{% if metadata.source_url %}</a>{% endif %}
         {% endif %}
-    {% endif %}
-    {% if metadata.source or metadata.source_url %}&middot;
-        Data source: {% if metadata.source_url %}
-            <a href="{{ metadata.source_url }}">
-        {% endif %}{{ metadata.source or metadata.source_url }}{% if metadata.source_url %}</a>{% endif %}
     {% endif %}
 </div>
 {% if select_templates %}<!-- Templates considered: {{ select_templates|join(", ") }} -->{% endif %}

datasette/templates/error.html

@@ -1,14 +0,0 @@
-{% 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 0; border: 3px solid red;">{{ error }}</div>
-{% endif %}
-
-{% endblock %}

tests/test_api.py

@@ -208,6 +208,17 @@ def test_table_json(app_client):
     }]
 
 
+def test_table_not_exists_json(app_client):
+    assert {
+        'ok': False,
+        'error': 'Table not found: blah',
+        'status': 404,
+        'title': None,
+    } == app_client.get(
+        '/test_tables/blah.json', gather_request=False
+    ).json
+
+
 def test_jsono_redirects_to_shape_objects(app_client):
     response_1 = app_client.get(
         '/test_tables/simple_primary_key.jsono',

tests/test_html.py

@@ -210,6 +210,12 @@ def test_row_html_simple_primary_key(app_client):
     ] == [[str(td) for td in tr.select('td')] for tr in table.select('tbody tr')]
 
 
+def test_table_not_exists(app_client):
+    assert 'Table not found: blah' in app_client.get(
+        '/test_tables/blah', gather_request=False
+    ).body.decode('utf8')
+
+
 def test_table_html_no_primary_key(app_client):
     response = app_client.get('/test_tables/no_primary_key', gather_request=False)
     table = Soup(response.body, 'html.parser').find('table')