Migrate Elsa.Api.Client to Microsoft.Extensions.Http.Resilience - #7907
Migrate Elsa.Api.Client to Microsoft.Extensions.Http.Resilience#7907sfmskywalker with Copilot wants to merge 3 commits into
Conversation
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| RequestCount++; | ||
|
|
||
| if (RequestCount <= failuresBeforeSuccess) | ||
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)); |
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) | ||
| { | ||
| Content = new StringContent(response, Encoding.UTF8, "application/json") | ||
| }); |
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
Greptile SummaryThis change moves the Elsa API client to the Microsoft resilience handler and adds retry coverage. A throttled task-completion request is now sent four times instead of once because HTTP 429 is included in the default retry behavior. Restrict the retry predicate before merging so callers receive rate-limit responses without repeated mutations. Confidence Score: 4/5Not safe to merge until the retry handler stops automatically repeating rate-limited mutating requests. An in-process client run reproduced repeated POST dispatches for HTTP 429 and confirmed that the prior implementation dispatched the same request once. Files Needing Attention: src/clients/Elsa.Api.Client/Options/ElsaClientBuilderOptions.cs
 T-Rex LogsWhat T-Rex did
Prompt To Fix All With AI### Issue 1
src/clients/Elsa.Api.Client/Options/ElsaClientBuilderOptions.cs:43-50
**HTTP 429 retries repeat task mutations**
`HttpRetryStrategyOptions` uses its default retry predicate because `ShouldHandle` is not configured. That predicate retries HTTP 429 responses, so a throttled non-idempotent API call is issued four times: the original request plus the three configured retries. The previous `AddTransientHttpErrorPolicy` behavior sent the identical 429 response once. Restrict the retry predicate to the intended transient failures so rate limiting is returned to the caller instead of repeating POST and DELETE operations.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "Migrate Elsa.Api.Client resilience handl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| public Action<IHttpClientBuilder>? ConfigureRetryPolicy { get; set; } = builder => builder.AddResilienceHandler("elsa-api-client-retry", pipeline => pipeline.AddRetry(new HttpRetryStrategyOptions | ||
| { | ||
| MaxRetryAttempts = 3, | ||
| Delay = TimeSpan.FromSeconds(2), | ||
| BackoffType = DelayBackoffType.Exponential, | ||
| UseJitter = false, | ||
| ShouldRetryAfterHeader = false | ||
| })); |
There was a problem hiding this comment.
HttpRetryStrategyOptions uses its default retry predicate because ShouldHandle is not configured. That predicate retries HTTP 429 responses, so a throttled non-idempotent API call is issued four times: the original request plus the three configured retries. The previous AddTransientHttpErrorPolicy behavior sent the identical 429 response once. Restrict the retry predicate to the intended transient failures so rate limiting is returned to the caller instead of repeating POST and DELETE operations.
Artifacts
trex-artifacts/http-429-repro.cs
- Runtime harness source that sends a non-idempotent task-completion POST to an in-process handler returning HTTP 429 and counts the requests.
trex-artifacts/http-429-01-before.log
- Executed git-parent policy run showing HTTP 429 produced one POST request, so the old policy did not retry it.
trex-artifacts/http-429-02-after.log
- Executed changed-policy run showing HTTP 429 produced four POST requests, confirming three configured retries.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/clients/Elsa.Api.Client/Options/ElsaClientBuilderOptions.cs
Line: 43-50
Comment:
**HTTP 429 retries repeat task mutations**
`HttpRetryStrategyOptions` uses its default retry predicate because `ShouldHandle` is not configured. That predicate retries HTTP 429 responses, so a throttled non-idempotent API call is issued four times: the original request plus the three configured retries. The previous `AddTransientHttpErrorPolicy` behavior sent the identical 429 response once. Restrict the retry predicate to the intended transient failures so rate limiting is returned to the caller instead of repeating POST and DELETE operations.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
PR author is not in the allowed authors list. |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 35183230 | Triggered | Generic High Entropy Secret | 168e3c7 | src/apps/Elsa.ModularServer.Web/appsettings.json | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Purpose
Replace
Elsa.Api.Client's deprecatedPolly.Extensions.Httpintegration so downstream consumers no longer inherit an unresolvable deprecated-package warning. The client now uses the currentMicrosoft.Extensions.Http.Resiliencepipeline for its default HTTP retry behavior.Scope
Select one primary concern:
Description
Problem
Elsa.Api.ClientreferencedPolly.Extensions.Http3.0.0, which NuGet marks as deprecated. Any project consuming the client inherited that warning transitively with no downstream fix available.Solution
Client resilience migration
AddTransientHttpErrorPolicy(...)registration with aMicrosoft.Extensions.Http.Resilienceretry handler.Dependency cleanup
Polly.Extensions.HttpfromElsa.Api.Client.Microsoft.Extensions.Http.PollyfromElsa.Api.Client.Focused coverage
Example of the new default registration shape:
Verification
Steps:
/src/clients/Elsa.Api.Client/Elsa.Api.Client.csprojand confirmPolly.Extensions.Http/Microsoft.Extensions.Http.Pollyare removed andMicrosoft.Extensions.Http.Resilienceis referenced./src/clients/Elsa.Api.Client/Options/ElsaClientBuilderOptions.csand confirm the default retry configuration usesAddResilienceHandler(...).AddRetry(...).dotnet list src/clients/Elsa.Api.Client/Elsa.Api.Client.csproj package --deprecated.Expected outcome:
Elsa.Api.Clientno longer reports deprecated packages, and its default HTTP client registration uses the resilience pipeline API instead of the legacy Polly HTTP integration.Screenshots / Recordings (if applicable)
Commit Convention
We recommend using conventional commit prefixes:
fix:– Bug fixes (behavior change)feat:– New featuresrefactor:– Code changes without behavior changedocs:– Documentation updateschore:– Maintenance, tooling, or dependency updatestest:– Test additions or modificationsClear commit messages make reviews easier and history more meaningful.
Checklist