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

AER-2539 error handling in service #45

Merged

Conversation

BertScholten
Copy link
Member

  • Retry-mechanism when Bing indicates that we've fired too many requests recently
  • Better error handling in the RxJava part, at least completing a search task if there's a runtime exception we didn't think about.

With this implementation runtime exceptions in a search service will cause the search task to return with a generic error message (and not wait for other running results). Another option would be to silently ignore it (well, log it, but who ever looks at the logs?).

There's a max requests per second Bing supports with non-enterprise keys (and probably also with enterprise keys, but it should be higher in that case).
If we hit that, wait a second before trying again. 1 second is pretty long, but as we're using the async process anyway, I think it's OK to do this(?)
As it was, a NullpointerException in a search service would cause the delegator to also throw an exception, and a task would never be set to completed. This would cause the client to wait forever (well, wait, it would keep on sending a where-are-my-results-request every 250ms) and the user would never get an indication that something failed.
The `doOnError` method does not keep the error from flowing down, it just does its action and then the subscriber will still have to handle the original exception. And as we didn't define an error handler there, another exception would pop up.
Instead of silently ignoring the error and just returning results from all other services, which would also be an option, I went with returning a pretty generic 'Failure during search' result that a user will receive. This hopefully should trigger the user to warn us about it, so we can investigate what is going wrong. For good measure did the same for the synchronous version.
Copy link
Member

@JornC JornC left a comment

Choose a reason for hiding this comment

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

ACK

@BertScholten BertScholten merged commit 3dcef09 into aerius:main Dec 5, 2023
1 check passed
@BertScholten BertScholten deleted the AER-2539_error_handling_in_service branch December 5, 2023 08:48
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