Skip to content

Conversation

joostas
Copy link

@joostas joostas commented Nov 8, 2024

Description

Enhances OperationContext creation.

Currently OperationContext is created using constructor. This prevents executing async code. But during construction of OperationContext there is a need to execute an asynchronous operation to read request content

await message.Content.ReadAsByteArrayAsync();

This requires to fallback to sync-over-async anti-pattern in OperationContext constructor:

SetContentAsync(message).GetAwaiter().GetResult();

This change introduces static asynchronous factory methods to create OperationContext:

public static async Task<OperationContext> CreateAsync(HttpRequestMessage message);
public static async Task<OperationContext> CreateAsync(HttpRequestMessage message, IHttpCustomHeaderCollection headers);

They are used through the codebase to create OperationContext instances:

 _context = await OperationContext.CreateAsync(request, _headers);

Additional notes:

Needed to add this exception to the rule to be able to build the solution.

<WarningsNotAsErrors>NU1903</WarningsNotAsErrors> <!-- To be able to restore Microsoft.Extensions.Caching.Memory -->

Didn't want to update the version of this dependency, because I think this is out of the current improvement scope.

Related issues

Addresses issue #140.

Testing

Added WebPipeline test that emulates long-running request message content reading (that allows to kick-in scenario described in issue #140)

Reviewer Checklist

  • Update the title of the PR to be succinct and less than 50 characters
  • Tag the PR with the type of update: Bug, New-Sample, Enhancement, or New-Feature
  • CI is green before merge

@joostas joostas requested a review from a team as a code owner November 8, 2024 13:04
@joostas
Copy link
Author

joostas commented Nov 8, 2024

@microsoft-github-policy-service agree

@joostas joostas force-pushed the operation-context-async-factory branch from 0f4e370 to a6dbd7d Compare May 4, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant