-
Notifications
You must be signed in to change notification settings - Fork 751
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
[New Feature Proposal] Add support for HttpRequestMessage objects containing StreamContent to AddStandardHedgingHandler() #5105
Labels
area-resilience
enhancement
This issue represents an ask for new feature or an enhancement to an existing one
work in progress 🚧
Comments
adamhammond
pushed a commit
to adamhammond/extensions
that referenced
this issue
Apr 16, 2024
Add support for HttpRequestMessage objects containing StreamContent to the AddStandardHedgingHandler() resilience API. This change does not update any public API contracts. It updates internal and private API contracts only. Link to issue: dotnet#5105
adamhammond
pushed a commit
to adamhammond/extensions
that referenced
this issue
Apr 16, 2024
Add support for HttpRequestMessage objects containing StreamContent to the AddStandardHedgingHandler() resilience API. This change does not update any public API contracts. It updates internal and private API contracts only. This is a small commit that is part of a larger PR. See the PR and its corresponding initial commit for the full set of changes.
adamhammond
pushed a commit
to adamhammond/extensions
that referenced
this issue
Apr 17, 2024
Add support for HttpRequestMessage objects containing StreamContent to the AddStandardHedgingHandler() resilience API. This change does not update any public API contracts. It updates internal and private API contracts only. This is a small commit to update the ConfigureAwait arg on an async
cc: @iliar-turdushev Can you please take a look at this? |
adamhammond
pushed a commit
to adamhammond/extensions
that referenced
this issue
Apr 18, 2024
Add support for HttpRequestMessage objects containing StreamContent to the AddStandardHedgingHandler() resilience API. This change does not update any public API contracts. It updates internal and private API contracts only. This is a small commit to resolve comments made on the PR
adamhammond
pushed a commit
to adamhammond/extensions
that referenced
this issue
Apr 19, 2024
Add support for HttpRequestMessage objects containing StreamContent to the AddStandardHedgingHandler() resilience API. This change does not update any public API contracts. It updates internal and private API contracts only. This is a small commit to resolve comments made on the PR.
adamhammond
pushed a commit
to adamhammond/extensions
that referenced
this issue
Apr 22, 2024
Add support for HttpRequestMessage objects containing StreamContent to the AddStandardHedgingHandler() resilience API. This change does not update any public API contracts. It updates internal and private API contracts only. This is a small commit to resolve comments made on the PR.
geeknoid
added
area-resilience
enhancement
This issue represents an ask for new feature or an enhancement to an existing one
and removed
untriaged
labels
May 31, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area-resilience
enhancement
This issue represents an ask for new feature or an enhancement to an existing one
work in progress 🚧
Background and motivation
Many clients trying to use the
AddStandardHedgingHandler()
resilience API based on top of Polly v8 have requirements that force them to sendHttpRequestMessage
objects that containStreamContent
. Today, if a client built with anIHttpClientBuilder
that was configured with a resilience handler via theAddStandardHedgingHandler()
API attempts to send anHttpRequestMessage
object that containsStreamContent
to a downstream service, then anInvalidOperationException
will be thrown. This exception is thrown by theInitialize()
method in RequestMessageSnapshot.cs:extensions/src/Libraries/Microsoft.Extensions.Http.Resilience/Internal/RequestMessageSnapshot.cs
Lines 73 to 80 in 10681a1
There is no reason to limit users by only supporting a subset of
HttpContent
types. Further, support can be added forHttpRequestMessage
objects whose content is of typeStreamContent
while still defaulting to the existing shallow copy logic forHttpRequestMessage
objects whose content is of any otherHttpContent
type. In this way, the change can be made fully backwards compatible with existing APIs and with no side effects for existing users of theAddStandardHedgingHandler()
API.Feature Proposal
Proposed Changes to RequestMessageSnapshot.cs:
Proposed Changes to ResilienceHttpClientBuilderExtensions.Hedging.cs:
Proposed Changes to RequestMessageSnapshotStrategy.cs:
The text was updated successfully, but these errors were encountered: