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

Update http proxy to handle client disconnect scenario #10688

Merged
merged 11 commits into from
Jan 9, 2025
3 changes: 3 additions & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
<!-- Please add your release notes in the following format:
- My change description (#PR)
-->

- Allow for an output binding value of an invocation result to be null (#10698)
- Updated dotnet-isolated worker to 1.0.12.
- [Corrected the path for the prelaunch app location.](https://github.com/Azure/azure-functions-dotnet-worker/pull/2897)
- [Added net9 prelaunch app.](https://github.com/Azure/azure-functions-dotnet-worker/pull/2898)
- Update the `DefaultHttpProxyService` to better handle client disconnect scenarios (#10688)
- Replaced `InvalidOperationException` with `HttpForwardingException` when there is a ForwarderError
22 changes: 22 additions & 0 deletions src/WebJobs.Script.Grpc/Exceptions/HttpForwardingException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using System;

namespace Microsoft.Azure.WebJobs.Script.Grpc.Exceptions
{
internal class HttpForwardingException : Exception
{
public HttpForwardingException()
{
}

public HttpForwardingException(string message) : base(message)
{
}

public HttpForwardingException(string message, Exception innerException) : base(message, innerException)
{
}
}
}
15 changes: 14 additions & 1 deletion src/WebJobs.Script.Grpc/Server/DefaultHttpProxyService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.Azure.WebJobs.Script.Description;
using Microsoft.Azure.WebJobs.Script.Grpc.Exceptions;
using Microsoft.Azure.WebJobs.Script.Workers;
using Microsoft.Extensions.Logging;
using Yarp.ReverseProxy.Forwarder;
Expand All @@ -20,10 +21,12 @@ internal class DefaultHttpProxyService : IHttpProxyService, IDisposable
private readonly IHttpForwarder _httpForwarder;
private readonly HttpMessageInvoker _messageInvoker;
private readonly ForwarderRequestConfig _forwarderRequestConfig;
private readonly ILogger<DefaultHttpProxyService> _logger;

public DefaultHttpProxyService(IHttpForwarder httpForwarder, ILogger<DefaultHttpProxyService> logger)
{
_httpForwarder = httpForwarder ?? throw new ArgumentNullException(nameof(httpForwarder));
_logger = logger ?? throw new ArgumentNullException(nameof(logger));

_handler = new SocketsHttpHandler
{
Expand Down Expand Up @@ -56,13 +59,23 @@ public async Task EnsureSuccessfulForwardingAsync(ScriptInvocationContext contex

if (httpProxyTaskResult is not ForwarderError.None)
{
_logger.LogDebug($"The function invocation failed to proxy the request with ForwarderError: {httpProxyTaskResult}");

Exception forwarderException = null;
if (context.TryGetHttpRequest(out HttpRequest request))
{
forwarderException = request.HttpContext.GetForwarderErrorFeature()?.Exception;
}

throw new InvalidOperationException($"Failed to proxy request with ForwarderError: {httpProxyTaskResult}", forwarderException);
if (request.HttpContext.RequestAborted.IsCancellationRequested)
liliankasem marked this conversation as resolved.
Show resolved Hide resolved
{
// Client disconnected, nothing we can do here but inform
_logger.LogInformation("The client disconnected while the function was processing the request.");
}
else
{
throw new HttpForwardingException($"Failed to proxy request with ForwarderError: {httpProxyTaskResult}", forwarderException);
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/WebJobs.Script.Grpc/Server/RetryProxyHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
{
return await base.SendAsync(request, cancellationToken);
}
catch (TaskCanceledException) when (cancellationToken.IsCancellationRequested)
{
_logger.LogDebug("Request was canceled. Stopping retries.");
throw new OperationCanceledException(cancellationToken);
}
catch (HttpRequestException) when (attemptCount < MaxRetries)
{
_logger.LogWarning("Failed to proxy request to the worker. Retrying in {delay}ms. Attempt {attemptCount} of {maxRetries}.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,12 @@ public static class ScriptHostServiceLoggerExtension
new EventId(530, nameof(LogHostInitializationSettings)),
"{hostInitializationSettings}");

private static readonly Action<ILogger, string, Exception> _requestAborted =
LoggerMessage.Define<string>(
LogLevel.Debug,
new EventId(531, nameof(RequestAborted)),
"The request was aborted by the client (requestId: '{mS_ActivityId}').");
liliankasem marked this conversation as resolved.
Show resolved Hide resolved

public static void HostStateChanged(this ILogger logger, ScriptHostState previousHostState, ScriptHostState newHostState)
{
var newState = newHostState.ToString();
Expand All @@ -205,6 +211,11 @@ public static void ExecutedHttpRequest(this ILogger logger, string mS_ActivityId
_executedHttpRequest(logger, mS_ActivityId, identities, statusCode, duration, null);
}

public static void RequestAborted(this ILogger logger, string mS_ActivityId)
{
_requestAborted(logger, mS_ActivityId, null);
}

public static void ScriptHostServiceInitCanceledByRuntime(this ILogger logger)
{
_scriptHostServiceInitCanceledByRuntime(logger, null);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using System;
using System.IO;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics.Extensions;
using Microsoft.Extensions.Logging;

namespace Microsoft.Azure.WebJobs.Script.WebHost.Middleware
{
internal class HandleCancellationMiddleware
{
private readonly ILogger _logger;
private readonly RequestDelegate _next;

public HandleCancellationMiddleware(RequestDelegate next, ILogger<HandleCancellationMiddleware> logger)
{
_logger = logger;
_next = next;
}

public async Task Invoke(HttpContext context)
{
var requestId = context.Request.HttpContext.Items[ScriptConstants.AzureFunctionsRequestIdKey].ToString();
try
{
await _next.Invoke(context);

if (context.RequestAborted.IsCancellationRequested && !context.Response.HasStarted)
{
_logger.RequestAborted(requestId);
context.Response.StatusCode = StatusCodes.Status499ClientClosedRequest;
}
}
catch (Exception ex) when ((ex is OperationCanceledException || ex is IOException) && context.RequestAborted.IsCancellationRequested)
{
_logger.RequestAborted(requestId);

if (!context.Response.HasStarted)
{
context.Response.StatusCode = StatusCodes.Status499ClientClosedRequest;
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public static IApplicationBuilder UseWebJobsScriptHost(this IApplicationBuilder
builder.UseMiddleware<ClrOptimizationMiddleware>();
builder.UseMiddleware<HttpRequestBodySizeMiddleware>();
builder.UseMiddleware<SystemTraceMiddleware>();
builder.UseMiddleware<HandleCancellationMiddleware>();
builder.UseMiddleware<HostnameFixupMiddleware>();
if (environment.IsAnyLinuxConsumption() || environment.IsAnyKubernetesEnvironment())
{
Expand Down
Loading