"Do not catch general exception types" should recognize common exception marshaling patterns

[jnm2]

Analyzer

Diagnostic ID: CA1031

Describe the improvement

Certain well-known patterns should not need suppression because it is clear that the exception is being necessarily marshaled for handling somewhere else. For example:

/// <summary>
/// Synchronously invokes the factory with the specified argument, or waits asynchronously for the return value
/// or exception if another thread has already started to invoke the factory. The factory will never be invoked
/// more than once.
/// </summary>
public Task<TResult> GetResultAsync(T1 arg)
{
    if (Interlocked.Exchange(ref resultFactory, null) is { } claimedResultFactory)
    {
        try
        {
            taskCompletionSource.SetResult(claimedResultFactory.Invoke(arg));
        }
#pragma warning disable CA1031 // Exception must be marshaled to all waiting callers through the task completion source.
        catch (Exception ex)
#pragma warning restore CA1031
        {
            taskCompletionSource.SetException(ex);
        }
    }

    return taskCompletionSource.Task;
}

Describe suggestions on how to achieve the rule

If the catch block unconditionally passes ex to TaskCompletionSource<T>.SetException or a similar type of method, no diagnostic should be reported. SetException would not be called unless it meant the exception was being marshaled to the appropriate audience.

The same thing is true for ExceptionDispatchInfo.Capture. If ex is passed to this method, you can be sure that reporting a diagnostic on the catch (Exception ex) cannot be helpful.

[jinek]

That is why CA1031 exists. Why can we assume that we can recover from any exception by marshaling?

[jnm2]

I'm asking for CA1031 to notice when the exception is marshaled in a way that is equivalent to rethrowing.

[jinek]

Sorry, I should clarify my doubts from start.

My point is exactly that marshalling is not equivalent to rethrowing.

At the moment ca1031 correctly reminds that it is impossible to recover the state if that state is not known. Some exceptions are bound to the stack and make sense only within it.

For example, HttpRequestException (supposed to go to HttpMessageHandler upper the stack), ThreadAbortException (which in code above would go to TaskScheduler.TaskUnobservedException, which in many cases is recognized as unhandled exception). User code wrapped to that factory will not stop the attached debugger by throwing an exception etc etc

Sure, current implementation of Task/async does not satisfy CA1031 in many cases, still this should not make developers to think that it is anyhow safe/similar to rethrow. With current implementation of TaskScheduler.PublishUnobservedTaskException that exception can be ignored by unkwnowledge.