Skip to content

Commit a05ad56

Browse files
satvusatvu
authored
Stop retrying http requests when script invocation result is failed (#11210)
1 parent f7ad5d6 commit a05ad56

File tree

8 files changed

+378
-52
lines changed

8 files changed

+378
-52
lines changed

‎src/WebJobs.Script/Http/DefaultHttpProxyService.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ internal class DefaultHttpProxyService : IHttpProxyService, IDisposable
2121
private readonly HttpMessageInvoker _messageInvoker;
2222
private readonly ForwarderRequestConfig _forwarderRequestConfig;
2323
private readonly ILogger<DefaultHttpProxyService> _logger;
24+
private readonly HttpTransformer _httpTransformer = ScriptInvocationRequestTransformer.Instance;
2425

2526
public DefaultHttpProxyService(IHttpForwarder httpForwarder, ILogger<DefaultHttpProxyService> logger)
2627
{
@@ -98,8 +99,12 @@ public void StartForwarding(ScriptInvocationContext context, Uri httpUri)
9899
// add invocation id as correlation id, override existing header if present
99100
httpRequest.Headers[ScriptConstants.HttpProxyCorrelationHeader] = context.ExecutionContext.InvocationId.ToString();
100101

101-
var forwardingTask = _httpForwarder.SendAsync(httpContext, httpUri.ToString(), _messageInvoker, _forwarderRequestConfig).AsTask();
102+
// Add the script invocation context for later observation of the ScriptInvocationResult task.
103+
// This helps track failures/cancellations that should halt retrying the http request.
104+
httpContext.Items[ScriptConstants.HttpProxyScriptInvocationContext] = context;
105+
106+
var forwardingTask = _httpForwarder.SendAsync(httpContext, httpUri.ToString(), _messageInvoker, _forwarderRequestConfig, _httpTransformer).AsTask();
102107
context.Properties[ScriptConstants.HttpProxyTask] = forwardingTask;
103108
}
104109
}
105-
}
110+
}

‎src/WebJobs.Script/Http/RetryProxyHandler.cs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33

44
using System;
55
using System.Net.Http;
6+
using System.Runtime.ExceptionServices;
67
using System.Threading;
78
using System.Threading.Tasks;
9+
using Microsoft.Azure.WebJobs.Script.Description;
810
using Microsoft.Extensions.Logging;
911

1012
namespace Microsoft.Azure.WebJobs.Script.Http
@@ -30,11 +32,22 @@ public RetryProxyHandler(HttpMessageHandler innerHandler, ILogger logger)
3032

