Retry of "Safe" HTTP requests

[samsp-msft]

when a request fails - retry against a different server

[analogrelay]

A big thing to consider here is request "safety" and idempotency.

A "safe" request is one which does not modify server state (other than diagnostic/tracing data), such as GET, HEAD, or OPTIONS request. (https://developer.mozilla.org/en-US/docs/Glossary/safe)

An idempotent request is one which can be made several times but will only ever have the effect once. All safe requests are idempotent, as well as properly-written PUT and DELETE requests (though it depends on the stability of the identifiers in the URL). (https://developer.mozilla.org/en-US/docs/Glossary/idempotent)

We'll want our retry to be conservative by default and likely only retry on safe requests. When we get an error response from a backend we don't know if the request was partially completed and state may be corrupted such that it's unsafe to retry. Safe requests don't modify state and are thus safe to be retried indefinitely.

We could have options in the future to allow retry on other kinds of request or on certain response errors, but I think those are out-of-scope for v1. So the v1 feature would be retrying of safe requests.

[Tratcher]

Open questions:

• What component initiates the retry? A middleware?
  • You'd want to be able to re-run load balancing.
  • You'd want to be able to update destination health state.
• Nothing with a request body would be considered retriable, you'd have to buffer all request bodies for that to work.
• What conditions are retriable?
  • HttpClient connection exceptions
  • A custom inspection of responses? Intercepting anything with a response body would add complexity.

[karelz]

Triage: We decided to not implement it for 1.0, moving to Future.
We could provide guidance how to do it if needed by customers.

[vdjuric]

Quick, high level idea

• cluster configuration
  • there would be RetryPolicy section
    • with policy handler name (with one or few default implementations available, possible to add custom implementations registered via dependency injection)
    • policy handler config data (handler specific), for example, built-in policy could be defined like this:

{
	"Clusters": {
		"cluster1": {
			"RetryPolicy": {
				"Handler": "SomeNiceHandlerName",

				"RetryOnStatusCode": ["401", "5xx"],
				"RetryOnException": ["System.Net.WebException", "*"],
				"RetryAfter": ["00:00:01", "00:00:03", "00:00:12"],

				"When": {
					"Method": ["GET", "HEAD", "OPTIONS", "*"]
				}
			}
		}
	}
}

ProxyInvokerMiddleware.Invoke, code around "await _httpProxy.ProxyAsync" ?

• add additional catch (Exception) for any unhandled exceptions

  • if there is no retry policy, re-throw exception (current behavior)
  • if there is retry policy, evaluate RetryPolicy with unhandled exception and repeat _httpProxy.ProxyAsync if needed

• if there are no unhandled exceptions after "await _httpProxy.ProxyAsync"

  • if there is retry policy:
    • evaluate RetryPolicy (can use HttpStatusCodes, Exception, IProxyErrorFeature, request method type, ...) and repeat _httpProxy.ProxyAsync if needed
    • if request still fails after few retries
      • destination health is updated accordingly
  • if there are no retry policies:
    • current behavior

[Tratcher]

ProxyInvokerMiddleware.Invoke, code around "await _httpProxy.ProxyAsync" ?

I expect this will be in its own middleware, like load balancing. That allows you to position it before or after load balancing and potentially retry on a different destination (naturally or forcefully selected).

  * destination health is updated accordingly

That's already handled by the passive health checks middleware. With retry as its own middleware we can ensure passive health checks see each failure before we retry it.

[vdjuric]

Sorry for late reply.

I think you are correct, current architecture seems very very good and it should be possible to achieve retry policy with special "retry middleware" (leaving ProxyInvokerMiddleware, HttpProxy components as is).

[AnderssonPeter]

@vdjuric any chance you could add a sample with retry logic? I'm just getting started with this framework and i have no idea where to add the retry logic.

A big plus would be if it was doable with the Direct Proxying method.

[vdjuric]

Hi @AnderssonPeter,

In your Startup.cs, Configure method you would have to register custom "RetryMiddleware" as following:

app.UseEndpoints(endpoints =>
{
	endpoints.MapReverseProxy(app =>
	{
		app.UseAffinitizedDestinationLookup();
		app.UseProxyLoadBalancing();
		app.UsePassiveHealthChecks();
		app.UseMiddleware<RetryMiddleware>(); //add custom retry logic to chain
	});
});

where RetryMiddleware is standard ASP.NET Core Middleware.

Implementation is still on my TODO list :)

Kind regards,
Vladimir Djurić
https://vladimir.djuric.si/

[Henery309]

@vdjuric don't think this approach will because:

• HttpProxy is already reading the request stream, you won't be able to read the request again in Retry middleware.
• _httpProxy.ProxyAsync sets the response content you'd have to inherit HttpTransformer and override TransformResponseAsync so that it short circuits the response. This is doable, in preview 11. The problem will be with point 1. Interested to know if there is a work around.

[Tratcher]

It would require buffering the request body. HttpRequest.EnableBuffering() is one way to do that, but it adds a lot of overhead to every request just in case you need to retry it.

It should be noted that requests with bodies are general considered unsafe to retry anyways given they are likely to cause side-effects and you don't know how far their processing got the first time. E.g. you wouldn't want to double charge someone's credit card.

[vdjuric]

I think overhead and unsafe retries are acceptable if user explicitly enables this behavior from configuration for specific endpoint. This behavior would be disabled by default.

If user enables this, possible duplicated requests should be properly handled by server (for example, you could introduce unique request id (sent by client) and server would know if this request was already processed).

[Henery309]

I Agree with @vdjuric, should be off by default. We want to retry request that result in intermittent failure. Our downstream services return custom http code when a intermittent failure occurs in which case the client is responsible for retrying the request.

Also buffering can be avoid if http request object can be made available to retry middleware. HttpProxy is passing the request object to http client here:

reverse-proxy/src/ReverseProxy/Service/Proxy/HttpProxy.cs

Line 131 in 7e71b11

destinationResponse = await httpClient.SendAsync(destinationRequest, requestTimeoutToken);

[Tratcher]

Also buffering can be avoid if http request object can be made available to retry middleware.

How will that help avoid buffering the request body?

[Henery309]

How will that help avoid buffering the request body?

You are right it's not possible. I just read through CreateRequestMessageAsync and it just appears to be just forwarding the stream to destination. I was thinking http proxy converted request into string and then forwarded that to http client.

[Henery309]

Had to roll up my own middleware for my specific use case . Not sure if this is optimal solution, any feedback is appreciated :)
Still unfinished have to complete exception handling.

