fix(codegen): support event streams in server SDK serde context#2175
Open
goelakash wants to merge 1 commit into
Open
fix(codegen): support event streams in server SDK serde context#2175goelakash wants to merge 1 commit into
goelakash wants to merge 1 commit into
Conversation
Server SDK generation stubbed the event stream serde context type as `& any` and never supplied an event stream marshaller, so generated SSDK code for operations with event streams did not type-check. Type the response serializer / request deserializer context with __EventStreamSerdeContext, and wire the Node eventStreamSerdeProvider into the handler's serde context base (it only needs utf8 encode/decode, which the base already provides). Adds server event stream codegen tests, which were previously absent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem: make a Smithy model with an event stream, generate a server from it, and the code you get is broken. Two reasons:
any— which tells the compiler "don't check this." So that error was there, just hidden. (The request side was typed correctly, which is why the package as a whole already failed to compile.)Related to #959.
The fix — two small changes, both in the code generator. No npm packages touched:
__EventStreamSerdeContext, instead ofany. Now the compiler checks it for real. (The request side already used the right type.)eventStreamSerdeProvider. All it needs is the utf8 helpers the server already had, so nothing new at runtime.Who this affects: only servers generated from models that have event streams — and their code was already broken, so this is strictly an improvement. Everyone else gets byte-for-byte the same output as before.
smithy-aws-typescript-codegenkeeps compiling unchanged; restJson1 will need its own follow-up PR in aws-sdk-js-v3.How it was tested: added the first-ever server event stream codegen tests, the full gradle build passes, and a sample of the new generated output compiles clean under
tsc --strict. The old output fails that same check witheventStreamMarshaller is missing— exactly the error theanywas hiding.Not fixed here: the rpcv2Cbor-based test in this diff only exercises the handler wiring — that protocol's own server codegen is still stubbed. Stream validation is still skipped (events arrive after up-front validation runs, so supporting it needs a design decision). And there's no live end-to-end test yet — that needs a concrete protocol, which lives in aws-sdk-js-v3.
Why I'm doing this / where it's heading: I want to build Smithy services that stream to clients over Server-Sent Events (
text/event-stream), which smithy-typescript has no story for today (smithy-java gained SSE handling for MCP; TypeScript has nothing). The marshaller is injected through the handler's serde context on purpose: all framing runs through that one object, so a protocol generator that wires in an SSE marshaller instead getstext/event-streamframing with the rest of the machinery unchanged. (To be precise: the plug-in point is the protocol-generator layer at codegen time, not a runtime hook in the emitted server.) I'm planning to prototype that as a third-party protocol generator and bring it back as an RFC — this PR is the first step, but it also stands on its own as a bugfix for #959.