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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

add more precise error when rabbitMQ cannot be reached when storing raw files #3774

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3087ee1
add more precise error when rabbitMQ cannot be reached when storing r…
underdarknl Oct 31, 2024
bb61f1a
Update router.py
underdarknl Oct 31, 2024
353685a
Merge branch 'main' into feat/rabbit-mq-missing-exception
underdarknl Jan 1, 2025
562cdba
Merge branch 'main' into feat/rabbit-mq-missing-exception
underdarknl Jan 9, 2025
f884104
Merge branch 'main' into feat/rabbit-mq-missing-exception
underdarknl Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions bytes/bytes/api/router.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from base64 import b64decode
from uuid import UUID

import pika.exceptions
import structlog
from cachetools import TTLCache, cached
from fastapi import APIRouter, Depends, HTTPException, Query
Expand Down Expand Up @@ -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:
Copy link
Contributor

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

logger.exception("Error sending 'new raw data' event to RabbitMQ")
raise HTTPException(
underdarknl marked this conversation as resolved.
Show resolved Hide resolved
status_code=codes.INTERNAL_SERVER_ERROR, detail="Error sending 'new raw data' event to RabbitMQ"
) from error
except Exception as error:
logger.exception("Error saving raw data")
raise HTTPException(status_code=codes.INTERNAL_SERVER_ERROR, detail="Could not save raw data") from error
Expand Down
Loading