Use case

• Have to use http 1.1
• If first destination fails retry request using second destination
• Do not use first destination for 16 minutes. Subsequent request must go to secondary destination (Even if second destination starts to fail).
• Retry on specific response header or if downstream returns http 500

Implementation

• Using a custom middleware with http client to send request to downstream service.
• Custom middleware is injected just before ForwarderMiddleware and it short circuits so that ForwarderMiddleware does not receive the request.
• When downstream returns an error we mark the current destination as unhealthy. I am using yarp health feature for this.

Code
Startup.cs

    public class Startup
    {
        public Startup(IConfiguration configuration)
        {
          
            Configuration = configuration;
        }

        public IConfiguration Configuration { get; }

      
        public void ConfigureServices(IServiceCollection services)
        {
            var proxyBuilder = services.AddReverseProxy();
            proxyBuilder.LoadFromConfig(Configuration.GetSection("ReverseProxy"));
        }

        // This method gets called by the runtime. Use this method to configure the HTTP request 
        // pipeline that handles requests
        public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
        {
            app.UseRouting();
            app.UseEndpoints(endpoints =>
            {
                endpoints.MapReverseProxy(m =>
                {
                    m.UsePassiveHealthChecks();
                    m.UseMiddleware<CustomForwarderMiddleware>();
                });
            });
        }
    }

