Skip to content

Disable request timeout for streaming methods #5

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 21, 2023
Merged

Disable request timeout for streaming methods #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dbb4c0f
Disable request timeout for streaming methods
JamesNK Apr 20, 2023
b483c6b
Test STJ issue
JamesNK Apr 20, 2023
97ffe52
Test STJ issue - add switch
JamesNK Apr 20, 2023
382deeb
Apply suggestions from code review
JamesNK Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/Grpc.AspNetCore.Server/Internal/CallHandlers/ClientStreamingServerCallHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand Down Expand Up @@ -45,8 +45,11 @@ public ClientStreamingServerCallHandler(

protected override async Task HandleCallAsyncCore(HttpContext httpContext, HttpContextServerCallContext serverCallContext)
{
// Disable request body data rate for client streaming
// Disable certain features for client streaming methods.
DisableMinRequestBodyDataRateAndMaxRequestBodySize(httpContext);
#if NET8_0_OR_GREATER
DisableRequestTimeout(httpContext);
#endif

TResponse? response;

Expand Down
7 changes: 5 additions & 2 deletions src/Grpc.AspNetCore.Server/Internal/CallHandlers/DuplexStreamingServerCallHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand Down Expand Up @@ -44,8 +44,11 @@ public DuplexStreamingServerCallHandler(

protected override async Task HandleCallAsyncCore(HttpContext httpContext, HttpContextServerCallContext serverCallContext)
{
// Disable request body data rate for client streaming
// Disable certain features for duplex streaming methods.
DisableMinRequestBodyDataRateAndMaxRequestBodySize(httpContext);
#if NET8_0_OR_GREATER
DisableRequestTimeout(httpContext);
#endif

var streamReader = new HttpContextStreamReader<TRequest>(serverCallContext, MethodInvoker.Method.RequestMarshaller.ContextualDeserializer);
var streamWriter = new HttpContextStreamWriter<TResponse>(serverCallContext, MethodInvoker.Method.ResponseMarshaller.ContextualSerializer);
Expand Down
28 changes: 27 additions & 1 deletion src/Grpc.AspNetCore.Server/Internal/CallHandlers/ServerCallHandlerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand All @@ -21,6 +21,9 @@
using Grpc.Shared.Server;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
#if NET8_0_OR_GREATER
using Microsoft.AspNetCore.Http.Timeouts;
#endif
using Microsoft.AspNetCore.Server.Kestrel.Core.Features;
using Microsoft.Extensions.Logging;
using Microsoft.Net.Http.Headers;
Expand Down Expand Up @@ -134,6 +137,29 @@ protected void DisableMinRequestBodyDataRateAndMaxRequestBodySize(HttpContext ht
}
}

#if NET8_0_OR_GREATER
protected void DisableRequestTimeout(HttpContext httpContext)
{
// Disable global request timeout on streaming methods.
var requestTimeoutFeature = httpContext.Features.Get<IHttpRequestTimeoutFeature>();
if (requestTimeoutFeature is not null)
{
// Don't disable if the endpoint has explicit timeout metadata.
var endpoint = httpContext.GetEndpoint();
if (endpoint is not null)
{
if (endpoint.Metadata.GetMetadata<RequestTimeoutAttribute>() is not null ||
Copy link

@BrennanConroy BrennanConroy Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

I don't think we'd want to include this in SignalR though since it's designed for long running request.

endpoint.Metadata.GetMetadata<RequestTimeoutPolicy>() is not null)
{
return;
}
}

requestTimeoutFeature.DisableTimeout();
}
}
#endif

private Task ProcessNonHttp2Request(HttpContext httpContext)
{
GrpcServerLog.UnsupportedRequestProtocol(Logger, httpContext.Request.Protocol);
Expand Down
7 changes: 6 additions & 1 deletion src/Grpc.AspNetCore.Server/Internal/CallHandlers/ServerStreamingServerCallHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand Down Expand Up @@ -44,6 +44,11 @@ public ServerStreamingServerCallHandler(

protected override async Task HandleCallAsyncCore(HttpContext httpContext, HttpContextServerCallContext serverCallContext)
{
// Disable certain features for server streaming methods.
#if NET8_0_OR_GREATER
DisableRequestTimeout(httpContext);
#endif

// Decode request
var request = await httpContext.Request.BodyReader.ReadSingleMessageAsync<TRequest>(serverCallContext, MethodInvoker.Method.RequestMarshaller.ContextualDeserializer);

Expand Down
58 changes: 57 additions & 1 deletion test/Grpc.AspNetCore.Server.Tests/CallHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand All @@ -22,7 +22,11 @@
using Grpc.Core;
using Grpc.Shared.Server;
using Grpc.Tests.Shared;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
#if NET8_0_OR_GREATER
using Microsoft.AspNetCore.Http.Timeouts;
#endif
using Microsoft.AspNetCore.Server.Kestrel.Core.Features;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -167,6 +171,58 @@ public async Task ProtocolValidation_IISHttp2Protocol_Success()
}
#endif

#if NET8_0_OR_GREATER
[TestCase(MethodType.Unary, false)]
[TestCase(MethodType.ClientStreaming, true)]
[TestCase(MethodType.ServerStreaming, true)]
[TestCase(MethodType.DuplexStreaming, true)]
public async Task RequestTimeoutFeature_Global_DisableWhenStreaming(MethodType methodType, bool expectedTimeoutDisabled)
{
// Arrange
var timeoutFeature = new TestHttpRequestTimeoutFeature();
var httpContext = HttpContextHelpers.CreateContext();
httpContext.Features.Set<IHttpRequestTimeoutFeature>(timeoutFeature);
var call = CreateHandler(methodType);

// Act
await call.HandleCallAsync(httpContext).DefaultTimeout();

// Assert
Assert.AreEqual(expectedTimeoutDisabled, timeoutFeature.TimeoutDisabled);
}

[TestCase(MethodType.Unary)]
[TestCase(MethodType.ClientStreaming)]
[TestCase(MethodType.ServerStreaming)]
[TestCase(MethodType.DuplexStreaming)]
public async Task RequestTimeoutFeature_WithEndpointMetadata_NotDisabledWhenStreaming(MethodType methodType)
{
// Arrange
var timeoutFeature = new TestHttpRequestTimeoutFeature();
var httpContext = HttpContextHelpers.CreateContext();
httpContext.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(new RequestTimeoutAttribute(100)), "Test endpoint"));
httpContext.Features.Set<IHttpRequestTimeoutFeature>(timeoutFeature);
var call = CreateHandler(methodType);

// Act
await call.HandleCallAsync(httpContext).DefaultTimeout();

// Assert
Assert.False(timeoutFeature.TimeoutDisabled);
}

private sealed class TestHttpRequestTimeoutFeature : IHttpRequestTimeoutFeature
{
public bool TimeoutDisabled { get; private set; }
public CancellationToken RequestTimeoutToken { get; }

public void DisableTimeout()
{
TimeoutDisabled = true;
}
}
#endif

[Test]
public async Task StatusDebugException_ErrorInHandler_SetInDebugException()
{
Expand Down
3 changes: 2 additions & 1 deletion testassets/InteropTestsGrpcWebClient/InteropTestsGrpcWebClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk.BlazorWebAssembly">
<Project Sdk="Microsoft.NET.Sdk.BlazorWebAssembly">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<DefineConstants>BLAZOR_WASM</DefineConstants>
<JsonSerializerIsReflectionEnabledByDefault>true</JsonSerializerIsReflectionEnabledByDefault>
</PropertyGroup>

<ItemGroup>
Expand Down