Skip to content
Snippets Groups Projects

catch query KeyError and return an informative message to the user

Merged catch query KeyError and return an informative message to the user
fix-258 into master
1 unresolved thread
Merged Alexander Pace requested to merge fix-258 into master
1 unresolved thread

"fixes": #258 (closed)

now instead of returning a 500 error and emailing the admins, if a user queries gpstimes under the created: field, they'll now see this on the web:

Screen_Shot_2023-07-25_at_12.00.36_PM

And this with the API:

Screen_Shot_2023-07-25_at_12.02.13_PM

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
gracedb/search/forms.py
21 21 htmlEntityStar = "★"
22 22 errorMarker = '<span style="color:red;">'+htmlEntityStar+'</span>'
23 23
24 # Helpful status messages:
25 created_vs_t0 = 'Invalid query. Hint: date queries on the created: field take the form YYYY-MM-DD .. ' \
26 'YYYY-MM-DD. Date queries with gpstime are supported using the t_0: field'
27
28
29 # Helper function to capture superevent created vs t_0 gpstime queries
30 # and return it to the user:
31 def se_gpstime_parseerror(err):
32 if "ParseResults" in str(err) and 'miltime' in str(err):
  • Daniel Wysocki
    Daniel Wysocki @daniel.wysocki ·
    Maintainer

    Is there a better approach than parsing str(err)? I worry this may be unstable with changes to pyparsing

  • Alexander Pace
    Alexander Pace @alexander.pace ·
    Author Owner

    i don't know tbh. I thought the important bit was to capture the KeyError. if the cryptic output isn't part of the error string, then it will just default to returning a generic 400 to the user.

  • Daniel Wysocki
    Daniel Wysocki @daniel.wysocki ·
    Maintainer

    Good point, I hadn't appreciated the worst case scenario was a 400 error.

    Also, playing around with KeyError myself, it seems the info in str(err) is also accessible in err.args without doing a conversion to str, which has basically the same downsides. So I think we might as well stick to your current implementation.

  • Please register or sign in to reply
  • Alexander Pace added 3 commits

    added 3 commits

    • 9781df83 - 1 commit from branch master
    • d32618c0 - catch query KeyError and return an informative message to the user
    • a8ac2bed - unbreak api queries caused by copy/paste error

    Compare with previous version

  • Alexander Pace enabled an automatic merge when the pipeline for a8ac2bed succeeds

    enabled an automatic merge when the pipeline for a8ac2bed succeeds

  • Alexander Pace merged

    merged

    • Please register or sign in to reply