CustomForwarderMiddleware

    public class CustomForwarderMiddleware
    {
        private readonly RequestDelegate _next;
        private const string RedirectToSecondaryDestination = "x-redirect-secondary-destination";

        public CustomForwarderMiddleware(RequestDelegate next)
        {
            _next = next;
        }

        public async Task InvokeAsync(HttpContext context, IDestinationHealthUpdater healthUpdater)
        {
            #region Prepare
            var route = context.GetRouteModel();
            var cluster = route.Cluster!;
            #endregion

            #region Read Request Content
            // We have to use the request content twice so parse and store.
            var reader = new StreamReader(context.Request.Body, Encoding.UTF8, bufferSize: 1024);
            var requestPayload = await reader.ReadToEndAsync();
            reader.Dispose();
            #endregion

            #region Find Healthy Desitnation
            var reverseProxyFeature = context.GetReverseProxyFeature();

            // Find destination which is not marked as unhealthy.
            var healthyDestination = reverseProxyFeature
                .AllDestinations
                .Where(m => m.Health.Passive != DestinationHealth.Unhealthy)
                .OrderBy(m => m.DestinationId)
                .ToList();

            // Get the first destination we should always have 1 healthy destination.
            var destinationConfig = healthyDestination[0];

            #endregion

            #region Prepare Cancellation Token
            var requestAborted = context.RequestAborted;
            var requestTimeoutSource = CancellationTokenSource.CreateLinkedTokenSource(requestAborted);
            requestTimeoutSource.CancelAfter(TimeSpan.FromSeconds(50));
            var requestTimeoutToken = requestTimeoutSource.Token;
            #endregion

            try
            {
                // Build the first request.
                var request = await GetRequestMessage(requestPayload, route, context, destinationConfig);

                // Call downstream service.
                var responseOne = await reverseProxyFeature.Cluster.HttpClient.SendAsync(request, requestTimeoutToken);

                // If downstream service returns http 500 or response contains RedirectToSecondaryDestination header. 
                // Then we have to mark destination_1 as unhealthy and retry request using secondary destination.
                var redirectToDestination2 = responseOne.Headers.Contains(RedirectToSecondaryDestination);
                if (((int)responseOne.StatusCode) >= 500 || redirectToDestination2)
                {
                    // Step 1 - Mark first destination as unhealthy for 16 minutes.
                    if (destinationConfig.DestinationId.Equals("destination_1"))
                        healthUpdater.SetPassive(cluster, destinationConfig, DestinationHealth.Unhealthy, TimeSpan.FromMinutes(16));

                    // Step 2 - Build request and try
                    var secondaryDestination = healthyDestination.FirstOrDefault(m => m.DestinationId.Equals("destination_2"));
                    if (secondaryDestination != null)
                    {
                        // Call downstream service using secondary destination.
                        var httpRequest = await GetRequestMessage(requestPayload, route, context, secondaryDestination);
                        var secondaryResponse = await reverseProxyFeature.Cluster.HttpClient.SendAsync(httpRequest, requestTimeoutToken);

                        // Transform request and return to client.
                        context.Response.StatusCode = (int)secondaryResponse.StatusCode;
                        await route.Transformer.TransformResponseAsync(context, secondaryResponse);
                        await secondaryResponse.Content.CopyToAsync(context.Response.Body, requestTimeoutToken);
                    }
                    else
                    {
                        context.Response.StatusCode = StatusCodes.Status500InternalServerError;
                    }
                }
                else
                {
                    context.Response.StatusCode = (int)responseOne.StatusCode;
                    await route.Transformer.TransformResponseAsync(context, responseOne);
                    await responseOne.Content.CopyToAsync(context.Response.Body, requestTimeoutToken);
                }
            }
            catch (OperationCanceledException canceledException)
            {
                if (!requestAborted.IsCancellationRequested && requestTimeoutToken.IsCancellationRequested)
                {
                    context.Response.Clear();
                    context.Response.StatusCode = StatusCodes.Status504GatewayTimeout;
                    return;
                }
                context.Response.StatusCode = StatusCodes.Status502BadGateway;
            }
            catch (Exception requestException)
            {
                //TODO
            }
            finally
            {
                requestTimeoutSource.Dispose();
            }
        }

        public async Task<HttpRequestMessage> GetRequestMessage(string payload, RouteModel route, HttpContext context, DestinationState destinationConfig)
        {
            var httpRequest = new HttpRequestMessage
            {
                Method = GetMethod(context.Request.Method),
                Version = HttpVersion.Version11,
                VersionPolicy = HttpVersionPolicy.RequestVersionOrLower,
                Content = new StringContent(payload, Encoding.UTF8)
            };

            // Builds RequestUri and Copies Headers
            await route.Transformer.TransformRequestAsync(context, httpRequest, destinationConfig.Model.Config.Address);

            return httpRequest;
        }
        
        #region GetMethod

        private static HttpMethod GetMethod(string method)
        {
            if (HttpMethods.IsDelete(method)) return HttpMethod.Delete;
            if (HttpMethods.IsGet(method)) return HttpMethod.Get;
            if (HttpMethods.IsHead(method)) return HttpMethod.Head;
            if (HttpMethods.IsOptions(method)) return HttpMethod.Options;
            if (HttpMethods.IsPost(method)) return HttpMethod.Post;
            if (HttpMethods.IsPut(method)) return HttpMethod.Put;
            if (HttpMethods.IsTrace(method)) return HttpMethod.Trace;
            return new HttpMethod(method);
        }

        #endregion
    }

