Skip to content

fix(billing-cost-management-mcp-server): complete the error envelope and fix retry classification gaps - #4781

Merged
finklen-amazon merged 5 commits into
awslabs:mainfrom
finklen-amazon:bcm-error-message-envelope
Oct 15, 2026
Merged

finklen-amazon merged 5 commits into
awslabs:mainfrom
finklen-amazon:bcm-error-message-envelope

Conversation

@finklen-amazon

@finklen-amazon finklen-amazon commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Fixes

Summary

Changes

The README's "Error responses" section documents a top-level message on every tool error, but 41 format_response('error', ...) sites across 16 tool files put their text only in data and pass no message. Clients reading the documented field get nothing for these errors:

  • aws-pricing, bcm-pricing-calc, cost-allocation-tags, cost-category, cost-explorer, credits, enterprise-support, invoice-units, invoicing, procurement-preferences (mostly validation, missing-parameter, and unsupported-operation paths).

Fix:

  • format_response: for status='error' with no message, a string data['message'] is also set as the top-level message. data is unchanged, so readers of data.message keep working. Success responses are untouched.
  • bcm-pricing-calc: the two missing-parameter errors that use data['error'] now pass message explicitly. Only data['message'] is lifted generically.
  • CHANGELOG entry under Fixed.

Fixing it in format_response rather than editing all 41 call sites keeps the diff small, avoids duplicating each message, and covers future sites that follow the same pattern.

Errors that defaulted to do_not_retry when a retry or a changed request would succeed. ErrorSignalingMiddleware derives retry_strategy from the top-level error_type, so an error whose AWS code is hidden, or whose type is not mapped, gets the conservative default. Fixed without renaming any existing error_type:

  • budgets, budget-actions, budget-notifications: get_aws_account_id wrapped STS errors in a plain Exception, so a throttle became unknown_exception. The original error now propagates.
  • storage-lens: the same wrapping for S3 GetObject/ListObjectsV2 and Athena StartQueryExecution errors. They are now re-raised. The Athena poll timeout raises TimeoutError (mapped as unknown_timeouterror to retry_same_request), and a failed query, a missing manifest or report file, and a query without a table reference raise ValueError (validation_error).
  • session-sql: error_type is the exception class (OperationalError, ValueError), too generic to map globally, so invalid or rejected SQL sets retry_strategy='modify_request' at that site.
  • compute-optimizer-automation: when every region fails for different reasons, the type is multi_region_error and per-region http_status stays nested, so throttling in one region plus a 5xx in another was do_not_retry. retry_strategy is now combined from each region's guidance by a new error_contract.combine_guidance (identical guidance is kept with its hint; otherwise do_not_retry > retry_same_request > modify_request).
  • compute-optimizer: the unsupported-operation and not-enrolled errors had error_type only in data; it is now top level. Its ClientError handler's throttling_error, service_unavailable, resource_not_found and opt_in_required are now mapped.
  • Mapping: added codes the Budgets, Pricing and Athena models use: NotFoundException, ExpiredNextTokenException, InvalidRequestException (modify), InternalErrorException (retry), plus enterprise-support's not_found (modify).
  • handle_aws_error: an AWS error with an empty Message falls back to the default text instead of ''.

Guardrail: tests/test_error_contract_conformance.py. Static checks over the package source so these gaps fail in CI instead of in production:

  • every literal error_type is mapped in error_contract.py, or listed in UNMAPPED_BY_DESIGN with a reason (each entry is also checked to still be unmapped)
  • no format_response('error', ...) with error_type only inside data
  • no Exception / RuntimeError raised in an except block, which hides the AWS error code

Run against the tool code before this PR, it flags the two compute-optimizer nested error_type sites and the four budget / storage-lens wrapping sites.

