From d7b21af028a2beb2a044dcf6ee1e49bed4db9fa1 Mon Sep 17 00:00:00 2001 From: Stanislav Hejny Date: Mon, 5 May 2025 15:55:47 +0100 Subject: [PATCH] #40 - ensure return is a JSON with 'detail' field in case of the error --- amqp/adapter/cleverthis_service_adapter.py | 36 ++++++++++++++++------ amqp/adapter/data_response_factory.py | 16 +++++++++- amqp/model/model.py | 27 ++++++++++++++++ cleverswarm/clever_swarm_adapter.py | 22 ++++++------- 4 files changed, 79 insertions(+), 22 deletions(-) diff --git a/amqp/adapter/cleverthis_service_adapter.py b/amqp/adapter/cleverthis_service_adapter.py index 9754d4a..126e794 100644 --- a/amqp/adapter/cleverthis_service_adapter.py +++ b/amqp/adapter/cleverthis_service_adapter.py @@ -24,7 +24,7 @@ from amqp.adapter.data_response_factory import DataResponseFactory from amqp.adapter.file_handler import FileHandler, StreamingFileHandler from amqp.adapter.logging_utils import logging_error, logging_debug, logging_info from amqp.config.amq_configuration import AMQConfiguration -from amqp.model.model import DataMessage, DataResponse +from amqp.model.model import DataMessage, DataResponse, AMQErrorMessage from amqp.router.router_base import AMQ_REPLY_TO_EXCHANGE from amqp.router.utils import await_result @@ -208,8 +208,10 @@ class CleverThisServiceAdapter: else: raise ValueError(f"Unexpected HTTP method: {message.method}") else: - _amq_response = DataResponseFactory.of( - message.id, 401, "text/plain", b"Unauthorized", message.trace_info + _amq_response = DataResponseFactory.of_error_message( + message.id, + AMQErrorMessage(401, "Unauthorized", "Unauthorized"), + message.trace_info, ) logging_info( f"ALL DONE in on_message id:{message.id}, resp code:{_amq_response.response_code}" @@ -244,11 +246,9 @@ class CleverThisServiceAdapter: await _http_resp.read(), message.trace_info, ) - return DataResponseFactory.of( + return DataResponseFactory.of_error_message( message.id, - 404, - "text/plain", - str("Destination location does not exists").encode("utf-8"), + AMQErrorMessage(404, "Not Found", "Destination location does not exists"), message.trace_info, ) @@ -399,10 +399,18 @@ class CleverThisServiceAdapter: message.trace_info, ) except HTTPException as http_error: + logging_error(f"Service REST endpoint invocation failed: {http_error}") _content = str(http_error.detail) if not is_likely_json(_content): - _content = '{"error": "' + _content + '"}' - logging_error(f"Service REST endpoint invocation failed: {http_error}") + return DataResponseFactory.of_error_message( + message.id, + AMQErrorMessage( + http_error.status_code, + "Service Invocation Failed", + _content, + ), + message.trace_info, + ) return DataResponseFactory.of( message.id, http_error.status_code, @@ -414,7 +422,15 @@ class CleverThisServiceAdapter: logging_error(f"Service endpoint invocation failed: {error}") _content = str(error) if not is_likely_json(_content): - _content = '{"error": "' + _content + '"}' + return DataResponseFactory.of_error_message( + message.id, + AMQErrorMessage( + 500, + "Service Invocation Failed", + _content, + ), + message.trace_info, + ) return DataResponseFactory.of( message.id, 500, diff --git a/amqp/adapter/data_response_factory.py b/amqp/adapter/data_response_factory.py index 5a7048d..3ec3f2d 100644 --- a/amqp/adapter/data_response_factory.py +++ b/amqp/adapter/data_response_factory.py @@ -5,7 +5,7 @@ method to create, serialize and deserialize DataResponse message import base64 import re from io import BytesIO -from amqp.model.model import DataResponse +from amqp.model.model import DataResponse, AMQErrorMessage from amqp.model.snowflake_id import SnowflakeId from amqp.adapter.serializer import parse_map_string, map_as_string @@ -51,6 +51,20 @@ class DataResponseFactory: trace_info=trace_info, ) + @staticmethod + def of_error_message( + id: SnowflakeId, + error_message: AMQErrorMessage, + trace_info: dict[str, str], + ) -> DataResponse: + return DataResponseFactory.of( + id, + error_message.response_code, + error_message.content_type, + error_message.serialze(), + trace_info, + ) + @staticmethod def serialize(response: DataResponse) -> bytes: """ diff --git a/amqp/model/model.py b/amqp/model/model.py index bb42adc..1825195 100644 --- a/amqp/model/model.py +++ b/amqp/model/model.py @@ -305,3 +305,30 @@ class ScalingRequest: data["requestType"] ), # Convert string to Enum ) + + +class AMQErrorMessage: + """ + The error response message to RPC style call (note: the request is made with DataMessage) + """ + + def __init__( + self, + response_code: int, + error: str, + detail: str, + content_type: str = "application/json", + ): + self.response_code = response_code + self.error = error + self.detail = detail + self.content_type = content_type + + def serialze(self) -> bytes: + return json.dumps( + { + "response_code": self.response_code, + "error": self.error, + "detail": self.detail, + } + ).encode("utf-8") diff --git a/cleverswarm/clever_swarm_adapter.py b/cleverswarm/clever_swarm_adapter.py index e5b83d7..13b781e 100644 --- a/cleverswarm/clever_swarm_adapter.py +++ b/cleverswarm/clever_swarm_adapter.py @@ -26,7 +26,7 @@ from amqp.adapter.data_parser import AMQDataParser from amqp.adapter.data_response_factory import AMQResponseFactory from amqp.adapter.cleverthis_service_adapter import CleverThisServiceAdapter, AMQMessage from amqp.config.amq_configuration import AMQConfiguration -from amqp.model.model import AMQResponse +from amqp.model.model import AMQResponse, AMQErrorMessage from amqp.service.amq_service import AMQService @@ -220,12 +220,12 @@ class CleverSwarmAmqpAdapter(CleverThisServiceAdapter): loop=self.loop, ) - return AMQResponseFactory.of( + return AMQResponseFactory.of_error_message( message.id, - 404, - "text/plain", - str(f"Destination location [{message.path}] does not exists").encode( - "utf-8" + AMQErrorMessage( + 404, + "Not Found", + f"Destination location [{message.path}] does not exists", ), message.trace_info, ) @@ -293,12 +293,12 @@ class CleverSwarmAmqpAdapter(CleverThisServiceAdapter): loop=self.loop, ) - return AMQResponseFactory.of( + return AMQResponseFactory.of_error_message( message.id, - 404, - "text/plain", - str(f"Destination location [{message.path}] does not exists").encode( - "utf-8" + AMQErrorMessage( + 404, + "Not Found", + f"Destination location [{message.path}] does not exists", ), message.trace_info, )