appsettings.json

{
  "Logging": {
    "LogLevel": {
      "Default": "Debug",
      "Microsoft": "Debug",
      "Microsoft.Hosting.Lifetime": "Debug",
      "Yarp": "Debug"
    }
  },
  "AllowedHosts": "*",
  "ReverseProxy": {
    "Routes": {
      "Route8": {
        "ClusterId": "api1",
        "Match": {
          "Methods": [ "Post", "GET" ],
          "Path": "/api1/v1/{*s}"
        },
        "Transforms": [
          {
            "PathSet": "/posts"
          },
          {
            "ResponseHeader": "Access-Control-Expose-Headers",
            "Append": "IsSuccessful",
            "When": "Always"
          },
          {
            "ResponseHeader": "Access-Control-Max-Age",
            "Append": "86400",
            "When": "Always"
          }
        ]
      }
    },
    "Clusters": {
      "api1": {
        "Destinations": {
          "destination_1": {
            // this destination generates http 500
            "Address": "https://webhook.site/5166e19c-9665-4db9-a05a-a7fb800a1cf4"
          },
          "destination_2": {
            "Address": "https://myapi1.net"
          }
        }
      }
    }
  }
}

[Tratcher]

@Henery309 glad you got it working. Some feedback:

• Let active or passive health checks handle pulling destinations out of rotation. This middleware's job is to retry from the list of available destinations.
• Don't bypass the forwarding middleware, you loose a lot of functionality that way.
• Buffer the request body using HttpContex.Request.EnableBuffering(). That stores the content as bytes, there's no need to decode them to strings. You also don't have to pre-read it, it will buffer as it's proxied.
• Use AvalableDestinations as your original list, no need to duplicate the health filtering here.
• Use a response transform to suppress the response body on failure.

  reverse-proxy/src/ReverseProxy/Transforms/ResponseTransformContext.cs

  Line 35 in 219a2fe

  public bool SuppressResponseBody { get; set; }

  , then your middleware can check the result and ForwarderError on the error feature https://microsoft.github.io/reverse-proxy/articles/direct-forwarding.html#error-handling.
• On failure, drop the selected destination from the list of available, update the feature's list of available, and try again using the normal pipeline. Repeat as desired or until you run out of destinations.

[Henery309]

@Tratcher thanks for you feedback really appreciated it.

Based on the feedback I am trying to simplify my solution. But when retrying on second destination I am getting.
Reading the request body timed out due to data arriving too slowly. See MinRequestBodyDataRate. on the second destination machine log file.

Here is my second approach.

1 - Retry middleware is added in the beginning of all middleware.

        public void Configure(IApplicationBuilder app)
        {
            app.UseMiddleware<RetryMiddleware>();
            app.UseRouting();
            app.UseEndpoints(endpoints =>
            {
                endpoints.MapReverseProxy();
            });
        }

2 - SuppressResponseBody is set to true if error occurs

        public void ConfigureServices(IServiceCollection services)
        {
            var proxyBuilder = services.AddReverseProxy().AddTransforms(m =>
            {
                m.AddResponseTransform(context =>
                {
                    // Suppress the response body from errors.
                    // The status code was already copied.
                    if ((int)context.ProxyResponse.StatusCode >= 500) 
                        context.SuppressResponseBody = true;

                    return default;
                });
            });
            proxyBuilder.LoadFromConfig(Configuration.GetSection("ReverseProxy"));
        }

