[Question]: Clone HttpRequestMessage for each retry by altering the input with Chaos Behavior?

[caigen]

What are you wanting to achieve?

I am trying to Clone HttpRequestMessage for each retry then use it for next retry since some reason.
From this doc https://www.pollydocs.org/chaos/behavior.html, I saw that allowing users to define specific behaviors such as altering the input.

So, I use AddChaosBehavior as one hack way. The issue for this way is that:

1. What I used is *Chaos* but for BeforeEachRetry, it does not make sense from naming.

2. Resilience.Http.RequestMessage is internally thing. So, it is hack but quick way.
   https://github.com/dotnet/extensions/blob/060e3facaadeb34aa45be95aeaa8ba977e5c8772/src/Libraries/Microsoft.Extensions.Http.Resilience/Internal/ResilienceKeys.cs#L13

3. I can create some customized strategy to finish it but need more effort.

Question:

1. Is this one expected way to use Polly for altering the input? We also need one sample about how to altering the input in that doc.
2. Should we rename AddChaosBehavior to AddBehavior then it can conceptually support not just for chaos but also for resilience strategy?
3. Should we provide something like BeforeRetry callback for Retry strategy?
4. Any short/quick solution without much customized code to Clone HttpRequestMessage ?

What code or approach do you have so far?

I am using this pattern

AddRetry(),
var optionsWithBehaviorGenerator = new ChaosBehaviorStrategyOptions
{
    Enabled = useClone,
    InjectionRate = 1.0,
    BehaviorGenerator = static async args =>
    {
        if (args.Context.Properties.TryGetValue(
            new ResiliencePropertyKey<HttpRequestMessage>("Resilience.Http.RequestMessage"), out HttpRequestMessage? request)
            && request != null)
        {
            args.Context.Properties.Set<HttpRequestMessage>(
                new ResiliencePropertyKey<HttpRequestMessage>("Resilience.Http.RequestMessage"),
                await request.Clone());
        }
    },
};
pipelineBuilder.AddChaosBehavior(optionsWithBehaviorGenerator);
)

Additional context

No response

[martincostello]

Should we rename AddChaosBehavior to AddBehavior then it can conceptually support not just for chaos but also for resilience strategy?

This would be a breaking change, so no we wouldn't do this.

I'll get back to you on the other questions.

[martincostello]

Resilience.Http.RequestMessage is internally thing

But note that it's in entirely different repository. It's an internal implementation detail of something that builds on top of Polly, not Polly itself.

Is this one expected way to use Polly for altering the input? We also need one sample about how to altering the input in that doc.

Yes and no. "Altering the input" isn't specifically what it's for, it's a generic "do something here" hook. You can just happen to maybe use it to alter the input if you factor your code in a way that allows it. This is probably why we don't have a specific example of it either, because how you'd do it would vary from use-case to use-case. For example if the input was a value type (e.g. int) there's no easy way to just replace the value as there's no reference to mutate.

As chaos behaviours are intended to see how your code responds to unexpected behaviours at the call site, not force those behaviours into the call site, I don't think that's something we'd make a first-class feature either.

Say your code called something with the value 1 but you wanted to mutate it to 0 to force it to fail, that wouldn't be what we'd expect you to do with chaos testing. In this example passing 0 would be a bug in the calling code because it never works, so that's not chaos - chaos would be using an outcome chaos strategy to have the callee fail to handle the valid value of 1 to test how the caller handles it.

Should we rename AddChaosBehavior to AddBehavior then it can conceptually support not just for chaos but also for resilience strategy?

We wouldn't rename it because:

1. It would be a breaking change;
2. Renaming it doesn't make it apply to anything that isn't a chaos strategy (like a retry). What you're actually asking for is a way to mutate the value in a retry strategy. I'm fairly sure you could use OnRetry to achieve the same effect with a retry policy with the existing code.

Should we provide something like BeforeRetry callback for Retry strategy?

We have that already - it's OnRetry: "If provided then it will be invoked before the strategy delays the next attempt."

Any short/quick solution without much customized code to Clone HttpRequestMessage ?

Not to hand, and that's a concern external to Polly anyway. Polly has no dependency on anything specific to HTTP.

[caigen]

@martincostello

OnRetry will be invoked before the strategy delays the next attempt.. It is not before the very 1st request. It is before the next attempt.
For cloning request, the asking here is to clone before the very 1st request before it is changed/disposed by something else.

[martincostello]

To take a step back, why not just clone it before passing it to the strategy? Why do you need to pass an object in to Polly, and then have Polly then have to know you want to clone it?

I am trying to Clone HttpRequestMessage for each retry then use it for next retry since some reason.

It would be helpful to have a specific example of what you're actually trying to achieve here that's the motivation for this request as to why specifically you need to clone the message. If there's zero retries, then the clone would just be wasteful?

[caigen]

If there's zero retries, then the clone would just be wasteful?
-> Yes. But before HttpResponseMessage is returned, we don't know if retry is needed or not. After HttpRequestMessge is sent out and response is returned, the sent out HttpRequestMessage should not be used again.

See:
https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httprequestmessage?view=net-9.0
An [HttpRequestMessage](https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httprequestmessage?view=net-9.0) instance should not be modified and/or reused after being sent.

[martincostello]

Interesting that ASP.NET Core's Polly implementation doesn't clone the request message at any point and I'm not aware of tha having issues related to the re-use: https://github.com/dotnet/aspnetcore/tree/main/src/HttpClientFactory/Polly/src

Do you have any thoughts on this @martintmk?

[martintmk]

Regarding the cloning, for retries this is not required as we won't have concurrent requests anyway. The only downside is that any headers added to request in the strategies bellow retries will be added for retry attempts too. One needs to be aware of such downside.

For hedging, the situation is quite different as there might be multiple concurrent requests being send. In this scenario, the request must be indeed cloned to prevent race conditions. This is where the cloning happens in dotnet/extensions repo:

https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.Http.Resilience/Internal/RequestMessageSnapshot.cs

Be aware that only request messages that do not stream content can be currently cloned.

[caigen]

@martintmk I got the difference between retry & hedging you shared.

But the key point is how can we use HttpRequestMessage.

• For .net extension itself, it may/can break the HttpRequestMessage doc since it is internal thing.
• But from logic app view which built on .net & polly, per the HttpRequestMessage doc, we cannot reuse/modify the HttpRequestMessage after it is being sent.

Or should we, I mean .net team, update that HttpRequestMessage doc to say HttpRequestMessage can be reused or modified for some special cases, e.g, retry, and will not cause side effect?

Similar issue link:
dotnet/extensions#4965

[martintmk]

Or should we, I mean .net team, update that HttpRequestMessage doc to say HttpRequestMessage can be reused or modified for some special cases, e.g, retry, and will not cause side effect?

This is new to me. I was not aware of this condition. Is this just a recommendation or is there something in the HTTP stack that prevents re-sending the request again? Currently, re-sending works just fine.