Skip to content

Conversation

@ysaito1001
Copy link
Contributor

@ysaito1001 ysaito1001 commented Dec 1, 2025

Motivation and Context

#4421

Description

This PR fixes setting the request headers in question for event stream in RPC v2 CBOR according to the spec, specifically

  • Event streaming requests: the value of the Content-Type header MUST be application/vnd.amazon.eventstream.
  • For requests with event streaming responses: the value of the Accept header MUST be application/vnd.amazon.eventstream.

Testing

  • Updated ClientEventStreamMarshallerGeneratorTest.kt to include verification for those headers

The spec regarding response header For requests with all other response types: the value of the Accept header MUST be application/cbor has already been covered by protocol tests.

Checklist

  • For changes to the smithy-rs codegen or runtime crates, I have created a changelog entry Markdown file in the .changelog directory, specifying "client," "server," or both in the applies_to key.
  • For changes to the AWS SDK, generated SDK code, or SDK runtime crates, I have created a changelog entry Markdown file in the .changelog directory, specifying "aws-sdk-rust" in the applies_to key.

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

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

A new generated diff is ready to view.

A new doc preview is ready to view.

@ysaito1001 ysaito1001 marked this pull request as ready for review December 1, 2025 23:04
@ysaito1001 ysaito1001 requested review from a team as code owners December 1, 2025 23:04
Copy link
Contributor

@landonxjames landonxjames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment on some of the tests, not a blocker

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

A new generated diff is ready to view.

A new doc preview is ready to view.

@rcoh
Copy link
Collaborator

rcoh commented Dec 2, 2025

We need to consider backwards compatibility here because there are servers that are relying on the old behavior. I fixed servers in the last release, but I'm a little bit worried about any in the wild servers that have not updated their list. Can the client send both accept headers?

@ysaito1001 ysaito1001 requested a review from rcoh December 2, 2025 19:55
@github-actions
Copy link

github-actions bot commented Dec 2, 2025

A new generated diff is ready to view.

A new doc preview is ready to view.

@ysaito1001 ysaito1001 enabled auto-merge (squash) December 4, 2025 21:59
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • Client Test (ignoring whitespace)
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@ysaito1001 ysaito1001 merged commit 3786d3c into main Dec 4, 2025
92 of 94 checks passed
@ysaito1001 ysaito1001 deleted the ysaito/fix-request-headers-for-rpc-v2-event-stream branch December 4, 2025 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants