dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.12k stars 4.7k forks source link

[Proposal] Automatically re-throw exceptions caught as System.Exception, SystemException and ApplicationException #45256

Open jinek opened 3 years ago

jinek commented 3 years ago

Goal

Primary goal is the possibility to develop and run programs satisfying fundamental code-analysis quality rule CA1031 (I'm offering to define terms CA1031-compliance, CA1031-compliant, CA1031-compatibility, CA1031-policy etc).

I am proposing to implement two independent features at runtime and/or compilation levels.

Briefly on CA1031

The idea is expressed by Exceptions and Exception Handling (C# Programming Guide)

Do not catch an exception unless you can handle it and leave the application in a known state. If you catch System.Exception, re-throw it using the throw keyword at the end of the catch block.

For people who want to only start digging in to the topic here are some resources: 1) CLR Inside Out - Handling Corrupted State Exceptions 1) Stackoverflow related thread 1) StackExchange related thread 1) CLR via C# by Jeffrey Richter 1) Problem definition is placed below in Problem definition and more background section

Compilation level

CA1031 is supposed to be followed in most cases, but c# and other languages offer opposite by default.
We could reverse the default behavior of the catch by introducing new keyword swallow:

try
{          
}
catch(Exception e) swallow (IOException,SecurityException,WebException)
{
    Debug.WriteLine(e.GetType()); //can be any type, for example NullReferenceException
    // Following is added by compiler automatically: if (e is not (IOException or SecurityException or WebException)) throw; 
}

This feature does not guarantee satisfaction of CA1031, however it reduces the codebase for those who follow CA1031 and helps to start following for those who do not.

To fallback to old behavior LegacyCatchBehaviorAttribute attribute can be applied to method/class/assembly/build.props.
Besides, it can be applied by default only to libraries, but not to executables.

Runtime level

We have already similar solutions in dotnet for ThreadAbortException, StackoverflowException etc.
Thus, this part is about to extend that special behavior to other scenarios (not only to required by the runtime itself).
In order to allow the old behavior this proposal also assumes several runtime modes.

Suggested changes

1) Add runtime modes CA1031Compliant (NotCompliant=default, Compliant, Force) 1) Add assembly attribute (+ compile option, project property etc) CA1031Compliant (true/false=default) 1) Compilation of compliant assembly must inject re-throwing code in such a way that Compliant assembly in NotCompliant runtime mode behaves same way as in Compliant runtime mode. 1) Add special rules of runtime Compliant and Force modes:

1) Auto-rethrow exceptions from `catch{}`, `catch(Exception){}`, `catch(SystemException){}` and `catch(ApplicationException){}` similar to current implementation of **ThreadAbortException**
1) Add `Exception.NoRethrow<TException>()` which instructs runtime not to rethrow in current catch block (attempt to use Exception or SystemException or if actual type is not assignable to provided one throws CS1031CompliancyException)
1) Attempt to call NotCompliant assembly from Compliant one in Compliant runtime mode throws an exception

1) Define compiler error “Project Xxx can not compile as CA1031-compliant because it references non-compliant assembly Yyy.dll. Either remove the reference or remove CA1031Compliant attribute/option.

Probably, this makes sense to discuss same time: 1) Possibility to catch multiple exceptions by one catch with catch(Exception1,Exception2,Exception3 (BaseException)ex){ } 1) Make constructor of System.Exception protected (same for SystemException and ApplicationException) 1) Remove special behavior of ThreadAbortException StackOverflow and others in Compliant and Force runtime modes. Deprecate ResetAbort as NoRethrow does the same (also explicit catch of ThreadAbortException should do the same as well). Make StackOverflowException internal etc 1) AppDomain.UnhandledException should not rethrow exceptions explicitly in Compliant and Force modes, it should be allowed to catch exceptions using Exception.NoRethrow<TException>() mentioned above 1) Introduce AsyncException for explicit passing the state in async/await scenarios. 1) New operators async throw exception; and async throw; for easy instantiating the AsyncException

Problem definition and more background

Exceptions in dotnet are surely important and popular way to pass the state among the "programs" in the call stack. It allows to significantly reduce the codebase and to greatly improve the performance. Same time, we receive several important requirements where one of them is the codebase to remain generally transparent for the exceptions.
This requirement satisfaction is left on developers and very often does not happen due to various reasons.

For example, as exceptions in most cases are associated with the fault state, we do not consider other (non-fault) states expressed by exceptions as well.
Additionally, very often we do not consider faults which are not even related to the context of our code.

In the following example we are assuming that any exception is the result of database connectivity error.

    private bool IsDatabaseAccessible()
    {
        try
        {
            _dbConnection.ExecuteProbeRequest();
            return true;
        }
        catch
        {
            return false;
        }
    }

Same time runtime throws NullReferenceException when _dbConnection is null while the host expects to receive it for the error report. It is wrongly recognized as a connectivity issue and there will be no any error report for us.
Another example: user requests to throw OperationAbortException which is assumed to hit the catch block of JobManager class upper in the stack. Lets assume that JobManager is responsible to free resources for other jobs.
While the end-user just experiences difficulties to cancel jobs from time to time, this case eventually becomes the divination process for developers.

More on the problem

Other possible implementations

With this proposal I would also consider other options. I am sure that the community has plenty of them.

As an example, we could extend HandleProcessCorruptedStateExceptionsAttribute technique to make general catch to work only for exceptions marked by same attribute as our method.
Another idea could be to analyse the stack trace and determine how much the exception is "native" to current or to calling method.
Relativity and/or reference orders of assemblies could also be considered.

kevingosse commented 3 years ago

Is your swallow keyword any different from exception filters? You can already write this in C# 9:

try
{          
}
catch(Exception e) when (e is not (IOException or SecurityException or WebException))
{
    Debug.WriteLine(e.GetType()); //can be any type, for example NullReferenceException
}
jinek commented 3 years ago

Is your swallow keyword any different from exception filters? You can already write this in C# 9:

Very different, they can go together:

try
{          
}
catch(Exception e) when (e is not (IOException or SecurityException or WebException)) swallow (WebException)
{
    Debug.WriteLine(e.GetType());
    // Following is added by compiler automatically: if (e is not (WebException)) throw;
}

Lets say, filters work opposite: when filters not defined - everything is caught. When swallows is not defined - everything is rethrown.

when - means what to catch swallow - means what not to rethrow

kevingosse commented 3 years ago

when - means what to catch swallow - means what not to rethrow

Got it. I missed the distinction when I first read it.

DemiMarie commented 3 years ago

What if a program really does want to catch System.Exception or even System.Object? I can think of many cases for this:

jinek commented 3 years ago

What if a program really does want to catch System.Exception or even System.Object? I can think of many cases for this:

Thank you for nice question. You have made me understanding that my initial description is not easy while being short. Both Compliant and NonCompliant modes allow to do that, because entry point assembly can be NonCompliant in both modes (but in force mode it will be forced to be compliant).

The difference is that in NotCompliant runtime mode you can call:
NotCompliant entrypoint --------> compliant library ------------> not compliant library
But in Compliant runtime mode last call is not possible (while first is):
NotCompliant entrypoint --------> compliant library ------------> not compliant library (NotCompliantCallException)

So, it's possible to have any catches in any call unless it is called from assembly which is claimed to be compliant. Obviously you are free to compile your entrypoint as not compliant, while having compliant sattelite assemblies