3133
protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
3234
{
35+
TaskCompletionSource<ScriptInvocationResult> resultSource = null;
36+
if (request.Options.TryGetValue(ScriptConstants.HttpProxyScriptInvocationContext, out ScriptInvocationContext scriptInvocationContext))
37+
{
38+
resultSource = scriptInvocationContext.ResultSource;
39+
}
40+
3341
var currentDelay = InitialDelay;
3442
for (int attemptCount = 1; attemptCount <= MaxRetries; attemptCount++)
3543
{
3644
try
3745
{
46+
if (resultSource is not null && (resultSource.Task.IsFaulted || resultSource.Task.IsCanceled))
47+
{
48+
ExceptionDispatchInfo.Capture(resultSource.Task.Exception).Throw();
49+
}
50+
3851
return await base.SendAsync(request, cancellationToken);
3952
}
4053
catch (TaskCanceledException) when (cancellationToken.IsCancellationRequested)
@@ -44,6 +57,12 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
4457
}
4558
catch (HttpRequestException) when (attemptCount < MaxRetries)
4659
{
60+
if (resultSource is not null && (resultSource.Task.IsFaulted || resultSource.Task.IsCanceled))
61+
{
62+
_logger.LogWarning("HTTP request will not be retried. The associated function invocation has failed.");
63+
throw;
64+
}
65+
4766
_logger.LogWarning("Failed to proxy request to the worker. Retrying in {delay}ms. Attempt {attemptCount} of {maxRetries}.",
4867
currentDelay, attemptCount, MaxRetries);
4968

@@ -55,7 +74,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
5574
{
5675
var message = attemptCount == MaxRetries
5776
? "Reached the maximum retry count for worker request proxying. Error: {exception}"
58-
: $"Unsupported exception type in {nameof(RetryProxyHandler)}. Request will not be retried. Exception: {{exception}}";
77+
: $"HTTP request will not be retried. Exception in {nameof(RetryProxyHandler)}: {{exception}}.";
5978

6079
_logger.LogWarning(message, ex);
6180

‎src/WebJobs.Script/Http/ScriptInvocationRequestTransformer.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System.Collections.Generic;
5+
using System.Net.Http;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Microsoft.AspNetCore.Http;
9+
using Microsoft.Azure.WebJobs.Script.Description;
10+
using Yarp.ReverseProxy.Forwarder;
11+
12+
namespace Microsoft.Azure.WebJobs.Script.Http
13+
{
14+
internal sealed class ScriptInvocationRequestTransformer : HttpTransformer
15+
{
16+
public static readonly ScriptInvocationRequestTransformer Instance = new ScriptInvocationRequestTransformer();
17+
18+
private ScriptInvocationRequestTransformer() { }
19+
20+
public override async ValueTask TransformRequestAsync(HttpContext httpContext, HttpRequestMessage proxyRequest, string destinationPrefix, CancellationToken cancellationToken)
21+
{
22+
// this preserves previous behavior (which called the default transformer) - base method is also called inside of here
23+
await HttpTransformer.Default.TransformRequestAsync(httpContext, proxyRequest, destinationPrefix, cancellationToken);
24+
25+
if (httpContext.Items.TryGetValue(ScriptConstants.HttpProxyScriptInvocationContext, out object result)
26+
&& result is ScriptInvocationContext scriptContext)
27+
{
28+
proxyRequest.Options.TryAdd(ScriptConstants.HttpProxyScriptInvocationContext, scriptContext);
29+
}
30+
}
31+
}
32+
}

‎src/WebJobs.Script/ScriptConstants.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ public static class ScriptConstants
262262
public static readonly string HttpProxyingEnabled = "HttpProxyingEnabled";
263263
public static readonly string HttpProxyCorrelationHeader = "x-ms-invocation-id";
264264
public static readonly string HttpProxyTask = "HttpProxyTask";
265+
public static readonly string HttpProxyScriptInvocationContext = "HttpProxyScriptInvocationContext";
265266

266267
public static readonly string OperationNameKey = "OperationName";
267268

‎test/WebJobs.Script.Tests/HttpProxyService/DefaultHttpProxyServiceTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
using Xunit;
1313
using Yarp.ReverseProxy.Forwarder;
1414

15-
namespace Microsoft.Azure.WebJobs.Script.Tests
15+
namespace Microsoft.Azure.WebJobs.Script.Tests.Http
1616
{
1717
public class DefaultHttpProxyServiceTests
1818
{

‎test/WebJobs.Script.Tests/HttpProxyService/RetryProxyHandlerTests.cs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Net.Http;
7+
using System.Reflection;
8+
using System.Threading;
9+
using System.Threading.Tasks;
10+
using Microsoft.Azure.WebJobs.Script.Description;
11+
using Microsoft.Azure.WebJobs.Script.Http;
12+
using Microsoft.Extensions.Logging.Abstractions;
13+
using Xunit;
14+
15+
namespace Microsoft.Azure.WebJobs.Script.Tests.Http
16+
{
17+
public class RetryProxyHandlerTests
18+
{
19+
[Fact]
20+
public async Task SendAsync_RetriesToMax()
21+
{
22+
var inner = new TestHandler();
23+
var handler = new RetryProxyHandler(inner, NullLogger.Instance);
24+
var request = new HttpRequestMessage();
25+
26+
var response = typeof(RetryProxyHandler)!
27+
.GetMethod("SendAsync", BindingFlags.NonPublic | BindingFlags.Instance)!
28+
.Invoke(handler, new object[] { request, CancellationToken.None })
29+
as Task<HttpResponseMessage>;
30+
31+
var result = await response.ContinueWith(t => t);
32+
33+
Assert.True(result.IsFaulted);
34+
Assert.True(result.Exception.InnerException is HttpRequestException);
35+
Assert.Equal(RetryProxyHandler.MaxRetries, inner.Attempts);
36+
}
37+
38+
[Fact]
39+
public async Task SendAsync_StopsRetriesWhenScriptInvocationResultIsFaulted()
40+
{
41+
var inner = new TestHandler();
42+
var handler = new RetryProxyHandler(inner, NullLogger.Instance);
43+
var request = new HttpRequestMessage();
44+
45+
// Create a faulted TaskCompletionSource for ScriptInvocationResult
46+
var faultedResultSource = new TaskCompletionSource<ScriptInvocationResult>();
47+
var invocationException = new InvalidOperationException("Function invocation failed");
48+
faultedResultSource.SetException(invocationException);
49+
50+
// Create ScriptInvocationContext with faulted result source
51+
var scriptInvocationContext = new ScriptInvocationContext
52+
{
53+
ExecutionContext = new ExecutionContext
54+
{
55+
InvocationId = Guid.NewGuid()
56+
},
57+
ResultSource = faultedResultSource
58+
};
59+
60+
// Add the context to the request options
61+
request.Options.TryAdd(ScriptConstants.HttpProxyScriptInvocationContext, scriptInvocationContext);
62+
63+
var response = typeof(RetryProxyHandler)!
64+
.GetMethod("SendAsync", BindingFlags.NonPublic | BindingFlags.Instance)!
65+
.Invoke(handler, new object[] { request, CancellationToken.None })
66+
as Task<HttpResponseMessage>;
67+
68+
var result = await response.ContinueWith(t => t);
69+
70+
// Verify that the task is faulted due to the ScriptInvocationResult being faulted
71+
Assert.True(result.IsFaulted);
72+
73+
// Verify that no retries were attempted since the result source was already faulted
74+
Assert.Equal(0, inner.Attempts);
75+
}
76+
77+
[Fact]
78+
public async Task SendAsync_TaskCanceledException_ThrowsOperationCanceledException_WhenCancellationRequested()
79+
{
80+
var cts = new CancellationTokenSource();
81+
cts.Cancel();
82+
83+
var inner = new TaskCanceledTestHandler();
84+
var handler = new RetryProxyHandler(inner, NullLogger.Instance);
85+
var request = new HttpRequestMessage();
86+
87+
var sendAsync = typeof(RetryProxyHandler)!
88+
.GetMethod("SendAsync", BindingFlags.NonPublic | BindingFlags.Instance)!;
89+
90+
var task = (Task<HttpResponseMessage>)sendAsync.Invoke(handler, new object[] { request, cts.Token });
91+
92+
var ex = await Assert.ThrowsAsync<OperationCanceledException>(() => task);
93+
Assert.True(cts.Token.IsCancellationRequested);
94+
Assert.Equal(cts.Token, ex.CancellationToken);
95+
}
96+
97+
private class TaskCanceledTestHandler : HttpMessageHandler
98+
{
99+
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
100+
{
101+
throw new TaskCanceledException();
102+
}
103+
}
104+
105+
private class TestHandler : HttpMessageHandler
106+
{
107+
public int Attempts { get; set; }
108+
109+
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
110+
CancellationToken cancellationToken)
111+
{
112+
Attempts++;
113+
114+
throw new HttpRequestException();
115+
}
116+
}
117+
}
118+
}

0 commit comments

Comments
 (0)