-
Notifications
You must be signed in to change notification settings - Fork 59
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
add more precise error when rabbitMQ cannot be reached when storing raw files #3774
base: main
Are you sure you want to change the base?
Conversation
Quality Gate passedIssues Measures |
@@ -190,6 +191,11 @@ def create_raw( | |||
raw_data=RawDataMeta(id=raw_id, boefje_meta=raw_data.boefje_meta, mime_types=raw_data.mime_types), | |||
) | |||
event_manager.publish(event) | |||
except pika.exceptions.AMQPError as error: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this works, because Byte's RabbitMQEventManager.publish
already handles the AMQPError
exceptions and doesn't reraise them. That could explain the missing stack traces in the logs. However, while the RabbitMQEventManager.publish
handles these errors, it could potentially still produce an AMQPError
due to subsequent API calls via .e.g basic_publish
or queue_declare
within the exception handler. But in that case you would still see the warning "Reconnected to RabbitMQ because connection was closed" in the logs before the exception.
But imo the API router shouldn't be dealing with AMQP related stuff. As part of the separation of concerns principle, it's the EventManager that needs to be dealing with this technology details
Quality Gate passedIssues Measures |
Changes
Specifically catches the rabbitMQ errors which might occur if (even after the retries) it still cannot be reached.
The original 500 does not show the stacktrace in de logs, and as such there is no easy way for the user to know rabbitMQ might be at fault.
Issue link
Closes ...
Demo
Please add some proof in the form of screenshots or screen recordings to show (off) new functionality, if there are interesting new features for end-users.
QA notes
Please add some information for QA on how to test the newly created code.
Code Checklist
.env
changes files if required and changed the.env-dist
accordingly.Checklist for code reviewers:
Copy-paste the checklist from the docs/source/templates folder into your comment.
Checklist for QA:
Copy-paste the checklist from the docs/source/templates folder into your comment.