Add protocol support for AWS JSON-RPC by jonathan343 · Pull Request #645 · smithy-lang/smithy-python

jonathan343 · 2026-02-26T17:02:22Z

Overview

This PR adds awsJson1_0/awsJson1_1 support: new protocol generators and protocol-test projections, plus smithy-aws-core runtime client protocols with awsJson-specific error identification/discriminator parsing, host-prefix handling, and event-stream support. It also includes supporting infrastructure changes: improved generated SigV4 test config, shared HTTP endpoint host-prefix merging, and smithy-json non-finite numeric serde (NaN/Infinity) round-trip support.

Note

This PR does not implement event streaming support for JSON-RPC. Of the 149 AWS services that use this protocol, only 2 of them use event streaming. This will be implemented as a follow up along with proper event streaming protocol tests.

Testing

I was able to generate a client for Amazon SQS from its smithy service model and make successfully calls:

SQS Test Script

import asyncio

from smithy_aws_core.identity import EnvironmentCredentialsResolver
from aws_sdk_sqs.client import SQSClient, ListQueuesInput
from aws_sdk_sqs.config import Config

async def main():
    client = SQSClient(
        config=Config(
            endpoint_uri="https://sqs.us-west-2.amazonaws.com",
            region="us-west-2",
            aws_credentials_identity_resolver=EnvironmentCredentialsResolver(),
        )
    )
    response = await client.list_queues(
        ListQueuesInput()
    )
    print(response)

asyncio.run(main())

Output:

