-
Notifications
You must be signed in to change notification settings - Fork 197
Update ContextReference & DefaultFromBodyConversionFeature to remove invocation cancellation token usage #2894
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
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
e99f5ff
Don't use the invocation CT to cancel context ref tasks
liliankasem 70644de
Why does DefaultFromBodyConversionFeature use the invocation CT?
liliankasem 15bbc50
Remove _tokenRegistration
liliankasem ac93ee7
Remove use of an invocation's CT in DefaultFromBodyConversionFeature
liliankasem 11e651b
Address feedback
liliankasem a980bcd
Remove contextRef.Dispose();
liliankasem c018b0b
Remove _invocationCancellationToken
liliankasem e5647a4
Update release notes
liliankasem 2c27749
Add static
liliankasem e683e5c
Merge branch 'main' into liliankasem/invocation-ct
liliankasem d242ed6
Remove unnecessary using directive in DefaultHttpCoordinator.cs
liliankasem f871d5b
Add cancellation e2e tests
liliankasem 7106e88
Fix netfx scenario
liliankasem 43eb8af
Use Assert.Throw
liliankasem 1e257b4
cancel sooner
liliankasem 4abc037
Add local settings json
liliankasem 7a6a29e
Separate e2e tests
liliankasem 8178012
Fix flakey
liliankasem File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
78 changes: 78 additions & 0 deletions
78
test/E2ETests/E2EApps/E2EAspNetCoreApp/CancellationHttpFunctions.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the MIT License. See License.txt in the project root for license information. | ||
|
||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.Extensions.Logging; | ||
using Microsoft.AspNetCore.Http; | ||
using Microsoft.AspNetCore.Mvc; | ||
using System; | ||
|
||
namespace Microsoft.Azure.Functions.Worker.E2EApp | ||
{ | ||
public class CancellationHttpFunctions(ILogger<CancellationHttpFunctions> logger) | ||
{ | ||
private readonly ILogger<CancellationHttpFunctions> _logger = logger; | ||
|
||
[Function(nameof(HttpWithCancellationTokenNotUsed))] | ||
public async Task<IActionResult> HttpWithCancellationTokenNotUsed( | ||
[HttpTrigger(AuthorizationLevel.Anonymous, "get", "post")] HttpRequest req) | ||
{ | ||
_logger.LogInformation("HttpWithCancellationTokenNotUsed processed a request."); | ||
|
||
await SimulateWork(CancellationToken.None); | ||
|
||
return new OkObjectResult("Processing completed successfully."); | ||
} | ||
|
||
[Function(nameof(HttpWithCancellationTokenIgnored))] | ||
public async Task<IActionResult> HttpWithCancellationTokenIgnored( | ||
[HttpTrigger(AuthorizationLevel.Anonymous, "get", "post")] HttpRequest req, | ||
CancellationToken cancellationToken) | ||
{ | ||
_logger.LogInformation("HttpWithCancellationTokenIgnored processed a request."); | ||
|
||
await SimulateWork(cancellationToken); | ||
|
||
return new OkObjectResult("Processing completed successfully."); | ||
} | ||
|
||
[Function(nameof(HttpWithCancellationTokenHandled))] | ||
public async Task<IActionResult> HttpWithCancellationTokenHandled( | ||
[HttpTrigger(AuthorizationLevel.Anonymous, "get", "post", Route = null)] HttpRequest req, | ||
CancellationToken cancellationToken) | ||
{ | ||
_logger.LogInformation("HttpWithCancellationTokenHandled processed a request."); | ||
|
||
try | ||
{ | ||
await SimulateWork(cancellationToken); | ||
|
||
return new OkObjectResult("Processing completed successfully."); | ||
} | ||
catch (OperationCanceledException) | ||
{ | ||
_logger.LogWarning("Request was cancelled."); | ||
|
||
// Take precautions like noting how far along you are with processing the batch | ||
await Task.Delay(1000); | ||
|
||
return new ObjectResult(new { statusCode = StatusCodes.Status499ClientClosedRequest, message = "Request was cancelled." }); | ||
} | ||
} | ||
|
||
private async Task SimulateWork(CancellationToken cancellationToken) | ||
{ | ||
_logger.LogInformation("Starting work..."); | ||
|
||
for (int i = 0; i < 5; i++) | ||
{ | ||
// Simulate work | ||
await Task.Delay(1000, cancellationToken); | ||
_logger.LogWarning($"Work iteration {i + 1} completed."); | ||
} | ||
|
||
_logger.LogInformation("Work completed."); | ||
} | ||
} | ||
} |
32 changes: 32 additions & 0 deletions
32
test/E2ETests/E2EApps/E2EAspNetCoreApp/E2EAspNetCoreApp.csproj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<TargetFramework>net8.0</TargetFramework> | ||
<AzureFunctionsVersion>v4</AzureFunctionsVersion> | ||
<OutputType>Exe</OutputType> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
<Nullable>enable</Nullable> | ||
<AssemblyName>Microsoft.Azure.Functions.Worker.E2EAspNetCoreApp</AssemblyName> | ||
<RootNamespace>Microsoft.Azure.Functions.Worker.E2EAspNetCoreApp</RootNamespace> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\..\..\..\extensions\Worker.Extensions.Http\src\Worker.Extensions.Http.csproj" /> | ||
<ProjectReference Include="..\..\..\..\extensions\Worker.Extensions.Http.AspNetCore\src\Worker.Extensions.Http.AspNetCore.csproj" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<None Update="host.json"> | ||
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> | ||
</None> | ||
<None Update="local.settings.json"> | ||
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> | ||
<CopyToPublishDirectory>Never</CopyToPublishDirectory> | ||
</None> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<FrameworkReference Include="Microsoft.AspNetCore.App" /> | ||
<PackageReference Include="Microsoft.Azure.Functions.Worker.Sdk" Version="2.0.0" /> | ||
<PackageReference Condition="$(TestBuild) != 'true'" Include="Microsoft.Azure.Functions.Worker" Version="2.0.0" /> | ||
</ItemGroup> | ||
</Project> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<configuration> | ||
<packageSources> | ||
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" /> | ||
<add key="AzureFunctionsTempStaging" value="https://azfunc.pkgs.visualstudio.com/e6a70c92-4128-439f-8012-382fe78d6396/_packaging/AzureFunctionsTempStaging/nuget/v3/index.json" /> | ||
</packageSources> | ||
</configuration> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
using Microsoft.Azure.Functions.Worker; | ||
using Microsoft.Extensions.Hosting; | ||
using Microsoft.Extensions.DependencyInjection; | ||
|
||
var host = new HostBuilder() | ||
.ConfigureFunctionsWebApplication() | ||
.Build(); | ||
|
||
host.Run(); |
9 changes: 9 additions & 0 deletions
9
test/E2ETests/E2EApps/E2EAspNetCoreApp/Properties/launchSettings.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"profiles": { | ||
"dni_net8_cancellation": { | ||
"commandName": "Project", | ||
"commandLineArgs": "--port 7097", | ||
"launchBrowser": false | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{ | ||
"version": "2.0", | ||
"logging": { | ||
"applicationInsights": { | ||
"samplingSettings": { | ||
"isEnabled": true, | ||
"excludedTypes": "Request" | ||
}, | ||
"enableLiveMetricsFilters": true | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"IsEncrypted": false, | ||
"Values": { | ||
"AzureWebJobsStorage": "UseDevelopmentStorage=true", | ||
"FUNCTIONS_WORKER_RUNTIME": "dotnet-isolated" | ||
} | ||
} |
68 changes: 68 additions & 0 deletions
68
test/E2ETests/E2ETests/AspNetCore/CancellationEndToEndTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
// 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.Collections.Generic; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.Azure.Functions.Tests; | ||
using Microsoft.Azure.Functions.Tests.E2ETests; | ||
using Xunit; | ||
using Xunit.Abstractions; | ||
|
||
namespace Microsoft.Azure.Functions.Worker.E2ETests.AspNetCore | ||
{ | ||
public class CancellationEndToEndTests : IClassFixture<CancellationEndToEndTests.TestFixture> | ||
{ | ||
private readonly TestFixture _fixture; | ||
|
||
public CancellationEndToEndTests(TestFixture fixture, ITestOutputHelper testOutputHelper) | ||
{ | ||
_fixture = fixture; | ||
_fixture.TestLogs.UseTestLogger(testOutputHelper); | ||
} | ||
|
||
[IgnoreOnNetFxTestRunTheory] | ||
[InlineData("HttpWithCancellationTokenNotUsed", "Work completed.", "Succeeded")] | ||
[InlineData("HttpWithCancellationTokenIgnored", "TaskCanceledException: A task was canceled", "Failed")] | ||
[InlineData("HttpWithCancellationTokenHandled", "Request was cancelled.", "Succeeded")] | ||
public async Task HttpTriggerFunctions_WithCancellationToken_BehaveAsExpected(string functionName, string expectedMessage, string invocationResult) | ||
liliankasem marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
using var cts = new CancellationTokenSource(); | ||
var task = HttpHelpers.InvokeHttpTrigger(functionName, cancellationToken: cts.Token); | ||
|
||
// Make sure the function invocation started before we cancel | ||
IEnumerable<string> invocationStartLog = null; | ||
await TestUtility.RetryAsync(() => | ||
{ | ||
invocationStartLog = _fixture.TestLogs.CoreToolsLogs.Where(p => p.Contains($"Executing 'Functions.{functionName}'")); | ||
return Task.FromResult(invocationStartLog.Count() >= 1); | ||
}); | ||
|
||
// The task should be cancelled before it completes, mimicing a client closing the connection. | ||
// This should lead to the worker getting an InvocationCancel request from the functions host | ||
cts.Cancel(); | ||
await Assert.ThrowsAsync<TaskCanceledException>(async () => await task); | ||
|
||
IEnumerable<string> invocationEndLog = null; | ||
await TestUtility.RetryAsync(() => | ||
{ | ||
invocationEndLog = _fixture.TestLogs.CoreToolsLogs.Where(p => p.Contains($"Executed 'Functions.{functionName}'")); | ||
return Task.FromResult(invocationEndLog.Count() >= 1); | ||
}); | ||
|
||
Assert.Contains(_fixture.TestLogs.CoreToolsLogs, log => log.Contains(expectedMessage, StringComparison.OrdinalIgnoreCase)); | ||
|
||
// TODO: 2/3 of the test invocations will fail until the host with the ForwarderProxy fix is released - uncomment this line when the fix is released | ||
// Assert.Contains(_fixture.TestLogs.CoreToolsLogs, log => log.Contains($"'Functions.{functionName}' ({invocationResult}", StringComparison.OrdinalIgnoreCase)); | ||
} | ||
|
||
public class TestFixture : FunctionAppFixture | ||
{ | ||
public TestFixture(IMessageSink messageSink) : base(messageSink, Constants.TestAppNames.E2EAspNetCoreApp) | ||
{ | ||
} | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.