3 - I rely on health check middleware to mark failed destination as unhealthy.

"Clusters": {
      "api1": {
        "HttpClient": {
          "DangerousAcceptAnyServerCertificate": true
        },
        "HttpRequest": {
          "Timeout": "00:01:00",
          "Version": "1.1"
        },
        "HealthCheck": {
          "Passive": {
            "Enabled": "true",
            "Policy": "TransportFailureRate",
            "ReactivationPeriod": "00:16:00"
          }
        },
        "MetaData": {
          "TransportFailureRateHealthPolicy.MinimalTotalCountThreshold": "1",
          "TransportFailureRateHealthPolicy.DefaultFailureRateLimit": "0.1",
          "TransportFailureRateHealthPolicy.DetectionWindowSize": "15"
        },
        "LoadBalancingPolicy": "FirstAlphabetical",
        "Destinations": {
          "destination_1": {
            // this destination generates http 500
            "Address": "https://webhook.site/5166e19c-9665-4db9-a05a-a7fb800a1cf4"
          },
          "destination_2": {
            "Address": "https://localhost:5001"
          }
        }
      }
    }

4 - Finally in my retry middleware if downstream returns http 500 or above I am using IHttpForwarder to rerun the same request again. Retrying is resulting in Reading the request body timed out due to data arriving too slowly. See MinRequestBodyDataRate. error on the downstream destination api. The downstream API is just a local host API. Any idea why this could be the issue?

public async Task InvokeAsync(HttpContext context)
        {
            context.Request.EnableBuffering();
            
            await _next(context);

            var statusCode = context.Response.StatusCode;
            if (statusCode >= 500)
            {
                var reverseProxyFeature = context.GetReverseProxyFeature();

                // Get available destinations
                var healthyDestination = reverseProxyFeature
                    .AvailableDestinations
                    .ToList();

                // Only retry if we have a destination available.
                if (healthyDestination.Count > 0)
                {
                    // Get configs of healthy destination 
                    var destinationConfig = healthyDestination[0];

                    // Snapshot request to reply the request.
                    var clusterConfig = reverseProxyFeature.Cluster;

                    // Current route config request to reply request.
                    var routeConfig = context.GetRouteModel();

                    // Finally forward request to secondary destination. If all goes well the request will be successful.
                    await _httpForwarder.SendAsync(context, destinationConfig.Model.Config.Address, clusterConfig.HttpClient, clusterConfig.Config.HttpRequest, routeConfig.Transformer);
                }
            }
        }

[Tratcher]

• Why did you move your retry middleware out of MapReverseProxy? You'll want it inside there to get full access to proxy features.
• You need to rewind the request body before retrying: httpContext.Request.Body.Position = 0;
• You shouldn't need IHttpForwarder for the retry, you should be able to call _next again after clearing some state like reverseProxyFeature.SelectedDestination and updating reverseProxyFeature.AvailableDestinations to remove the previous SelectedDestination.

[Henery309]

Thanks @Tratcher it worked. I actually did not know that you can re-try middleware by just calling _next again. I thought pipelines only run sequence in 1 direction. Not sure if this should be added to https://docs.microsoft.com/en-us/aspnet/core/fundamentals/middleware/?view=aspnetcore-5.0#middleware-order.

As per the retry middleware this is my final code.

1 - Startup.cs

        public void Configure(IApplicationBuilder app)
        {
            app.UseRouting();
            app.UseEndpoints(endpoints =>
            {
                endpoints.MapReverseProxy(m =>
                {
                    m.UseMiddleware<RetryMiddleware>();
                    m.UseLoadBalancing();
                    m.UsePassiveHealthChecks();
                });
            });
        }

