runtime: API Proposal: UnreachableException
Motivation
This exception is expected to never be thrown. Code coverage tools might choose to ignore non-coverage of the statement or branch that throws this exception. In fact, coverage of such a statement or branch is undesirable, as it would indicate that the code is making an invalid assumption.
Proposal
#nullable enable
using System.Runtime.Serialization;
namespace System
{
public sealed class UnreachableException : SystemException
{
public UnreachableException()
: base("The program executed an instruction that was thought to be unreachable.")
{
}
public UnreachableException(string? message)
: base(message)
{
}
public UnreachableException(string? message, Exception? innerException)
: base(message, innerException)
{
}
private UnreachableException(SerializationInfo info, StreamingContext context)
: base(info, context)
{
}
}
}
See Also
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 28
- Comments: 29 (25 by maintainers)
Commits related to this issue
- Add UnreachableException Fix #35324 — committed to i3arnon/runtime by i3arnon 2 years ago
- Add UnreachableException Fix #35324 — committed to i3arnon/runtime by i3arnon 2 years ago
One suggestion was
System.Diagnostics.CodeAnalysis
, but maybe that’s a bad fit becaust it contains the attributes for helping code analysis, this feels more like a plumbing type. Personally, I think I’m leaning towardsSystem
.What would happen if a single request in a web app encounters such an exception? Would thread or process be torn down? That’s unacceptable. The application must keep running and keep serving traffic.
Bugs happen all the time. Even state corrupting ones. We can’t tear down the process when that happens.
Web apps also rarely experience true state corruption because, usually, requests are isolated and the request state is cleanly thrown away at the end of request processing.
How would this interact with the proposed “never” return type (https://github.com/dotnet/csharplang/issues/538)? Is this API a point-in-time patch until that feature comes online?
Presumably code coverage analyzers could already look for methods marked with
[DoesNotReturn]
and exclude any code after such call sites from being included in the final metrics.It would be helpful to clarify this exception’s place in the ecosystem in a world where both of these capabilities exist.
Video
We seem to have hit the most agreeable (perhaps “least disagreeable”) position with System.Diagnostics, on the basis that it’s rather like an assert (
System.Diagnostics.Debug.Assert
); and based on @jkotas feedback, the exception isn’t really special… so: Exception
instead of: SystemException
.Personally I think this should not have any kind of uncatchable behavior. Consider a plugin system, like Roslyn analyzers: an
UnreachableException
thrown from a plugin means that the plugin has a bug, but that doesn’t necessarily mean the whole process (Visual Studio, or its child process) should crash. It should be up to the plugin runner to decide what happens.Also, there’s already a somewhat similar type,
SwitchExpressionException
, that’s thrown by aswitch
expression with unexpected input. Would it make sense to makeSwitchExpressionException
inherit fromUnreachableException
? Though this is probably not particularly valuable, since both of these exceptions shouldn’t be caught by name.Stack overflow is hard fail fast. We do not allow any handling of it in managed code. (.NET Framework allowed handling stackoverflow exceptions using SQL hosting interfaces. We do not have those in .NET Core.)
Thread abort was not a simple uncatchable exception. The thread abort switched the thread into a special aborting mode, it was about much more than just rethrowing exception at the end of catch block.
We do not have concept of uncatchable exceptions in the runtime and I do not think we should be introducing it for this one.
The only difference between uncatchable exceptions and
Environment.FailFast
is that uncatchable exception would allow custom logging. The process is going to be unconditionally terminated in both cases anyway. If we want to make this situation uncatchable, but still allow custom logging for it, I think we may want to rather look at adding logging hook toEnvironment.FailFast
.For reference: https://github.com/dotnet/csharplang/issues/538
I know this is a bit late, but I was surprised the discussion didn’t float the idea that this exception is quite similar (at least in my mind) in concept to
NotImplementedException
which lives in theSystem
namespace.There’s already an in-flight PR for it.
The PR observed that there were still some floating points. Marking as review-ready and blocking so we discuss them today to unblock the PR.
All exceptions have these constructors; it’s the standard pattern.
@GrabYourPitchforks Any other concerns or can this be put into the review queue?