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

explicitly check for error state in bento results from summon #92

Merged
merged 3 commits into from
Sep 30, 2020

Conversation

magibney
Copy link
Contributor

otherwise this errors out when expecting a number in total_items
See: #80

otherwise this errors out when expecting a number in total_items
See: #80
@magibney
Copy link
Contributor Author

Note: this would still be "fixed" without the Gemfile.lock upgrade of bento_search. The only thing the upgrade gets us is a slightly more informative error message. 84fa596 initially just echoes this back to user. In addition (or perhaps instead) we probably want to log this to notify us of these errors so that we can follow up on them ... @mksndz thoughts?

@mksndz
Copy link
Member

mksndz commented Sep 30, 2020

If we're going to continue to work on a fork of bento_search, that repo should be moved under the UPenn org so that it is accessible to the whole team.

To use a Honeybadger notification, you can call Honeybadger.notify. The context hash param is particularly useful.

Copy link
Member

@mksndz mksndz left a comment

Choose a reason for hiding this comment

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

Looks good!

@magibney magibney merged commit 13da156 into master Sep 30, 2020
@magibney magibney deleted the 80_check_for_summon_error_state branch September 30, 2020 13:55
magibney added a commit that referenced this pull request Sep 30, 2020
…n_qparser_errors

follow-up to PR #92; mitigate server-side summon API qparser errors
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