(smithy-python) $ python test.py
/Users/gytndd/dev/GitHub/smithy-python/packages/aws-sdk-signers/src/aws_sdk_signers/signers.py:794: AWSSDKWarning: Payload signing is enabled. This may result in decreased performance for large request bodies.
  warnings.warn(
Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x10b598110>
ListQueuesOutput(queue_urls=['https://sqs.us-west-2.amazonaws.com/<REDACTED>.fifo', 'https://sqs.us-west-2.amazonaws.com/<REDACTED>.fifo'], next_token=None)

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

JordonPhillips · 2026-02-27T15:23:07Z

codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java

        writer.addImport("smithy_core.aio.utils", "async_list");

        writer.write("""
+                def _assert_modeled_value(actual: object, expected: object) -> None:


I'm not a fan of this. When you do a plain assert with two dataclasses, pytest will give you a nice result showing you the diff if they're not equal. This will fail at the first non-equal value. Also, since it only recurses on dataclasses it'll never find nan in a list or map.

In the spirit of moving things out of Java, it might be best to have a test-utils package that implements an assert that builds up an error. It could go in test dependencies so normal installs don't catch it.

Yea, this is more complicated that I thought. I'm gonna back track from this here so the scope is on JSON-RPC and I don't add complexity to this PR.

I played around with this approach for replacing NaN's with a custom object that won't fail on the ==. But I'm not a fan of having to rebuild the object just for this.

class _ModeledNaN: def __repr__(self) -> str: return "float('nan')" class _NormalizedDocument: def __init__(self, value: object) -> None: self.value = value def __eq__(self, other: object) -> bool: return isinstance(other, _NormalizedDocument) and self.value == other.value def __repr__(self) -> str: return f"Document(value={self.value!r})" _MODELED_NAN = _ModeledNaN() def _normalize_modeled_value(value: Any) -> Any: if isinstance(value, float) and isnan(value): return _MODELED_NAN if isinstance(value, Document): return _NormalizedDocument(_normalize_modeled_value(value.as_value())) if is_dataclass(value) and not isinstance(value, type): return cast(Any, type(value))( **{ field.name: _normalize_modeled_value(getattr(value, field.name)) for field in fields(value) if field.init }, ) if isinstance(value, list): return [_normalize_modeled_value(member) for member in cast(list[Any], value)] if isinstance(value, tuple): return tuple( _normalize_modeled_value(member) for member in cast(tuple[Any, ...], value) ) if isinstance(value, dict): return { key: _normalize_modeled_value(member) for key, member in cast(dict[Any, Any], value).items() } return value def _assert_modeled_value(actual: Any, expected: Any) -> None: assert _normalize_modeled_value(actual) == _normalize_modeled_value(expected)

I'm gonna revert these changes and ignore the nan tests again. I'll create an issue for us to track the work for getting a proper solution.

Yeah it's a bit complex. What if you just to a recursive equalty check if it's a known nan test? I'm okay with the rest result being less good for those tests specifically. I think it should be pretty easy to traverse the document during codegen to see if there's a nan value.

...core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsJson10ProtocolGenerator.java

...core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsJson11ProtocolGenerator.java

JordonPhillips · 2026-03-02T17:28:26Z

packages/smithy-aws-core/src/smithy_aws_core/traits.py

 from smithy_core.traits import DynamicTrait, Trait


+def _parse_http_protocol_values(


This could really use a comment explaining what's going on.

Added in 3d2f6c5

Let me know if you find it helpful. If not, I'm happy to update it

JordonPhillips · 2026-03-02T17:31:38Z

packages/smithy-aws-core/src/smithy_aws_core/utils.py

    return ShapeID.from_parts(name=code, namespace=default_namespace)
+
+
+def parse_header_error_code(code: str, default_namespace: str | None) -> ShapeID | None:


Why does this function exist? The function above should handle it everything. If it's not handling some edge case, it should be updated.

Thanks for asking about this. Turns out I was misinterpreting some failed protocol tests that turned out to be false negatives. I removed parse_header_error_code and refactored some other pieces to make things simpler.

JordonPhillips · 2026-03-02T17:45:05Z

packages/smithy-http/src/smithy_http/aio/protocols.py

+        has_host_prefix = bool(previous.host) and previous.host != "."
+        host = uri.host
+        if has_host_prefix and uri.host and previous.host.endswith("."):
+            host = f"{previous.host}{uri.host}"
+        elif has_host_prefix:
+            host = previous.host


This was another misunderstanding on my part. I was trying to compensate for something we don't support yet. I added the related protocol tests to the skip list in 05c957e. We ignore them for REST-JSON.

JordonPhillips · 2026-03-02T17:46:44Z

packages/smithy-json/src/smithy_json/_private/deserializers.py

+            case int():
+                return Decimal(event.value)
+            case float():
                return Decimal.from_float(event.value)
+            case "Infinity" | "-Infinity" | "NaN":
+                return Decimal(event.value)


If the outcome is the same, they should be combined

Addressed in e051943

jonathan343 · 2026-03-07T22:25:15Z

Reviewer Notes:

Failing Tests: The failing protocol tests are expected and will be resolved once Fix awsJson docs examples and awsJson1_1 error namespaces smithy#2990 is released in the next version of smithy. I'm hoping they do a release before this PR is merge ready, otherwise I'll add them to the ignore list temporarily and follow up with a removal.
- I added fallback behavior that attempts to find an error in the error_registry using the default namespace. Protocol tests are now passing.
Kinesis Testing: Kinesis is a JSON-RPC service that I've been testing with. I'm currently seeing issues with the CRT adapter not working for HTTP/1.1. When a HTTP/1.1 connection is negotiated, CRT expects HttpRequest.body_stream instead, doesn’t get one, and raises a smithy_core.exceptions.SmithyError: 2056 (AWS_ERROR_HTTP_MISSING_BODY_STREAM): Given the provided headers (ex: Content-Length), a body is expected. error. This is not directly related to protocol work so I can address it as a follow up.
Event Stream: When testing with generated kinesis and logs (CloudWatchLogs) clients, I'm seeing smithy_core.exceptions.SerializationError: Unions must have exactly one value, but found none. The source of this is the awsJson output event-stream path leaving the initial-response frame in the stream instead of consuming it as the operation output; the event receiver then tries to deserialize that {} payload as the modeled event union, finds no active member, and raises.

Summary

Non-event stream operations are working for service clients that use the JSON-RPC protocol (see SQS example in the description). However, there is some additional work to do to handle event streams. I have a local fixes that address both issues mentioned above which are seen when testing with kinesis and logs clients.

I'm thinking I should scope this PR to just add support for non-event stream JSON-RPC support and follow up with a proper event stream PR that includes the fixes and event stream protocol tests. Open to feedback

jonathan343 · 2026-03-11T15:09:09Z

Update:

I added a fallback in modeled error resolution for cases where the wire error ShapeID uses a different namespace than the modeled error registered on the operation. We still prefer an exact error_registry match first, but on a miss we now retry with the operation's default namespace and the same shape name.

Protocol tests are now passing and we don't need to wait for smithy-lang/smithy#2990

JordonPhillips · 2026-03-13T13:45:03Z

...core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsJson10ProtocolGenerator.java

+            // TODO: support client error-correction behavior when the server
+            // omits required values in modeled error responses.
+            "AwsJson10ClientErrorCorrectsWhenServerFailsToSerializeRequiredValues",
+            "AwsJson10ClientErrorCorrectsWithDefaultValuesWhenServerFailsToSerializeRequiredValues");


Didn't you say you fixed this?

No, maybe you're thinking of the error namespace mismatch fallback behavior I added?

JordonPhillips · 2026-03-13T14:36:13Z

packages/smithy-http/src/smithy_http/aio/protocols.py


        return request

+    def _resolve_error_id(


This isn't the right place for this as it's a quirk of how AWS SDKs have historically been generated. There is already a method that handles such quirks, and before you'd created a new one right next to it. Why not just update that?

The reason I initially put _resolve_error_id on HttpClientProtocol was for reuse. REST-JSON gets its error handling from HttpBindingClientProtocol._create_error, while awsJson/JSON-RPC has its own _create_error path because it doesn’t use the HTTP binding client protocol. Both paths needed the same modeled-error ID fallback logic, and the nearest common base between them was HttpClientProtocol, so I factored the shared resolution step there rather than duplicating it in both implementations.

Any suggestions for a better alternative?

I agree with Jordon; the logic doesn't feel like it fits here. We could potentially add a new AWSClientProtocol base class that inherits from this, we could add a mixin, or a utility function.

Since all protocols will need this, I think the shared base class makes the most sense

JordonPhillips · 2026-03-13T14:40:23Z

packages/smithy-json/src/smithy_json/_private/deserializers.py

                return event.value
            case int() | float():
                return Decimal.from_float(event.value)
+            case "Infinity" | "-Infinity" | "NaN":


github is being weird and won't let me do a code suggestion, but this also needs to be moved up the same way as was done for float

I'm trying to figure out what you're suggesting and the only thing that makes sense is to move off of Decimal.from_float(event.value) and do Decimal(event.value) like below:

case int() | float() | "Infinity" | "-Infinity" | "NaN": return Decimal(event.value)

Let me know if I misunderstood.

JordonPhillips · 2026-03-13T14:42:41Z

packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py

+    ) -> bool:
+        return 200 <= response.status < 300
+
+    def _resolve_host_prefix(


You either gotta be running the tests for this or you gotta not have an implementation

Removed. I'll defer to when we have endpoints actually working so the integration is clean.

JordonPhillips · 2026-03-13T14:44:20Z

packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py

+            return content_length == 0
+        return False
+
+    async def _create_error(


You should mention what's different here vs the default

I updated the comment to compare against HttpBindingClientProtocol._create_error, which is the closest existing implementation. awsJson doesn’t inherit that code path directly because it doesn’t use per-operation HTTP bindings, so the comment now calls out the actual differences. It keeps the same error ID resolution logic, but deserializes modeled errors directly from the JSON body and normalizes empty bodies to {} for structured errors.

SamRemis · 2026-03-27T23:34:19Z

packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py

+            if isinstance(response_body, bytearray):
+                response_body = bytes(response_body)
+            deserializer = self.payload_codec.create_deserializer(source=response_body)
+            document = deserializer.read_document(schema=DOCUMENT)


[Genuine question]

What happens if they sent us a content type but invalid json, for instance in the form of an incomplete payload? Is this going to safely pass and fall back to the next step, or will it raise a clear error?

Can we add a test for that scenario?

SamRemis · 2026-03-27T23:40:37Z

packages/smithy-http/src/smithy_http/aio/protocols.py


        return request

+    def _resolve_error_id(


I agree with Jordon; the logic doesn't feel like it fits here. We could potentially add a new AWSClientProtocol base class that inherits from this, we could add a mixin, or a utility function.

Since all protocols will need this, I think the shared base class makes the most sense

SamRemis · 2026-03-27T23:48:19Z

packages/smithy-aws-core/tests/unit/aio/test_protocols.py

+    assert await request.consume_body_async() == b"{}"
+
+
+@pytest.mark.asyncio


I got these tests to pass locally without this annotation, likely due to

smithy-python/pyproject.toml

Line 48 in 2e3bb33

asyncio_mode = "auto" # makes pytest run async tests without having to be marked with the @pytest.mark.asyncio decorator

SamRemis · 2026-03-28T00:05:35Z

packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py

+        # but awsJson operations do not use per-operation HTTP bindings. Once
+        # the error shape is resolved, deserialize it directly from JSON and
+        # normalize empty bodies to {} for structured errors.
+        error_id = self.error_identifier.identify(


[Non-blocking suggestion]

Bit of an odd logical flow here:

(line 416) error identifier is an instance of AWSErrorIdentifier and only looks in the header

(lines 420-434) If the first identifier fails, then we parse the body as JSON and try to grab it from the expected locations there; if it's found, we set error_id to that

(line 436-441) If we found an error id, we try to resolve it again; if we already entered the block from 428, we will re-resolve it

Can we update this code path to remove that double resolve? So it would first try to identify the header, then the body discriminator if it wasn't in the header; in another block later, we would call _resolve_error_id instead of twice.

[Non-blocking-suggestion 2]

The variable names and abstraction levels here make it really hard to follow what line is doing what. For instance, error_id gets reused across different meanings (the raw wire value, then the resolved registry key), and resolved_error_id vs error_id makes it unclear which is the "final" value at any given point.

Also, how are we supposed to figure out that error_identifier is for the HTTPErrorIdentifier for generic identifiers and the rest of this logic is more AWS specific? Why doesn't the variable named error_identifier handle all of the error identification?

Can we add some comments or rename some of these variables for readability?

SamRemis · 2026-03-28T00:12:55Z

packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py



-class RestJsonClientProtocol(HttpBindingClientProtocol):
+class _EventStreamClientProtocolMixin:


[Genuine question]

What happens when you try to generate an AwsJson client with an event stream? Does codegen fail? Or does it pass and then fail when you try to call the operation with the NotImplementedError?

SamRemis · 2026-03-28T00:17:05Z

packages/smithy-aws-core/tests/unit/aio/test_protocols.py

+            [("content-type", "application/x-amz-json-1.1; charset=utf-8")]
+        ),
+    )
+    assert getattr(protocol, "_matches_content_type")(response)


Should we assert the negative here as well? Something doesn't match the expected content type?

SamRemis · 2026-03-28T00:18:35Z

packages/smithy-aws-core/tests/unit/aio/test_protocols.py

+        body=b"",
+    )
+
+    error = await getattr(protocol, "_create_error")(


We are doing a lot of these tests where we are testing the private _create_error attribute with calling getattr; can we avoid that and just pass in an error response and make sure it gets handled properly?

SamRemis · 2026-03-28T00:22:49Z

packages/smithy-aws-core/tests/unit/aio/test_protocols.py

+
+
+@pytest.mark.asyncio
+async def test_rest_json_resolves_modeled_error_from_header_default_namespace_fallback() -> (


Isn't this testing the same code path as test_aws_json11_resolves_modeled_error_from_header_default_namespace_fallback? And aren't all four of these tests testing logic that's in smithy-http, not smithy-aws-core?

jonathan343 added 6 commits February 25, 2026 11:45

Add AWS JSON 1.0 and 1.1 traits

5c1d94b

smithy-json: support quoted non-finite numeric serde

d6704da

codegen: improve generated protocol test assertions

2eb2023

smithy-http: preserve host prefixes when applying endpoints

ad43c77

smithy-aws-core: add awsJson client protocol support

0243826

codegen: add awsJson protocol generators and tests

5a4659c

jonathan343 changed the title ~~Json rpc v1~~ Add protocol support for AWS JSON-RPC Feb 26, 2026

JordonPhillips reviewed Feb 27, 2026

View reviewed changes

JordonPhillips requested changes Mar 2, 2026

View reviewed changes

jonathan343 added 6 commits March 3, 2026 11:13

codegen: use protocol-specific protocol test module names

7d332fb

Add docstring to _parse_http_protocol_values for clarity

3d2f6c5

smithy-json: combine equivalent number deserializer branches

e051943

smithy-aws-core: simplify awsJson error identification

3512866

smithy-http: stop preserving host prefixes on endpoints

76ac447

codegen: skip awsJson endpoint trait protocol tests

05c957e

Handle modeled error namespace mismatches during lookup

4b07405

jonathan343 added 2 commits March 12, 2026 00:43

Skip NaN protocol compliance cases in generated tests

306961c

remove eventstream mix in until we get proper support

7614b45

jonathan343 marked this pull request as ready for review March 12, 2026 05:17

jonathan343 requested a review from a team as a code owner March 12, 2026 05:17

JordonPhillips requested changes Mar 13, 2026

View reviewed changes

arandito mentioned this pull request Mar 19, 2026

Add AWS QUERY protocol support #663

Merged

Address feedback

679c259

SamRemis reviewed Mar 28, 2026

View reviewed changes

		from smithy_core.traits import DynamicTrait, Trait


		def _parse_http_protocol_values(

		return ShapeID.from_parts(name=code, namespace=default_namespace)


		def parse_header_error_code(code: str, default_namespace: str \| None) -> ShapeID \| None:

		assert await request.consume_body_async() == b"{}"


		@pytest.mark.asyncio



		class RestJsonClientProtocol(HttpBindingClientProtocol):
		class _EventStreamClientProtocolMixin:



		@pytest.mark.asyncio
		async def test_rest_json_resolves_modeled_error_from_header_default_namespace_fallback() -> (

Conversation

jonathan343 commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Testing

SQS Test Script

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathan343 commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Notes:

Summary

Uh oh!

jonathan343 commented Mar 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SamRemis Mar 28, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathan343 commented Feb 26, 2026 •

edited

Loading

jonathan343 commented Mar 7, 2026 •

edited

Loading

SamRemis Mar 28, 2026 •

edited

Loading