From 48e99e5c345a12e9d1332bf2c22c557295808c8c Mon Sep 17 00:00:00 2001 From: Ganesh Murthy Date: Mon, 27 Jun 2016 14:21:06 -0400 Subject: [PATCH] DISPATCH-373 - Added appropriate error messages for AMQP_NOT_FOUND messages. Added logging got linkRoute, address and autoLink --- .../management/agent.py | 3 ++- src/router_core/agent.c | 1 + src/router_core/agent_config_address.c | 20 ++++++++++++++--- src/router_core/agent_config_auto_link.c | 20 ++++++++++++++--- src/router_core/agent_config_link_route.c | 22 ++++++++++++++++--- src/router_core/router_core_private.h | 1 + tests/system_tests_link_routes.py | 2 +- 7 files changed, 58 insertions(+), 11 deletions(-) diff --git a/python/qpid_dispatch_internal/management/agent.py b/python/qpid_dispatch_internal/management/agent.py index 8fd4e84f2b..68d34ed6be 100644 --- a/python/qpid_dispatch_internal/management/agent.py +++ b/python/qpid_dispatch_internal/management/agent.py @@ -775,7 +775,8 @@ def receive(self, request, unused_link_id, unused_cost): """Called when a management request is received.""" def error(e, trace): """Raise an error""" - self.log(LOG_ERROR, "Error dispatching %s: %s\n%s"%(request, e, trace)) + self.log(LOG_ERROR, "Error performing %s of %s: %s"%(request.properties.get('operation'), + request.properties.get('type'), e)) self.respond(request, e.status, e.description) # If there's no reply_to, don't bother to process the request. diff --git a/src/router_core/agent.c b/src/router_core/agent.c index 272b75c986..9f15d634b5 100644 --- a/src/router_core/agent.c +++ b/src/router_core/agent.c @@ -88,6 +88,7 @@ qdr_query_t *qdr_query(qdr_core_t *core, query->context = context; query->body = body; query->more = false; + query->log_source = qd_log_source("AGENT"); return query; } diff --git a/src/router_core/agent_config_address.c b/src/router_core/agent_config_address.c index edb9ba56ab..f300150f62 100644 --- a/src/router_core/agent_config_address.c +++ b/src/router_core/agent_config_address.c @@ -42,6 +42,7 @@ const char *qdr_config_address_columns[] = "egressPhase", 0}; +const char *CONFIG_ADDRESS_TYPE = "org.apache.qpid.dispatch.router.config.address"; static void qdr_config_address_insert_column_CT(qdr_address_config_t *addr, int col, qd_composed_field_t *body, bool as_map) { @@ -67,7 +68,7 @@ static void qdr_config_address_insert_column_CT(qdr_address_config_t *addr, int } case QDR_CONFIG_ADDRESS_TYPE: - qd_compose_insert_string(body, "org.apache.qpid.dispatch.router.config.address"); + qd_compose_insert_string(body, CONFIG_ADDRESS_TYPE); break; case QDR_CONFIG_ADDRESS_PREFIX: @@ -259,8 +260,11 @@ void qdra_config_address_delete_CT(qdr_core_t *core, { qdr_address_config_t *addr = 0; - if (!name && !identity) + if (!name && !identity) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "No name or identity provided"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing DELETE of %s: %s", CONFIG_ADDRESS_TYPE, query->status.description); + } else { if (identity) addr = qdr_address_config_find_by_identity_CT(core, identity); @@ -311,11 +315,14 @@ void qdra_config_address_create_CT(qdr_core_t *core, if (!!addr) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "Name conflicts with an existing entity"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_ADDRESS_TYPE, query->status.description); break; } if (!qd_parse_is_map(in_body)) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "Body of request must be a map"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_ADDRESS_TYPE, query->status.description); break; } @@ -333,6 +340,8 @@ void qdra_config_address_create_CT(qdr_core_t *core, // if (!prefix_field) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "prefix field is mandatory"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_ADDRESS_TYPE, query->status.description); break; } @@ -347,6 +356,7 @@ void qdra_config_address_create_CT(qdr_core_t *core, if (!!addr) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "Address prefix conflicts with an existing entity"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_ADDRESS_TYPE, query->status.description); break; } @@ -369,6 +379,7 @@ void qdra_config_address_create_CT(qdr_core_t *core, if (in_phase < 0 || in_phase > 9 || out_phase < 0 || out_phase > 9) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "Phase values must be between 0 and 9"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_ADDRESS_TYPE, query->status.description); break; } @@ -440,8 +451,11 @@ void qdra_config_address_get_CT(qdr_core_t *core, { qdr_address_config_t *addr = 0; - if (!name && !identity) + if (!name && !identity) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "No name or identity provided"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing READ of %s: %s", CONFIG_ADDRESS_TYPE, query->status.description); + } else { if (identity) //If there is identity, ignore the name addr = qdr_address_config_find_by_identity_CT(core, identity); diff --git a/src/router_core/agent_config_auto_link.c b/src/router_core/agent_config_auto_link.c index 4f77348cc9..a3556e0bb6 100644 --- a/src/router_core/agent_config_auto_link.c +++ b/src/router_core/agent_config_auto_link.c @@ -49,6 +49,7 @@ const char *qdr_config_auto_link_columns[] = "lastError", 0}; +const char *CONFIG_AUTOLINK_TYPE = "org.apache.qpid.dispatch.router.config.autoLink"; static void qdr_config_auto_link_insert_column_CT(qdr_auto_link_t *al, int col, qd_composed_field_t *body, bool as_map) { @@ -73,7 +74,7 @@ static void qdr_config_auto_link_insert_column_CT(qdr_auto_link_t *al, int col, break; case QDR_CONFIG_AUTO_LINK_TYPE: - qd_compose_insert_string(body, "org.apache.qpid.dispatch.router.config.autoLink"); + qd_compose_insert_string(body, CONFIG_AUTOLINK_TYPE); break; case QDR_CONFIG_AUTO_LINK_ADDR: @@ -297,8 +298,11 @@ void qdra_config_auto_link_delete_CT(qdr_core_t *core, { qdr_auto_link_t *al = 0; - if (!name && !identity) + if (!name && !identity) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "No name or identity provided"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing DELETE of %s: %s", CONFIG_AUTOLINK_TYPE, query->status.description); + } else { if (identity) al = qdr_auto_link_config_find_by_identity_CT(core, identity); @@ -337,11 +341,14 @@ void qdra_config_auto_link_create_CT(qdr_core_t *core, if (!!al) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "Name conflicts with an existing entity"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_AUTOLINK_TYPE, query->status.description); break; } if (!qd_parse_is_map(in_body)) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "Body of request must be a map"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_AUTOLINK_TYPE, query->status.description); break; } @@ -359,6 +366,8 @@ void qdra_config_auto_link_create_CT(qdr_core_t *core, // if (!addr_field || !dir_field) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "addr and dir fields are mandatory"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_AUTOLINK_TYPE, query->status.description); break; } @@ -367,6 +376,7 @@ void qdra_config_auto_link_create_CT(qdr_core_t *core, if (error) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = error; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_AUTOLINK_TYPE, query->status.description); break; } @@ -382,6 +392,7 @@ void qdra_config_auto_link_create_CT(qdr_core_t *core, if (phase < 0 || phase > 9) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "autoLink phase must be between 0 and 9"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_AUTOLINK_TYPE, query->status.description); break; } @@ -450,8 +461,11 @@ void qdra_config_auto_link_get_CT(qdr_core_t *core, { qdr_auto_link_t *al = 0; - if (!name && !identity) + if (!name && !identity) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "No name or identity provided"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing READ of %s: %s", CONFIG_AUTOLINK_TYPE, query->status.description); + } else { if (identity) //If there is identity, ignore the name al = qdr_auto_link_config_find_by_identity_CT(core, identity); diff --git a/src/router_core/agent_config_link_route.c b/src/router_core/agent_config_link_route.c index e2a38bfbb2..33084cff00 100644 --- a/src/router_core/agent_config_link_route.c +++ b/src/router_core/agent_config_link_route.c @@ -45,6 +45,9 @@ const char *qdr_config_link_route_columns[] = "operStatus", 0}; +const char *CONFIG_LINKROUTE_TYPE = "org.apache.qpid.dispatch.router.config.linkRoute"; + +const qd_amqp_error_t QD_AMQP_NAME_IDENTITY_MISSING = { 400, "No name or identity provided" }; static void qdr_config_link_route_insert_column_CT(qdr_link_route_t *lr, int col, qd_composed_field_t *body, bool as_map) { @@ -70,7 +73,7 @@ static void qdr_config_link_route_insert_column_CT(qdr_link_route_t *lr, int col } case QDR_CONFIG_LINK_ROUTE_TYPE: - qd_compose_insert_string(body, "org.apache.qpid.dispatch.router.config.linkRoute"); + qd_compose_insert_string(body, CONFIG_LINKROUTE_TYPE); break; case QDR_CONFIG_LINK_ROUTE_PREFIX: @@ -311,8 +314,11 @@ void qdra_config_link_route_delete_CT(qdr_core_t *core, { qdr_link_route_t *lr = 0; - if (!name && !identity) + if (!name && !identity) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "No name or identity provided"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing DELETE of %s: %s", CONFIG_LINKROUTE_TYPE, query->status.description); + } else { if (identity) lr = qdr_link_route_config_find_by_identity_CT(core, identity); @@ -351,11 +357,14 @@ void qdra_config_link_route_create_CT(qdr_core_t *core, if (!!lr) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "Name conflicts with an existing entity"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_LINKROUTE_TYPE, query->status.description); break; } if (!qd_parse_is_map(in_body)) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "Body of request must be a map"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_LINKROUTE_TYPE, query->status.description); break; } @@ -373,6 +382,8 @@ void qdra_config_link_route_create_CT(qdr_core_t *core, // if (!prefix_field || !dir_field) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "prefix and dir fields are mandatory"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_LINKROUTE_TYPE, query->status.description); break; } @@ -381,6 +392,7 @@ void qdra_config_link_route_create_CT(qdr_core_t *core, if (error) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = error; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_LINKROUTE_TYPE, query->status.description); break; } @@ -389,6 +401,7 @@ void qdra_config_link_route_create_CT(qdr_core_t *core, if (error) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = error; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing CREATE of %s: %s", CONFIG_LINKROUTE_TYPE, query->status.description); break; } @@ -440,8 +453,11 @@ void qdra_config_link_route_get_CT(qdr_core_t *core, { qdr_link_route_t *lr = 0; - if (!name && !identity) + if (!name && !identity) { query->status = QD_AMQP_BAD_REQUEST; + query->status.description = "No name or identity provided"; + qd_log(query->log_source, QD_LOG_ERROR, "Error performing READ of %s: %s", CONFIG_LINKROUTE_TYPE, query->status.description); + } else { if (identity) //If there is identity, ignore the name lr = qdr_link_route_config_find_by_identity_CT(core, identity); diff --git a/src/router_core/router_core_private.h b/src/router_core/router_core_private.h index 3113165f7e..ee7f7b2a9f 100644 --- a/src/router_core/router_core_private.h +++ b/src/router_core/router_core_private.h @@ -162,6 +162,7 @@ struct qdr_query_t { int next_offset; bool more; qd_amqp_error_t status; + qd_log_source_t *log_source; }; DEQ_DECLARE(qdr_query_t, qdr_query_list_t); diff --git a/tests/system_tests_link_routes.py b/tests/system_tests_link_routes.py index 77844d6230..74a482837d 100644 --- a/tests/system_tests_link_routes.py +++ b/tests/system_tests_link_routes.py @@ -184,7 +184,7 @@ def test_aaa_qdmanage_query_link_route(self): out = self.run_qdmanage(cmd=cmd, address=self.routers[1].addresses[0]) except Exception, e: exception_occurred = True - self.assertTrue("BadRequestStatus: Bad Request" in e.message) + self.assertTrue("BadRequestStatus: No name or identity provided" in e.message) self.assertTrue(exception_occurred)