Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client response streaming minimum speed limit/timeout #1562

Closed
Tracked by #724
jdisanti opened this issue Jul 21, 2022 · 1 comment
Closed
Tracked by #724

Client response streaming minimum speed limit/timeout #1562

jdisanti opened this issue Jul 21, 2022 · 1 comment

Comments

@jdisanti
Copy link
Collaborator

Clients should have a way of timing out if the response stream started but then slowed significantly or stopped entirely without closing the connection part way through. For example, if a call to S3 GetObject was made, and some bytes of the object were received in the streaming response body, but the server stopped responding with more for a significant period of time, that should result in a timeout.

Internal tracking: P66130286

@jdisanti
Copy link
Collaborator Author

@Velfi wrote a prototype for this in #1627

github-merge-queue bot pushed a commit that referenced this issue Oct 26, 2023
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
#1562

## Description
<!--- Describe your changes in detail -->
This change adds a new body wrapper: The minimum throughput limit
wrapper. It tracks the rate that data is being streamed from itself. If
that rate falls below some configurable limit, it emits an error instead
of the next chunk. This protects users from requests that start quickly
but then slow down considerably.

I'd like to get this merged and then figure out the
codegen/docs/examples/config part in a separate PR.

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
Tests are included.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
github-merge-queue bot pushed a commit that referenced this issue Nov 17, 2023
[See the upgrade guide for this feature to learn
more.](awslabs/aws-sdk-rust#956)

The breaking change is to the `MinimumThroughputBody::new` method.

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
#1562

## Description
<!--- Describe your changes in detail -->
awslabs/aws-sdk-rust#956

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
I added several tests

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

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

---------

Co-authored-by: John DiSanti <[email protected]>
Co-authored-by: Russell Cohen <[email protected]>
@goabv goabv assigned jdisanti and unassigned Velfi Mar 4, 2024
@jdisanti jdisanti removed this from the SDK GA milestone Apr 4, 2024
@jdisanti jdisanti closed this as completed Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants