Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleaner mechanism for handling custom errors #193

Closed
simonw opened this issue Apr 3, 2018 · 3 comments
Closed

Cleaner mechanism for handling custom errors #193

simonw opened this issue Apr 3, 2018 · 3 comments

Comments

@simonw
Copy link
Owner

simonw commented Apr 3, 2018

This code is pretty messy:

datasette/datasette/app.py

Lines 245 to 265 in 0abd3ab

if shape == 'object':
error = None
if 'primary_keys' not in data:
error = '_shape=object is only available on tables'
else:
pks = data['primary_keys']
if not pks:
error = '_shape=object not available for tables with no primary keys'
else:
object_rows = {}
for row in data['rows']:
pk_string = path_from_row_pks(row, pks, not pks)
object_rows[pk_string] = row
data['rows'] = object_rows
if error:
data = {
'ok': False,
'error': error,
'database': name,
'database_hash': hash,
}

Instead, it would be nice if I could raise an exception that would be converted into the appropriate JSON or HTML error message, with a corresponding HTTP code.

@carlmjohnson
Copy link

carlmjohnson commented Apr 6, 2018

You could try pulling out a validate query strings method. If it fails validation build the error object from the message. If it passes, you only need to go down a happy path.

@simonw
Copy link
Owner Author

simonw commented Apr 9, 2018

This is harder than I thought, because the _shape= logic actually runs AFTER the main block of code which is set up to catch exceptions - this code here:

datasette/datasette/app.py

Lines 200 to 216 in 0abd3ab

try:
response_or_template_contexts = await self.data(
request, name, hash, **kwargs
)
if isinstance(response_or_template_contexts, response.HTTPResponse):
return response_or_template_contexts
else:
data, extra_template_data, templates = response_or_template_contexts
except (sqlite3.OperationalError, InvalidSql) as e:
data = {
'ok': False,
'error': str(e),
'database': name,
'database_hash': hash,
}
status_code = 400
templates = ['error.html']

simonw added a commit that referenced this issue Apr 9, 2018
Verifies that they match an existing column, and only one or the other option
is provided - refs #189

Eses a new DatasetteError exception that closes #193
@simonw simonw closed this as completed in a87df96 Apr 9, 2018
@simonw
Copy link
Owner Author

simonw commented Apr 11, 2018

I can clean this up further with the mechanism I'm using for #184

@simonw simonw reopened this Apr 11, 2018
@simonw simonw closed this as completed in 9f28bbe Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants