[Fix] Elide Some Asyncs

[SimonCropp]

No description provided.

[iancooper]

Review: Elide Some Asyncs

Thanks for the PR. I've analyzed the changes in detail, particularly regarding whether eliding async/await alters behavior.

Summary of Changes

The PR makes two types of changes:

1. BrighterAsyncContext.Run optimization (Safe)

// Before
BrighterAsyncContext.Run(async () => await SomeMethodAsync())
// After  
BrighterAsyncContext.Run(() => SomeMethodAsync())

This is a pure optimization - it eliminates an unnecessary async state machine. The lambda now returns a Task directly instead of creating a state machine that just awaits and returns. No behavioral difference.

2. Async method elision (Mostly safe, with minor caveats)

// Before
public async Task<T> MethodAsync(...) 
{
    return await SomeOtherMethodAsync(...);
}
// After
public Task<T> MethodAsync(...)
{
    return SomeOtherMethodAsync(...);
}

Behavioral Differences to Consider

Exception Timing: When async/await is elided, any synchronous exceptions thrown before the first await in the called method will propagate synchronously rather than being wrapped in the returned Task. For example, argument validation exceptions would throw immediately at call-site rather than when the Task is awaited.

In practice, this rarely matters because:

• Well-designed async methods capture exceptions in the Task
• Callers typically await immediately anyway
• The methods being changed here don't have complex pre-await logic

Specific Cases Worth Noting:

1. Proactor.RequeueMessage and RejectMessage - These have logging/counter operations before the return. If those throw, it's now synchronous. This is acceptable since those operations are unlikely to throw.

2. FallbackPolicyHandlerRequestHandlerAsync.HandleAsync - Now throws ArgumentException synchronously if _exceptionHandlerFunc is null. This is actually preferable for configuration errors - fail fast rather than deferred.

3. Test changes - The async () => await to () => changes in Assert.ThrowsAsync calls are purely stylistic. xUnit handles both forms identically.

Methods That Are Safe

The following patterns are completely safe with no behavioral difference:

• Methods that only return another async call (no code before return)
• Lambdas passed to BrighterAsyncContext.Run
• Lambdas passed to Assert.ThrowsAsync

Verdict

✅ Approve - The changes are valid optimizations. The minor exception-timing differences are acceptable and in some cases (like FallbackPolicyHandlerRequestHandlerAsync) actually improve the behavior by failing fast on configuration errors.

The elision pattern is well-documented best practice when:

• There's no using statement that needs to stay in scope
• There's no code after the awaited call
• The method doesn't need to modify the result

All the changes in this PR meet these criteria.

[iancooper]

Thanks @SimonCropp

[codescene-delta-analysis]

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.