2 - RetryMiddleware

    public class RetryMiddleware
    {
        private readonly RequestDelegate _next;

        public RetryMiddleware(RequestDelegate next)
        {
            _next = next;
        }

        public async Task InvokeAsync(HttpContext context)
        {
            context.Request.EnableBuffering();
            
            // Proceed to next middleware
            await _next(context);

            var statusCode = context.Response.StatusCode;
            
            if (statusCode >= 500)
            {
                var reverseProxyFeature = context.GetReverseProxyFeature();

                // Get available destinations
                var healthyDestination = reverseProxyFeature
                    .AllDestinations
                    .Where(m => m.Health.Passive != DestinationHealth.Unhealthy)
                    .ToList();

                // Set the available destination to available healthy destination.
                reverseProxyFeature.AvailableDestinations = healthyDestination;

                context.Request.Body.Position = 0;

                // Rerun middleware, this will re-run LoadBalancingMiddleware PassiveHealthCheckMiddleware and  ForwarderMiddleware
                await _next(context);

            }
        }
    }

[Tratcher]

Much better. A few more suggestions (coding in the browser, may not compile 😁):

    public class RetryMiddleware
    {
        private readonly RequestDelegate _next;

        public RetryMiddleware(RequestDelegate next)
        {
            _next = next;
        }

        public async Task InvokeAsync(HttpContext context)
        {
            context.Request.EnableBuffering();

            var reverseProxyFeature = context.GetReverseProxyFeature();
            var available = reverseProxy.AvailableDestinations;
            
            // Proceed to next middleware
            await _next(context);

            var statusCode = context.Response.StatusCode;
            
            if (statusCode >= 500)
            {
                // Update available destinations to exclude the one we just tried.
                var healthyDestinations = available
                    .Where(m => m != reverseProxyFeature.ProxiedDestination)
                    .ToList();
               if (healthyDestinations.Count == 0)
               {
                   return;
               }

                // Set the available destination to available healthy destination.
                reverseProxyFeature.AvailableDestinations = healthyDestinations;

                context.Request.Body.Position = 0;
                reverseProxyFeature.ProxiedDestination = null;

                // Rerun middleware, this will re-run LoadBalancingMiddleware PassiveHealthCheckMiddleware and  ForwarderMiddleware
                await _next(context);
            }
        }
    }

[Henery309]

Hi @Tratcher,

PathPattern transformation does not work correctly when using retry middleware. For the first request the path is appended to the request url as expected. However, for second retry path is not appended. This is because the context.Request.RouteValues is empty.

After this line the RouteValues is cleared.

reverse-proxy/src/ReverseProxy/Transforms/PathRouteValuesTransform.cs

Line 42 in d31b388

context.Path = binder.BindValues(acceptedValues: routeValues);

For a work around I am storing the route Values in a Dictionary<string,string> and then re populating them in context.Request.RouteValues just before the _next. This seems a bit hacky any better solution?

Following is my configs

"Routes": {
      "Route8": {
        "ClusterId": "api1",
        "Match": {
          "Methods": [ "Post" ],
          "Path": "myctrl/v1/{*routeName}"
        },
        "Transforms": [
          {
            "PathPattern": "/{routeName}"
          }
        ]
      }
    }

[Tratcher]

Oh, I didn't know the binder removed matched values from the collection. That's not great. Can you please make a separate issue for that? We don't want the transforms to have side effects on the original request.
https://github.com/dotnet/aspnetcore/blob/23b704915cdb9a73df7a61ff1aaa894712098615/src/Http/Routing/src/Template/TemplateBinder.cs#L556

Making a backup copy of the route values is your best mitigation for now.

[Henery309]

@Tratcher done.

#1163

[karelz]

Triage: We should limit it to safe HTTP methods without body.
We need to design retry policies.

[divinebovine]

@karelz graphql has a use case where posts can be a safe request. Any request bodies without a mutation would be considered safe. I think it would be good to try to support graphql along with traditional rest services.

[karelz]

@divinebovine well, POST is in general NOT safe, right? How would one distinguish if it is graphql safe POST vs. not?
Do you have a specific deployment which requires it, or are you just trying to optimize things "just in case"?

[Tratcher]

The other reason we'd avoid POST is that it would require buffering every request body. That's more overhead than we'd want to introduce. Since most aren't safe anyways, that shouldn't be a major loss.

[divinebovine]

@karelz I agree that POST is generally NOT safe per http specifications. However, graphql over http implementations like Apollo, require all queries (safe) and mutations (unsafe) with variables to be sent as a POST.

I understand not wanting to do it like @Tratcher mentioned, though it would be nice to be able to specify a retry middleware specific for certain routes that could inspect the bodies and determine if a retry would be safe.

