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

YARP not able to detect UseRequestTimeouts middleware #2404

Closed
jasidhu opened this issue Feb 16, 2024 · 12 comments · Fixed by #2501
Closed

YARP not able to detect UseRequestTimeouts middleware #2404

jasidhu opened this issue Feb 16, 2024 · 12 comments · Fixed by #2501
Assignees
Labels
Type: Bug Something isn't working

Comments

@jasidhu
Copy link

jasidhu commented Feb 16, 2024

Describe the bug

I am trying to use .net8 request timeout middleware with Yarp proxy 2.1.0 but proxy is throwing following error.

To Reproduce

Error Message: The timeout was not applied for route 'route1', ensure IApplicationBuilder.UseRequestTimeouts() is called between IApplicationBuilder.UseRouting() and IApplicationBuilder.UseEndpoints()

System.InvalidOperationException: The timeout was not applied for route 'route1', 
ensure `IApplicationBuilder.UseRequestTimeouts()` is called between `IApplicationBuilder.UseRouting()` and `IApplicationBuilder.UseEndpoints()`.
   at Yarp.ReverseProxy.Model.ProxyPipelineInitializerMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutsMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

Further technical details

  • Yarp.ReverseProxy - 2.1.0
  • Local windows 11 machine

The config

"ReverseProxy": {
    "Routes": {
      "route1": {
        "ClusterId": "cluster1",
        "Timeout": "00:05:00",
        "Match": {
          "Methods": [ "GET" ],
          "Path": "/_api/v1/test/{**remainder}"
        }
      }
    },
    "Clusters": {
      "cluster1": {
        "Destinations": {
          "production": {
            "Address": "https://example.com"
          }
        }
      }
    }
  }

Pipeline:

var app = builder.Build();

app.UseHttpsRedirection();
app.UseRouting();
app.UseAuthentication();
app.UseAuthorization();
app.UseRequestTimeouts();
app.UseEndpoints(endpoints => endpoints.MapReverseProxy());

app.Run();
@jasidhu jasidhu added the Type: Bug Something isn't working label Feb 16, 2024
@Tratcher
Copy link
Member

Tratcher commented Feb 16, 2024

Does TimeoutPolicy work instead?
"TimeoutPolicy": "customPolicy",

@jasidhu
Copy link
Author

jasidhu commented Feb 16, 2024

I am getting same error with TimeoutPolicy as well.

Does TimeoutPolicy work instead? "TimeoutPolicy": "customPolicy",

@Kahbazi
Copy link
Collaborator

Kahbazi commented Feb 19, 2024

@jasidhu Is by any chance the debugger is attached? The timeout middleware skips if debugger is attached and there will be no IHttpRequestTimeoutFeature.

@BenWissel
Copy link

BenWissel commented Feb 19, 2024

We are having the same issue which is keeping us from upgrading to .NET 8.0. Below is a snippet of our .NET Core MVC app's Program.cs.

builder.Services.AddRequestTimeouts();

var app = builder.Build();

if (!app.Environment.IsDevelopment())
{
	app.UseHsts();
}

app.UseRouting();
app.UseRequestTimeouts();
app.UseHttpsRedirection();
app.UseStaticFiles();
app.UseAuthentication();
app.UseAuthorization();
app.UseSystemWebAdapters();
app.MapDefaultControllerRoute();
app.UseEndpoints(_ => { });
app.MapReverseProxy();

app.Run();

--Route in appsettings.json throwing error.
"ReverseProxy": {
"Routes": {
"reports_route": {
"ClusterId": "frameworkCluster",
"Timeout": "00:10:00",
"Match": {
"Path": "/Reports"
}
},

@MihaZupan MihaZupan added this to the v.Next milestone Feb 20, 2024
@MihaZupan
Copy link
Member

As far as I can tell we will currently incorrectly flag requests in the following cases:

  • A debugger is attached to the proxy
  • A custom policy returns a null / infinite timeout (seems like a plausibly valid use case, we can detect it)
  • Misc cases we don't care about (e.g. removing existing policy from options, manually removing the timeout feature, ...)

Given a config with just a timeout set (e.g. "Timeout": "00:10:00"), this should only happen if the debugger was attached to the proxy. Is that the case for you @jasidhu, @BenWissel?

@MihaZupan
Copy link
Member

Triage: We should improve the check here to at least avoid throwing when the debugger is attached.
Moving to vNext since this is annoying when debugging and should be simple to fix.

@jasidhu
Copy link
Author

jasidhu commented Feb 20, 2024

"Timeout": "00:05:00",

@MihaZupan Yes. It is failing only when debugger is attached.

@BenWissel
Copy link

Triage: We should improve the check here to at least avoid throwing when the debugger is attached. Moving to vNext since this is annoying when debugging and should be simple to fix.

Thanks for running this down. We've been instructed internally to wait on our upgrade to .NET 8.0 until this issue in YARP is resolved.

@doddgu
Copy link

doddgu commented Mar 18, 2024

Is there any way to work on .NET 8, before the next version?

@MihaZupan
Copy link
Member

As a workaround, you could disable request timeouts in YARP and rely on the activity timeout instead

@bfwissel
Copy link

bfwissel commented Apr 4, 2024

Triage: We should improve the check here to at least avoid throwing when the debugger is attached. Moving to vNext since this is annoying when debugging and should be simple to fix.

Thanks for running this down. We've been instructed internally to wait on our upgrade to .NET 8.0 until this issue in YARP is resolved.

Is there a release schedule for when the timeout bugs will be resolved in YARP? In our application we're having timeout issues with legacy report embedding and would also like to be able to upgrade to .NET 8 sooner rather than later.

@MihaZupan
Copy link
Member

My understanding is that request timeouts are an opt-in (off by default) .NET 8 feature, where you can opt-in via the YARP config.
As such, I don't see why this would be a blocker for migrating to .NET 8. You can avoid enabling the feature and use alternatives such as the existing YARP-specific activity timeout when you do want to override timeouts for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants