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

♻️ Refactor pg exceptions to flask error handler #85

Merged
merged 2 commits into from
Feb 3, 2018

Conversation

dankolbman
Copy link
Contributor

Old error handling for sqlite breaks when we change backends. This implements a general flask exception handler for IntegrityError that processes error responses from postgres.

@dankolbman dankolbman changed the title ♻️ Refactor pg exceptions to flask error handler ♻️ Refactor pg exceptions to flask error handler Feb 2, 2018
@dankolbman dankolbman requested a review from znatty22 February 2, 2018 15:41
@dankolbman dankolbman self-assigned this Feb 2, 2018
@dankolbman dankolbman force-pushed the postgres-errors branch 2 times, most recently from 854bd25 to 753813c Compare February 2, 2018 15:54
'Cannot {0} {1}, {2} already has a {1}'

DEFAULT_INVALID_INPUT_TEMPLATE = 'Client error: invalid {} provided'
FOREIGN_KEY_RE = re.compile(r'DETAIL:.*\((?P<kf_id>\w{8})\)' +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you may want to change this regex to be a bit more general (i.e don't limit to 8 chars, etc). Right now requests like

POST /demographics
{
"participant_id": ""
}

result in the default error message of error saving changes.



def http_error(e):
""" Handles all HTTPExceptions """
return ErrorSchema().jsonify(e), e.code


def handle_integrity_error(**kwargs):
def integrity_error(e):
"""
Copy link
Member

@znatty22 znatty22 Feb 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update docstring

message = '{} "{}" does not exist'.format(m.group('table'),
m.group('kf_id'))

m = UNIQUE_RE.match(error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm the error message is a little confusing here. If someone tries to create a demographic for a participant that already has a demographic the error message is: participant "144WDN78" already exists

Can we make it something like: participant "144WDN78" already has demographic.
You could get the primary entity (demographic) by parsing e.statement (i.e. INSERT INTO demographic, UPDATE demographic etc).

@dankolbman dankolbman force-pushed the postgres-errors branch 2 times, most recently from 83392fa to ebb8e7a Compare February 3, 2018 22:14
@dankolbman dankolbman merged commit f930196 into postgres Feb 3, 2018
@dankolbman dankolbman deleted the postgres-errors branch February 3, 2018 22:26
@dankolbman dankolbman mentioned this pull request Apr 2, 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

Successfully merging this pull request may close these issues.

2 participants