Edit:
To answer your previous question @karelz yes we are needing to retry failed graphql requests. We initially tried some of the suggestions in this thread and were unsuccessful.

[andredewes]

While we don't have a built-in retry feature I'm also trying to perform it with a HTTP POST body scenario by using the RetryMiddleware posted by @Tratcher together with the SuppressBodyResponse transform code (originally posted by @Henery309):

            var proxyBuilder = services.AddReverseProxy().AddTransforms(m =>
            {
                m.AddResponseTransform(context =>
                {
                    // Suppress the response body from errors.
                    // The status code was already copied.
                    if ((int)context.ProxyResponse.StatusCode >= 500) 
                        context.SuppressResponseBody = true;

                    return default;
                });
            });

The problem is that when we call the middleware again in case of errors 500):

// Rerun middleware, this will re-run LoadBalancingMiddleware PassiveHealthCheckMiddleware and  ForwarderMiddleware
await _next(context);

It throws an exception saying the response already started:

System.InvalidOperationException: The request cannot be forwarded, the response has already started
   at Yarp.ReverseProxy.Forwarder.HttpForwarder.SendAsync(HttpContext context, String destinationPrefix, HttpMessageInvoker httpClient, ForwarderRequestConfig requestConfig, HttpTransformer transformer, CancellationToken cancellationToken)
   at Yarp.ReverseProxy.Forwarder.ForwarderMiddleware.Invoke(HttpContext context)
   at Yarp.ReverseProxy.Health.PassiveHealthCheckMiddleware.Invoke(HttpContext context)
   at openai_loadbalancer.LoadBalancer.RetryMiddleware.InvokeAsync(HttpContext context) in 

Shouldn't the context.SuppressResponseBody = true; prevent that response from being written?
Right before calling the middleware I checked if context.Response.HasStarted is false and it is.

Any insights here will be appreciated.

[Tratcher]

Here's the actual check for that error:

reverse-proxy/src/ReverseProxy/Forwarder/RequestUtilities.cs

Lines 419 to 423 in c664310

	internal static bool IsResponseSet(HttpResponse response)
	{
	return response.StatusCode != StatusCodes.Status200OK
	|| response.HasStarted;
	}

I suggest calling HttpResponse.Clear() before retrying. SuppressResponseBody prevents the body from being proxied, but the status and headers are still copied and should be cleared out.

[Elderry]

Hi @andredewes , have you fixed your problem? My problem looks the same, SuppressResponseBody doesn't prevent the response from being written. Also I tried @Tratcher 's suggestion to invoke HttpResponse.Clear() before retrying, but it just change the error message into The response cannot be cleared, it has already started sending.

Here is my code:

• in RetryMiddleware.cs

public async Task InvokeAsync(HttpContext context)
{
    context.Request.EnableBuffering();
    await _next(context); // returns 500
    context.Response.Clear();
    await _forwarder.SendAsync(context, "http://localhost:5002", _client); // calls an externeral service which returns 200
}

• in 500Controller.cs

[HttpGet]
public ActionResult Get()
{
    return new StatusCodeResult(500);
}

• in Program.cs

builder.Services.AddReverseProxy().AddTransforms(m =>
{
    m.AddResponseTransform(context =>
    {
        context.SuppressResponseBody = true;
        return default;
    });
});

[Elderry]

Another idea is that my case is slightly different because the 1st attempt is processed in local controller, only forward to external service if failed. If that is the reason, may I ask what is the correct practice then?

[RelDGanama]

Hi @Elderry and @Tratcher I realized the issue in the code @Elderry posted is YARP will be suppressing all responses in the current way the transform is configured. You will need to add some logic to stop it when the response YARP receives is the one you want to send back to your client.

Something like

builder.Services.AddReverseProxy()
    .LoadFromConfig(builder.Configuration.GetSection("ReverseProxy"))
    .AddTransforms(builderContext =>
    {
            builderContext.AddResponseTransform(transformContext =>
            {
                    transformContext.SuppressResponseBody = transformContext.HttpContext.Response.StatusCode == 429;
                    return default;
             });
     });

In this case only suppress 429s and retry them or add some more detailed logic to stop within a certain retry count