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

3652 Catch APIError on multi-search requests due to bad query syntax #3659

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

albertisfu
Copy link
Contributor

Most of the errors in #3652 relate to queries with incorrect syntax.
I described in #3171 the cases that seem different.

The problem was that previously, when using the Search API, it returned these kinds of errors as a RequestError, which we caught and logged as a warning.

The Multi-search API now raises an APIError instead with the message Failed to parse query ...

To stop raising these errors on bad query syntax and to display this screen to users instead:
Screenshot 2024-01-24 at 14 51 02

Now it catch the APIError and only log a warning if the message error is Failed to parse query ....

  • Additionally I believe there could be other types of APIError not related to query parsing because some of the errors logged in Sentry appear to have valid queries. However, the error message in Sentry just shows: ApiError /, so it's better to log an error with the full message so we can identify if there are additional APIErrors that we should handle or fix.

@albertisfu albertisfu marked this pull request as ready for review January 24, 2024 20:59
@albertisfu albertisfu requested a review from mlissner January 24, 2024 20:59
Copy link
Member

@mlissner mlissner left a comment

Choose a reason for hiding this comment

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

Simple enough and a good improvement, thanks.

@mlissner mlissner enabled auto-merge January 24, 2024 21:01
@mlissner mlissner merged commit da91a3b into main Jan 24, 2024
11 checks passed
@mlissner mlissner deleted the 3652-fix-multi-search-api-error branch January 24, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants