ASF: fix exception serialization for `smithy-rpc-v2-cbor` and `json` protocol #13180

bentsku · 2025-09-22T17:11:53Z

Motivation

This PR is part of a bigger initiative (see #13028) to implement support for the new Smithy Protocol smithy-rpc-v2-cbor, as well as the new Botocore multi-protocol support for single services (before, a service could only define one protocol)

This PR fixes the ASF / CloudWatch issues encountered during validating the full test suite (see #13173). Those were cherry picked to make the PR more digestible and understand which tests needed the fix.

A failure popped up when the exception message would be empty for the json and smithy-rpc-v2-cbor: the message field would not be serialized at all when the value would be empty. I validated that it's actually the behavior wanted as I knew the KMS serviced the same thing, and realized it had snapshot skips so fixed it for everything.

I've also found a major issue: the CBOR serializer absolutely needs a Botocore Shape in order to serialize any structure, but our custom-defined exception subclassed from CommonServiceException won't have any. I created a workaround by defining a fallback shape in case it is not defined, with the default field the serializer needs to properly encode the error.

This is also used by the json serializer, as it was doing some workaround by manually setting some keys in the dictionary.

I've also realized that the CloudWatch service will return different status code for exceptions, depending on the protocol and the x-amzn-query-mode request header, so this behavior has been implemented.

There is also a small fix for the boolean parsing in CBOR, showed by the test_set_alarm test.

Changes

validate 2 more tests that put into light bigger issues
create a QueryCompatibleProtocolMixin to add the utilities shared by json and smithy-rpc-v2-cbor to clean up
- add logic for overriding response status code for Query Compatible services
rework the __type hack to use shallow copy instead of deep copy of shape members
centralize exception handling of CBOR serializers:
- fix serialization of user-defined exceptions that are not present in the Botocore specs
- add new empty field logic for json and smithy-rpc-v2-cbor serializer for exceptions
update logic for parsing boolean in CBOR parser

github-actions · 2025-09-22T17:25:26Z

S3 Image Test Results (AMD64 / ARM64)

2 files ±0 2 suites ±0 8m 13s ⏱️ -32s
532 tests ±0 480 ✅ ±0 52 💤 ±0 0 ❌ ±0
1 064 runs ±0 960 ✅ ±0 104 💤 ±0 0 ❌ ±0

Results for commit d4642d1. ± Comparison against base commit 4be950c.

♻️ This comment has been updated with latest results.

github-actions · 2025-09-22T17:37:33Z

Test Results - Preflight, Unit

22 259 tests +1 20 518 ✅ +1 15m 58s ⏱️ -42s
1 suites ±0 1 741 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit d4642d1. ± Comparison against base commit 4be950c.

♻️ This comment has been updated with latest results.

github-actions · 2025-09-22T18:59:05Z

LocalStack Community integration with Pro

2 files ±0 2 suites ±0 1h 58m 29s ⏱️ +23s
4 681 tests +7 4 359 ✅ +7 322 💤 ±0 0 ❌ ±0
4 683 runs +7 4 359 ✅ +7 324 💤 ±0 0 ❌ ±0

Results for commit d4642d1. ± Comparison against base commit 4be950c.

This pull request removes 2 and adds 9 tests. Note that renamed tests count towards both.

tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input

tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[smithy-rpc-v2-cbor]

♻️ This comment has been updated with latest results.

tests/aws/services/kms/test_kms.snapshot.json

localstack-core/localstack/aws/protocol/serializer.py

github-actions · 2025-09-22T21:51:08Z

Test Results (amd64) - Acceptance

7 tests ±0 5 ✅ ±0 3m 30s ⏱️ +10s
1 suites ±0 2 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit d4642d1. ± Comparison against base commit 4be950c.

♻️ This comment has been updated with latest results.

github-actions · 2025-09-22T21:52:28Z

Test Results - Alternative Providers

459 tests - 781 287 ✅ - 414 7m 37s ⏱️ - 33m 10s
2 suites - 3 172 💤 - 367
2 files - 3 0 ❌ ± 0

Results for commit d4642d1. ± Comparison against base commit 4be950c.

This pull request removes 790 and adds 9 tests. Note that renamed tests count towards both.

tests.aws.services.cloudformation.api.test_changesets ‑ test_autoexpand_capability_requirement
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_and_then_remove_non_supported_resource_change_set
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_and_then_remove_supported_resource_change_set
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_and_then_update_refreshes_template_metadata
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_change_set_create_existing
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_change_set_invalid_params
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_change_set_missing_stackname
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_change_set_no_changes
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_change_set_update_nonexisting
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_change_set_update_without_parameters
…

tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[smithy-rpc-v2-cbor]

This pull request removes 375 skipped tests and adds 8 skipped tests. Note that renamed tests count towards both.

tests.aws.services.cloudformation.api.test_changesets ‑ test_create_and_then_update_refreshes_template_metadata
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_change_set_no_changes
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_change_set_update_without_parameters
tests.aws.services.cloudformation.api.test_changesets ‑ test_create_while_in_review
tests.aws.services.cloudformation.api.test_changesets ‑ test_describe_changeset_after_delete
tests.aws.services.cloudformation.api.test_changesets ‑ test_execute_change_set
tests.aws.services.cloudformation.api.test_changesets ‑ test_update_change_set_with_aws_novalue_repro
tests.aws.services.cloudformation.api.test_changesets ‑ test_using_pseudoparameters_in_places[condition]
tests.aws.services.cloudformation.api.test_changesets ‑ test_using_pseudoparameters_in_places[parameter]
tests.aws.services.cloudformation.api.test_changesets ‑ test_using_pseudoparameters_in_places[resource]
…

tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[smithy-rpc-v2-cbor]

♻️ This comment has been updated with latest results.

github-actions · 2025-09-22T22:35:36Z

Test Results (amd64) - Integration, Bootstrap

5 files ±0 5 suites ±0 2h 34m 51s ⏱️ - 1m 14s
5 055 tests +7 4 573 ✅ +7 482 💤 ±0 0 ❌ ±0
5 061 runs +7 4 573 ✅ +7 488 💤 ±0 0 ❌ ±0

Results for commit d4642d1. ± Comparison against base commit 4be950c.

This pull request removes 2 and adds 9 tests. Note that renamed tests count towards both.

tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input

tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_set_alarm_invalid_input[smithy-rpc-v2-cbor]

♻️ This comment has been updated with latest results.

alexrashed

Wow, that's quite a deep dive! 🤿 🤩
With the changes here and the strong similarities between the CBOR and JSON serializers, I am really wondering if we are missing an abstraction level which could simplify the two serializers and their weird special behavior in combination with the query protocol and the shape handling. Specifically also when looking at the changes in the _serialize_error method in the two serializers. What do you think?

localstack-core/localstack/aws/protocol/serializer.py

…l Mixin, rework the `__type` hack to do a shallow copy

alexrashed

Awesome! Thanks for addressing the comments! The default response serializer are minimal now, and the weird query handling is nicely encapsulated in the mixin! 💯

alexrashed · 2025-09-23T10:10:18Z

localstack-core/localstack/aws/protocol/serializer.py

+        value: dict,
+        shape: Shape | None,
+        name: str | None = None,
+        shape_members: dict[str, Shape] | None = None,


nit: It think it would be good to mention why the signature of this _serialize_type_structure differs from all the others (as it's also used for the specific error serialization).

bentsku self-assigned this Sep 22, 2025

bentsku added aws:cloudwatch Amazon CloudWatch area: asf semver: patch Non-breaking changes which can be included in patch releases docs: skip Pull request does not require documentation changes labels Sep 22, 2025

bentsku added this to the 4.9 milestone Sep 22, 2025

bentsku force-pushed the cloudwatch-multi-protocol-runs-fixes branch 3 times, most recently from be37fe7 to 778c673 Compare September 22, 2025 17:18

bentsku mentioned this pull request Sep 22, 2025

ASF: validate full CloudWatch suite with multi-protocol #13173

Merged

bentsku force-pushed the cloudwatch-multi-protocol-runs-fixes branch from 778c673 to bb7ab45 Compare September 22, 2025 17:39

bentsku added 6 commits September 22, 2025 21:16

fix boolean parsing for RPC v2

1d0684d

support x-amzn-query-mode in request and status code override

f1da5af

add way to serialize user-defined exception for CBOR

9afa8e1

fix error serialization

8bdc6d6

fix empty message in json protocol

8857831

fix query override

cc64c6a

bentsku force-pushed the cloudwatch-multi-protocol-runs-fixes branch from bb7ab45 to cc64c6a Compare September 22, 2025 19:26

bentsku added 4 commits September 22, 2025 21:32

remove early removal

bf992e7

fix undefined Error field in JSON

c1f033e

fix RPC v2 sender fault

2e1e6a5

fix empty message fallback

3503d08

bentsku commented Sep 22, 2025

View reviewed changes

tests/aws/services/kms/test_kms.snapshot.json Outdated Show resolved Hide resolved

localstack-core/localstack/aws/protocol/serializer.py Show resolved Hide resolved

localstack-core/localstack/aws/protocol/serializer.py Outdated Show resolved Hide resolved

fix headers type in serializer

dc64432

bentsku added 2 commits September 23, 2025 01:18

update kms snapshot

0bb8cda

skip v1

8071bb6

bentsku marked this pull request as ready for review September 23, 2025 08:04

bentsku requested review from alexrashed, baermat, gregfurman, maxhoheiser, pinzon, sannya-singal, steffyP and thrau as code owners September 23, 2025 08:04

bentsku assigned alexrashed Sep 23, 2025

bentsku requested review from pinzon and removed request for baermat, gregfurman, maxhoheiser, pinzon, sannya-singal and steffyP September 23, 2025 08:04

alexrashed reviewed Sep 23, 2025

View reviewed changes

localstack-core/localstack/aws/protocol/serializer.py Outdated Show resolved Hide resolved

localstack-core/localstack/aws/protocol/serializer.py Show resolved Hide resolved

localstack-core/localstack/aws/protocol/serializer.py Outdated Show resolved Hide resolved

address PR comments: rework inheritance with a QueryCompatibleProtoco…

d4642d1

…l Mixin, rework the `__type` hack to do a shallow copy

bentsku requested a review from alexrashed September 23, 2025 10:03

alexrashed approved these changes Sep 23, 2025

View reviewed changes

add comment

0832212

bentsku merged commit 4d7c543 into main Sep 23, 2025
15 checks passed

bentsku deleted the cloudwatch-multi-protocol-runs-fixes branch September 23, 2025 11:41

bentsku mentioned this pull request Sep 23, 2025

ASF: fix empty exception member serialization when required #13186

Merged

alexrashed added the notes: skip Pull request does not have to be mentioned in the release notes label Sep 24, 2025

Uh oh!

ASF: fix exception serialization for smithy-rpc-v2-cbor and json protocol #13180

ASF: fix exception serialization for smithy-rpc-v2-cbor and json protocol #13180

Uh oh!

Conversation

bentsku commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Uh oh!

github-actions bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

S3 Image Test Results (AMD64 / ARM64)

Uh oh!

github-actions bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results - Preflight, Unit

Uh oh!

github-actions bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LocalStack Community integration with Pro

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (amd64) - Acceptance

Uh oh!

github-actions bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results - Alternative Providers

Uh oh!

github-actions bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (amd64) - Integration, Bootstrap

Uh oh!

alexrashed left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexrashed left a comment

Choose a reason for hiding this comment

Uh oh!

alexrashed Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ASF: fix exception serialization for `smithy-rpc-v2-cbor` and `json` protocol #13180

ASF: fix exception serialization for `smithy-rpc-v2-cbor` and `json` protocol #13180

bentsku commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading

alexrashed left a comment •

edited

Loading