Skip to content

Commit 49de7cc

Browse files
bhatia-dibhatia-di
authored
fix(billing-cost-management): classify remaining local validation failures in aws-pricing, bcm-pricing-calc, cost-explorer, cost-anomaly, cost-allocation-tags, and cost-category (#4740)
* fix(billing-cost-management): classify remaining local validation failures in aws-pricing, bcm-pricing-calc, cost-explorer, cost-anomaly, cost-allocation-tags, and cost-category The remaining local parameter checks in these tools (unknown or unsupported operation, missing operation-specific parameters, unparseable JSON filter arguments) returned format_response('error', ...) with no top-level classification, so failures landed in UNKNOWN/UNKNOWN telemetry buckets. aws-pricing, bcm-pricing-calc, and cost-explorer route them through a per-module _validation_error helper; the single-site cost-allocation-tags and cost-category returns pass the kwargs inline. Each sets top-level error_type='validation_error', operation, and service, matching what handle_aws_error attaches to AWS-side errors in the same tool. cost-anomaly ValidationException responses now carry the AWS error code as error_type plus operation and service, as handle_aws_error would. The bcm-pricing-calc invalid-operation message listed operations the tool does not support; it now lists VALID_OPERATIONS. The legacy data payload (including error_code='MISSING_PARAMETER') and messages are preserved for backward compatibility. * refactor(billing-cost-management): share a format_validation_error helper instead of per-module _validation_error copies Each tool module defined its own _validation_error wrapper that differed only in the service name. Add format_validation_error to utilities/aws_service_base next to format_response, taking operation and service as keyword arguments, and call it directly from aws-pricing, bcm-pricing-calc, cost-explorer, cost-allocation-tags, and cost-category. Responses are unchanged. * refactor(billing-cost-management): call format_response directly instead of per-module validation helpers format_response already accepts error_type, operation, and service, and cost-anomaly and regional_fanout call it that way. Drop the _validation_error wrappers in aws-pricing, bcm-pricing-calc, and cost-explorer and pass the kwargs inline with each module's service-name constant. Add COST_EXPLORER_SERVICE_NAME to cost_explorer_tools and use it for the existing 'Cost Explorer' literals. Responses are unchanged.
1 parent 6798a1f commit 49de7cc

13 files changed

Lines changed: 147 additions & 14 deletions

‎src/billing-cost-management-mcp-server/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4545
- Corrected AWS Compute Optimizer ECS and Lambda recommendation response field names that read non-existent SDK fields and returned null. ECS now reads `currentPerformanceRisk`, `autoScalingConfiguration`, and `projectedUtilizationMetrics` (previously the non-existent `currentPerformance`, `autoScalingGroupArn`, and `projectedPerformance`), and Lambda derives the function name from the ARN (there is no `functionName` field).
4646
- Fixed `aws-pricing` tool error responses omitting a structured error type. Each `aws-pricing` operation caught exceptions and returned `format_response('error', ...)` (everything nested under `data`, no top-level `error_type`/`operation`), so the `handle_aws_error` call in the tool wrapper was never reached and failures surfaced without a classifiable type. Operation error paths now route through `handle_aws_error` so responses carry top-level `error_type` and `operation`, and the two non-exception "no results" cases (`get_service_attributes` service-not-found, `get_pricing_from_api` empty price list) carry `error_type='no_results'` plus `operation`. The legacy `data` payload is preserved for backward compatibility.
4747
- Fixed `session-sql` tool error responses omitting a structured error type. `execute_session_sql` caught every failure and returned a bare `{'status': 'error', 'message': ...}` with no `error_type`/`operation`, so failures surfaced without a classifiable type. Error responses now carry top-level `error_type`, `operation` (`session_sql`), and `service` (`SQL`). The `error_type` is the exception class name (e.g. `OperationalError`, `IntegrityError`, `ValueError`) -- a fixed identifier that never contains the query text or bound values, so no query content is exposed. The legacy `status='error'` and `message` payload is preserved for backward compatibility.
48+
- Fixed `aws-pricing`, `bcm-pricing-calc`, `cost-explorer`, `cost-anomaly`, `list-cost-allocation-tags`, and `list-cost-category-definitions` local error responses missing a structured error type. Their remaining parameter checks (an unknown or unsupported `operation`, a missing operation-specific parameter such as `service_code`, `attribute_name`, `identifier`, `workload_estimate_id`, `dimension`, or `metric`, and unparseable `tag_keys`/`supported_resource_types` JSON) returned `format_response('error', ...)` with no top-level classification, so failures surfaced as `UNKNOWN`/`UNKNOWN` in telemetry. Error responses now carry top-level `error_type='validation_error'`, `operation`, and `service`, matching what `handle_aws_error` already attaches to AWS-side errors in the same tool. `cost-anomaly` `ValidationException` responses now carry the AWS error code as `error_type`, plus `operation` and `service`. The `bcm-pricing-calc` invalid-operation message now lists the operations the tool actually accepts. The legacy `data` payload (including `error_code='MISSING_PARAMETER'`) and messages are preserved for backward compatibility.
4849
- Fixed `invoicing` tool `list_invoice_summaries` validation-error responses missing a structured error type. Its ten local parameter checks (selector, time-filter presence, `billing_period` format and month range, date parsing, reversed/empty range, over-one-month range) returned `format_response('error', ...)` with no top-level classification, so failures surfaced as `UNKNOWN`/`UNKNOWN` in telemetry and could not be told apart from service-side failures. Error responses now carry top-level `error_type='validation_error'`, `operation='ListInvoiceSummaries'`, and `service='Invoicing'`, matching what `handle_aws_error` already attaches to AWS-side errors in the same tool. The legacy `data` payload and messages are preserved for backward compatibility.
4950
- Fixed `compute-optimizer-automation` tool error responses missing a structured error type. Its local validation failures (missing required parameters, malformed or account-global-incompatible `regions`, malformed `filters` or unknown filter names, an unsupported `operation`, and a multi-region `next_token` that is malformed, names regions outside `regions`, or is passed to a single-region request) returned `format_response('error', ...)` with `operation` nested under `data` and no top-level classification, so failures surfaced as `UNKNOWN`/`UNKNOWN` in telemetry. They now carry top-level `error_type='validation_error'`, `operation`, and `service='Compute Optimizer Automation'`. Multi-region requests in which no region succeeds previously exposed the failure type only under the region-keyed `data.region_errors.<region>.error_type`; they now also carry a top-level `error_type` (the type every region shares, e.g. `AccessDeniedException` or `ResourceNotFoundException`, otherwise `multi_region_error`), `operation`, and `service`, with `region_errors` kept for per-region detail. The legacy `data` payload and messages are preserved for backward compatibility.
5051
- Fixed `bcm-pricing-calc`, `cost-anomaly`, and `cost-explorer` error responses missing a structured error type. `bcm-pricing-calc` read the AWS message from a non-existent `data.error` key and reported every `get_preferences` failure (including `AccessDenied`) only as `PREFERENCES_NOT_CONFIGURED`; error responses now carry top-level `error_type`, `operation`, and `service`. `cost-anomaly` local input validation (bad date format, `start_date` after `end_date`, a future `end_date`, unknown `feedback`/`total_impact_operator`, missing `total_impact_end`) now carries top-level `error_type='validation_error'`, `operation`, and `service`. `cost-explorer` client-creation failures now carry top-level `error_type='client_creation_error'`, `operation`, and `service`. The legacy `data` payload is preserved for backward compatibility.

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/aws_pricing_tools.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ async def aws_pricing(
102102
return format_response(
103103
'error',
104104
{'message': 'service_code is required for get_service_attributes operation'},
105+
error_type='validation_error',
106+
operation=operation,
107+
service=AWS_PRICING_SERVICE_NAME,
105108
)
106109
return await get_service_attributes(ctx, service_code)
107110

@@ -112,6 +115,9 @@ async def aws_pricing(
112115
{
113116
'message': 'service_code and attribute_name are required for get_attribute_values operation'
114117
},
118+
error_type='validation_error',
119+
operation=operation,
120+
service=AWS_PRICING_SERVICE_NAME,
115121
)
116122
return await get_attribute_values(
117123
ctx, service_code, attribute_name, max_results=max_results
@@ -122,6 +128,9 @@ async def aws_pricing(
122128
return format_response(
123129
'error',
124130
{'message': 'service_code is required for get_pricing_from_api operation'},
131+
error_type='validation_error',
132+
operation=operation,
133+
service=AWS_PRICING_SERVICE_NAME,
125134
)
126135
return await get_pricing_from_api(ctx, service_code, filters, max_results=max_results)
127136

@@ -131,6 +140,9 @@ async def aws_pricing(
131140
{
132141
'message': f'Unknown operation: {operation}. Supported operations: get_service_codes, get_service_attributes, get_attribute_values, get_pricing_from_api'
133142
},
143+
error_type='validation_error',
144+
operation=operation,
145+
service=AWS_PRICING_SERVICE_NAME,
134146
)
135147

136148
except Exception as e:

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/bcm_pricing_calculator_tools.py‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@
3333
DATETIME_FORMAT = '%Y-%m-%d %H:%M:%S UTC'
3434
UTC_TIMEZONE_OFFSET = '+00:00'
3535
BCM_PRICING_CALCULATOR_SERVICE_NAME = 'BCM Pricing Calculator'
36+
# The operations this tool supports, in the order presented to callers.
37+
VALID_OPERATIONS = [
38+
'get_workload_estimate',
39+
'list_workload_estimates',
40+
'list_workload_estimate_usage',
41+
'get_preferences',
42+
]
3643
PREFERENCES_NOT_CONFIGURED_ERROR = 'BCM Pricing Calculator preferences are not configured. Please configure preferences before using this service.'
3744

3845
bcm_pricing_calculator_server = FastMCP(
@@ -116,16 +123,14 @@ async def bcm_pricing_calc_core(
116123
await ctx.info(f'Received BCM Pricing Calculator operation: {operation}')
117124

118125
# Check if the operation is valid
119-
if operation not in [
120-
'get_workload_estimate',
121-
'list_workload_estimates',
122-
'list_workload_estimate_usage',
123-
'get_preferences',
124-
]:
126+
if operation not in VALID_OPERATIONS:
125127
return format_response(
126128
'error',
127129
{'invalid_parameter': 'operation'},
128-
f'Invalid operation: {operation}. Valid operations are: get_workload_estimates, get_preferences, describe_workload_estimates',
130+
f'Invalid operation: {operation}. Valid operations are: {", ".join(VALID_OPERATIONS)}',
131+
error_type='validation_error',
132+
operation=operation,
133+
service=BCM_PRICING_CALCULATOR_SERVICE_NAME,
129134
)
130135

131136
# Call the appropriate operation
@@ -172,7 +177,13 @@ async def bcm_pricing_calc_core(
172177
},
173178
)
174179
else:
175-
return format_response('error', {'message': f'Unknown operation: {operation}'})
180+
return format_response(
181+
'error',
182+
{'message': f'Unknown operation: {operation}'},
183+
error_type='validation_error',
184+
operation=operation,
185+
service=BCM_PRICING_CALCULATOR_SERVICE_NAME,
186+
)
176187

177188
except Exception as e:
178189
# Use shared error handler for consistent error handling
@@ -562,6 +573,9 @@ async def get_workload_estimate(
562573
'error': 'Identifier is required when calling get_workload_estimate',
563574
'error_code': 'MISSING_PARAMETER',
564575
},
576+
error_type='validation_error',
577+
operation='get_workload_estimate',
578+
service=BCM_PRICING_CALCULATOR_SERVICE_NAME,
565579
)
566580

567581
# Log the request
@@ -653,6 +667,9 @@ async def list_workload_estimate_usage(
653667
'error': 'workload_estimate_id is required when calling list_workload_estimate_usage',
654668
'error_code': 'MISSING_PARAMETER',
655669
},
670+
error_type='validation_error',
671+
operation='list_workload_estimate_usage',
672+
service=BCM_PRICING_CALCULATOR_SERVICE_NAME,
656673
)
657674

658675
# Log the request

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_allocation_tags_tools.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ async def list_cost_allocation_tags(
9999
{
100100
'message': f'Invalid JSON for tag_keys parameter: {e}',
101101
},
102+
error_type='validation_error',
103+
operation='ListCostAllocationTags',
104+
service='Cost Explorer',
102105
)
103106
if tag_type:
104107
params['Type'] = tag_type

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_anomaly_tools.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,12 +237,18 @@ async def cost_anomaly(
237237
f'Cost Anomaly Detection validation error for 2024 data: {error_message}. '
238238
f'Note that cost anomalies may not be available yet for very recent data. '
239239
f'Try querying a date range that ends at least 24-48 hours in the past.',
240+
error_type=error_code,
241+
operation='cost_anomaly',
242+
service='Cost Explorer',
240243
)
241244
elif error_code == 'ValidationException':
242245
return format_response(
243246
'error',
244247
{'error_code': error_code},
245248
f'Cost Anomaly Detection validation error: {error_message}',
249+
error_type=error_code,
250+
operation='cost_anomaly',
251+
service='Cost Explorer',
246252
)
247253
else:
248254
# Use shared error handler for other AWS errors

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_category_tools.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@ async def list_cost_category_definitions(
169169
{
170170
'message': f'Invalid JSON for supported_resource_types parameter: {e}',
171171
},
172+
error_type='validation_error',
173+
operation='ListCostCategoryDefinitions',
174+
service='Cost Explorer',
172175
)
173176
if max_results is not None:
174177
params['MaxResults'] = max_results

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_explorer_tools.py‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
from typing import Any, Dict, Optional
3333

3434

35+
COST_EXPLORER_SERVICE_NAME = 'Cost Explorer'
36+
3537
cost_explorer_server = FastMCP(
3638
name='cost-explorer-tools', instructions='Tools for working with AWS Cost Explorer API'
3739
)
@@ -219,7 +221,7 @@ async def cost_explorer(
219221
error_message,
220222
error_type='client_creation_error',
221223
operation=operation,
222-
service='Cost Explorer',
224+
service=COST_EXPLORER_SERVICE_NAME,
223225
)
224226

225227
# Route to the appropriate operation handler
@@ -259,7 +261,11 @@ async def cost_explorer(
259261
elif operation == 'getDimensionValues':
260262
if not dimension:
261263
return format_response(
262-
'error', {'message': 'dimension is required for getDimensionValues operation'}
264+
'error',
265+
{'message': 'dimension is required for getDimensionValues operation'},
266+
error_type='validation_error',
267+
operation=operation,
268+
service=COST_EXPLORER_SERVICE_NAME,
263269
)
264270

265271
return await get_dimension_values(
@@ -280,7 +286,11 @@ async def cost_explorer(
280286
elif operation == 'getCostForecast':
281287
if not metric:
282288
return format_response(
283-
'error', {'message': 'metric is required for getCostForecast operation'}
289+
'error',
290+
{'message': 'metric is required for getCostForecast operation'},
291+
error_type='validation_error',
292+
operation=operation,
293+
service=COST_EXPLORER_SERVICE_NAME,
284294
)
285295

286296
return await get_cost_forecast(
@@ -298,7 +308,11 @@ async def cost_explorer(
298308
elif operation == 'getUsageForecast':
299309
if not metric:
300310
return format_response(
301-
'error', {'message': 'metric is required for getUsageForecast operation'}
311+
'error',
312+
{'message': 'metric is required for getUsageForecast operation'},
313+
error_type='validation_error',
314+
operation=operation,
315+
service=COST_EXPLORER_SERVICE_NAME,
302316
)
303317

304318
return await get_usage_forecast(
@@ -348,8 +362,14 @@ async def cost_explorer(
348362
)
349363

350364
else:
351-
return format_response('error', {'message': f'Unknown operation: {operation}'})
365+
return format_response(
366+
'error',
367+
{'message': f'Unknown operation: {operation}'},
368+
error_type='validation_error',
369+
operation=operation,
370+
service=COST_EXPLORER_SERVICE_NAME,
371+
)
352372

353373
except Exception as e:
354374
# The shared handler classifies ClientError and all other exceptions
355-
return await handle_aws_error(ctx, e, operation, 'Cost Explorer')
375+
return await handle_aws_error(ctx, e, operation, COST_EXPLORER_SERVICE_NAME)

‎src/billing-cost-management-mcp-server/tests/tools/test_aws_bcm_pricing_calculator_tools.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,9 @@ async def test_get_workload_estimate_missing_identifier(self, mock_context):
452452
assert result['status'] == 'error'
453453
assert 'Identifier is required' in result['data']['error']
454454
assert result['data']['error_code'] == 'MISSING_PARAMETER'
455+
assert result['error_type'] == 'validation_error'
456+
assert result['operation'] == 'get_workload_estimate'
457+
assert result['service'] == 'BCM Pricing Calculator'
455458

456459
@patch(
457460
'awslabs.billing_cost_management_mcp_server.tools.bcm_pricing_calculator_tools.get_preferences'
@@ -534,6 +537,9 @@ async def test_list_workload_estimate_usage_missing_id(self, mock_context):
534537
assert result['status'] == 'error'
535538
assert 'workload_estimate_id is required' in result['data']['error']
536539
assert result['data']['error_code'] == 'MISSING_PARAMETER'
540+
assert result['error_type'] == 'validation_error'
541+
assert result['operation'] == 'list_workload_estimate_usage'
542+
assert result['service'] == 'BCM Pricing Calculator'
537543

538544
@patch(
539545
'awslabs.billing_cost_management_mcp_server.tools.bcm_pricing_calculator_tools.get_preferences'
@@ -1670,6 +1676,14 @@ async def test_bcm_pricing_calc_core_invalid_operation(self, mock_context):
16701676
assert result['status'] == 'error'
16711677
assert 'Invalid operation' in result['message']
16721678
assert 'invalid_parameter' in result['data']
1679+
# The message lists the operations the tool actually accepts
1680+
assert (
1681+
'get_workload_estimate, list_workload_estimates, list_workload_estimate_usage, '
1682+
'get_preferences' in result['message']
1683+
)
1684+
assert result['error_type'] == 'validation_error'
1685+
assert result['operation'] == 'invalid_operation'
1686+
assert result['service'] == 'BCM Pricing Calculator'
16731687
mock_context.info.assert_called_with(
16741688
'Received BCM Pricing Calculator operation: invalid_operation'
16751689
)

‎src/billing-cost-management-mcp-server/tests/tools/test_aws_pricing_tools.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,9 @@ async def test_ap_real_get_service_attributes_reload_identity_decorator(mock_con
735735
)
736736
assert res2['status'] == 'error'
737737
assert 'service_code is required' in res2.get('data', {}).get('message', '')
738+
assert res2['error_type'] == 'validation_error'
739+
assert res2['operation'] == 'get_service_attributes'
740+
assert res2['service'] == 'AWS Pricing'
738741

739742

740743
# ---------------------------------------------------------------------------
@@ -858,6 +861,9 @@ async def test_ap_real_get_attribute_values_reload_identity_decorator(mock_conte
858861
)
859862
assert res2['status'] == 'error'
860863
assert 'service_code and attribute_name' in res2.get('data', {}).get('message', '')
864+
assert res2['error_type'] == 'validation_error'
865+
assert res2['operation'] == 'get_attribute_values'
866+
assert res2['service'] == 'AWS Pricing'
861867

862868

863869
@pytest.mark.asyncio
@@ -885,6 +891,9 @@ async def test_ap_real_get_pricing_from_api_reload_identity_decorator(mock_conte
885891
)
886892
assert res2['status'] == 'error'
887893
assert 'service_code is required' in res2.get('data', {}).get('message', '')
894+
assert res2['error_type'] == 'validation_error'
895+
assert res2['operation'] == 'get_pricing_from_api'
896+
assert res2['service'] == 'AWS Pricing'
888897

889898

890899
@pytest.mark.asyncio
@@ -899,6 +908,9 @@ async def test_ap_real_unknown_operation_error_reload_identity_decorator(mock_co
899908
assert res['status'] == 'error'
900909
# Real format_response puts message under data
901910
assert 'Unknown operation' in res.get('data', {}).get('message', '')
911+
assert res['error_type'] == 'validation_error'
912+
assert res['operation'] == 'definitely_not_supported'
913+
assert res['service'] == 'AWS Pricing'
902914

903915

904916
@pytest.mark.asyncio

‎src/billing-cost-management-mcp-server/tests/tools/test_cost_allocation_tags_tools.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,9 @@ async def test_list_tags_invalid_tag_keys_json(self, mock_context):
287287

288288
assert result['status'] == 'error'
289289
assert 'Invalid JSON for tag_keys parameter' in result['data']['message']
290+
assert result['error_type'] == 'validation_error'
291+
assert result['operation'] == 'ListCostAllocationTags'
292+
assert result['service'] == 'Cost Explorer'
290293

291294

292295
@pytest.mark.asyncio

0 commit comments

Comments
 (0)