Out of scope here: BotoCoreError subclasses such as ParamValidationError still map to aws_connection_error (being addressed in #4766), and arguments rejected by FastMCP before the tool runs still have no envelope.

User experience

Before, an invalid billing_period to invoicing returned:

{"status": "error", "data": {"message": "billing_period must be in YYYY-MM format (e.g. \"2026-05\")."},
 "error_type": "validation_error", "operation": "ListInvoiceSummaries", "service": "Invoicing",
 "retry_strategy": "modify_request"}

After, the same body plus:

{"message": "billing_period must be in YYYY-MM format (e.g. \"2026-05\")."}

Checklist

If your change doesn't seem to apply, please leave them unchecked.

  • I have reviewed the contributing guidelines
  • I have performed a self-review of this change
  • Changes have been tested
  • Changes are documented

Tests (each fix has a test that fails without it): tests/utilities/test_aws_service_base.py covers lifting data['message'], an explicit message winning, data['error'] and non-string or non-dict data not being lifted, and success responses keeping their shape. The invoicing, credits, and bcm-pricing-calc validation tests now assert the top-level message. Reverting the fix fails the new assertions. Also checked end to end through a FastMCP client for invoicing, credits, and aws-pricing: isError is set, message is top level, and the text content matches structured_content. New tests cover the added mappings, combine_guidance, multi-region guidance, session-sql, the storage-lens timeout/failed-query paths and unwrapped re-raises, the STS re-raise, the compute-optimizer top-level error_type, and the empty-message fallback. A FastMCP client run confirms session-sql, compute-optimizer and invoicing errors carry the expected error_type, retry_strategy and top-level message. All 1915 tests pass; ruff check/format and pyright pass.

Is this a breaking change? (Y/N)

N. Existing error_type values and fields are unchanged. Error responses gain a top-level message where it was missing, and some errors get a more accurate retry_strategy. The wrapped storage-lens and budget errors now report their real AWS error_type instead of unknown_exception.

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

…ors whose text is only in data

41 format_response('error', ...) sites across 16 tool files pass their text in
data without a message argument, so the error body lacks the top-level message
documented under "Error responses" in the README. format_response now lifts a
string data['message'] to the top-level message (data is unchanged), and the two
bcm-pricing-calc sites that use data['error'] pass message explicitly.
@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.55%. Comparing base (1e5589d) to head (4aa80ab).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4781      +/-   ##
==========================================
+ Coverage   93.53%   93.55%   +0.01%     
==========================================
  Files        1068     1068              
  Lines       92320    92342      +22     
  Branches    15017    15024       +7     
==========================================
+ Hits        86352    86386      +34     
+ Misses       3573     3562      -11     
+ Partials     2395     2394       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…id-request errors that defaulted to do_not_retry

- budget_tools, storage_lens_tools: stop wrapping ClientError/BotoCoreError in a
  generic Exception, which hid the AWS code (throttling, 5xx) from
  handle_aws_error. Athena poll timeout raises TimeoutError (mapped to retry);
  failed queries, missing manifests and bad table references raise ValueError.
- sql_utils: invalid or rejected session SQL is modify_request.
- compute_optimizer_automation_tools: when every region fails, combine the
  per-region guidance (new error_contract.combine_guidance) instead of relying
  on multi_region_error, which always defaulted to do_not_retry.
- compute_optimizer_tools: unsupported-operation and enrollment errors set
  error_type at the top level; map its throttling_error, service_unavailable,
  resource_not_found and opt_in_required types.
- error_contract: map NotFoundException, ExpiredNextTokenException,
  InvalidRequestException, InternalErrorException, not_found and
  unknown_timeouterror.
- handle_aws_error: an empty AWS Message falls back to the default text.

Existing error_type values are unchanged.
@finklen-amazon finklen-amazon changed the title fix(billing-cost-management-mcp-server): set top-level message on errors whose text is only in data fix(billing-cost-management-mcp-server): complete the error envelope and fix retry classification gaps Oct 15, 2026
Comusus
Comusus previously approved these changes Oct 15, 2026
…onformance checks

Scans the package source and fails when a tool error path would silently get
retry_strategy=do_not_retry:
- an error_type literal that is not mapped in error_contract.py (or listed in
  UNMAPPED_BY_DESIGN with a reason)
- format_response('error', ...) with error_type only inside data
- Exception/RuntimeError raised in an except block, hiding the AWS error code

Against the code before this PR it flags the compute-optimizer nested error_type
sites and the budget and storage-lens wrapping fixed in the previous commits.
@finklen-amazon
finklen-amazon added this pull request to the merge queue Oct 15, 2026
Merged via the queue into awslabs:main with commit ccef831 Oct 15, 2026
142 checks passed
@finklen-amazon
finklen-amazon deleted the bcm-error-message-envelope branch October 15, 2026 18:50
@github-project-automation github-project-automation Bot moved this from To triage to Done in awslabs/mcp Project Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants