That Shouldn't Happen – UnreachableException in .NET 7 | Hacker News Reader

That Shouldn't Happen – UnreachableException in .NET 7

ab.bot

84 pointsicey58 comments

The state machine example is a good one. It's a textbook case of unreachable code.

The null-checking example is not good. If the external API returns a null where you don't expect one, UnreachableException will be thrown. It doesn't matter if the documentation says it will never be null; clearly the code isn't unreachable. All you need to reach it is bad data! (InvalidDataException would be a better choice, though arguably not an ideal one.)

If you can write a unit test that makes your code throw UnreachableException, you almost certainly should be throwing a different exception.

Invalid date means you got a value that’s not a valid date which is different than falling into an impossible state.

Invalid Data, not date

I would guess they were spell-checker-corrected, and meant "data" both times: the sentence makes sense with either, though "data" is obviously what makes it make sense in this context :)

This was my initial thought as well, but from the text I gather there is a flow like this:

[Input Data, maybe null] -> Validate field is not null -> Call this method with the assertion.

This is a small bug-bear for me with nullable types and I wish there was a better way to do it, but many languages allow you to smart-cast away nulls, but only within the local scope. If you want to pass a struct-type around which has nullable fields, but you have already checked for non-null (like this one) you need to convert to a different struct-type, which doesn't have the nullability on its fields. I can't think of a good way round this - as you say with the unit test remark, there is nothing to stop another piece of code calling this method with nulls.

> If you want to pass a struct-type around which has nullable fields, but you have already checked for non-null (like this one) you need to convert to a different struct-type, which doesn't have the nullability on its fields.

Which is exactly what IMO the author should have done. It's actually a reasonable use-case for inheritance:

    #nullable enable

    record SlackEvent
        ( int EventId
        , string Content
        , string? TeamId
        );

    record TeamSlackEvent
        ( int EventId
        , string Content
        , string TeamId
        ) 
        : SlackEvent
            ( EventId : EventId
            , Content : Content
            , TeamId : TeamId
            );

> If you can write a unit test that makes your code throw UnreachableException, you almost certainly should be throwing a different exception.

ArgumentNullException, InvalidOperationException... There's plenty that have been around since the beginning.

What's more important, that I didn't see in the first two examples, are useful error messages.

`ArgumentNullException` was my first idea too. I would and did use it for this kind of purpose.

> Fortunately, .NET 7 adds a new type to help us out here: UnreachableException.

There was an UnreachableException before .NET 7 as well.

Look:

public class UnreachableException : Exception {}

Underrated comment. I’m all for precisely modeling domain and I/O errors. But for “this is a bug” type of exceptions, what good do all those tiny variations (and the resulting philosophical debates about when to use which one) do, if all that changes in practice is the text of log message? You’ll never catch that one specifically or treat it any differently than an ArgumentNullException — or any other of the zillion already existing ones.

When opening article, I was thinking: hey, maybe they introduced some compile-time checks for this special exception type? Like switching non-exhaustive list of enums and default:ing with UnreachableException... Errm, maybe not.

it helps significantly for debugging

> all that changes in practice is the text of log message

Just wanted to highlight a relevant tool i recently discovered: Sentry.

It's already become invaluable in my .net apps for tracking down tricky errors, especially in the new MAUI apps.

Caught one just yesterday where the app was trying to call an api with decimals serialized with a comma because it was on a dutch user's phone! I imagine it would've been quite the headache catching that without sentry. Integrates with most tech-stacks too, not just .net btw

Had to google Sentry, I've been using Raygun for ~10 years. These sort of tools are awesome. The weird errors that users seem to be able to produce but getting a full stack trace and being able to resolve them.

My previous job we used to send exceptions to email until we blew out the gmail account for the day (exceeded the receive amount) and boss switched over to raygun (~10 years ago), because of the global error handling we ended up fixing tons of errors we didn't even know our customers were experiencing. Product became more stable over time with the fixes.

In one company, the app error page would also be a "Submit Ticket" page where the poor human would be able to quickly explain what they were trying to do while I could gather all sorts of user-side information.

> with decimals serialized with a comma because it was on a dutch user's phone

If you're doing any kind of non-binary serialization/deserialization or file IO, it's always a good exercise to change the user and system regional settings when running tests.

Server runs de-DE, my dev machine is en-US. So many errors found with formatting/parsing because of it :D

As far as I'm aware these kinds of tools usually come with proprietary cloud services attached to them. Are there good local/FOSS versions?

Sentry is open https://github.com/getsentry/sentry

These are all the SDK's for instrumenting various languages. As far as I know you are not able to run a self hosted version of their server side software which aggregates all of the traces and provides all of the value.

Yes, you are. Sentry famously started in open source (as a Django plugin!), and its source code is still available today.

https://develop.sentry.dev/self-hosted/ https://github.com/getsentry/self-hosted

Latest release 3 days ago.

I wasn't aware of that at all,. Thank you!

For null checking I'd use the relatively new ArgumentNullException.ThrowIfNull, that automatically adds the parameter name in the exception and is a very compact and obvious way to check for null in your method parameters.

For enums I use ArgumentExceptions, something called this code with an invalid enum argument. I think there are some cases where Unreachable is a better error, but I'd probably be more inclined to use that for logic errors, where the code itself should not be reachable according to your understanding and reaching it would mean the code is wrong. The enums and null are cases where the arguments are unexpected, which to me is different.

I don’t think that’s a good name for this. Clearly, that code can be reached. From the name, I would expect ”throw new UnreachableException” to be something the compiler inserts to make it more robust in the sense that, if the compiler has some bug in its control flow analysis, it throws, rather than executes the wrong code.

Now, for a better name? Maybe ShouldNeverGetHereException, but that may be seen as too whimsical.

It's anagolous to `unreachable` in Rust. I think the author is defining it too broadly. It's for scenarios where it would be possible to statically guarantee that something is unreachable, but is not implemented in control flow analysis (e.g. it could be too computationally expensive, or it could be a structure invariant). It's useful for getting the compiler to shut up about a branch that you know can't be reached, e.g. you might not return a value from the `default` case in an exhaustive switch.

It doesn't seem like .Net is implementing it correctly either. It is supposed to be optimized away in release code, resulting in UB if the developer was wrong about the condition.

> It's anagolous to `unreachable` in Rust. [...] It doesn't seem like .Net is implementing it correctly either. It is supposed to be optimized away in release code, resulting in UB if the developer was wrong about the condition.

Rust actually has two things called "unreachable". There's the unreachable!() macro (https://doc.rust-lang.org/core/macro.unreachable.html), which is a short-hand for panic!(), and therefore is never UB even if somehow it's reached; and there's the unsafe unreachable_unchecked() compiler hint (https://doc.rust-lang.org/core/hint/fn.unreachable_unchecked...), which is optimized away in release code, and is always UB if somehow it's reached. Most of the time, you should prefer the safer unreachable()! macro; the unsafe hint is for when it makes a performance difference (and as with every use of unsafe in Rust, you really should carefully review the code to make sure it's really unreachable).

Maybe UnexpectedCodePathException

There is code I wrote that has `raise ImpossibleError("X should never happen")` or, simply, a `NotImplementedError("You shouldn't be here")`

> Maybe ShouldNeverGetHereException, but that may be seen as too whimsical.

In Delphi we have the EProgrammerNotFound exception[1]. I use it for code paths which should never be executed.

[1]: https://docwiki.embarcadero.com/Libraries/Alexandria/en/Syst...

I agree, `UnreachableException` is really poorly used here, and most of these are argument exceptions, and clearly are reachable. However, for other "unexpected behaviors", you should really throw an `InvalidOperationException` - you did something that is invalid. And even better if you sub-class Invalid with a custom exception that more clearly describes the unexpected behavior (also easier to search for that exception type in your log analysis tool).

[0] https://learn.microsoft.com/en-us/dotnet/api/system.invalido...

Ignoring the subject matter entirely, it's always fun to see some snippets of C# du jour and marvel at the number of new syntactical features added since my last encounter.

Yeah, it started like C++ for wannabee programmers, but during the last years it has being becoming a more and more useful tool for professional developers.

seems to me they are using unreachable to mean unsupported. Also they should put a message in for the first example which should highlight the int value as that is the very first thing you are going to want to know.

If TeamId is a non-nullable string then the construction of that object should fail when TeamId is null, not at the consumption site.

This is exactly the issue what Rust is solving. A struct construction completes or fails. There is no weird in between state.

That’s all well and good when you write both the consumer and producer. But how do you detect consumers which depend on some data integrity aspect you need to change for whatever reason?

The types or constructors ought change to match?

If TeamId goes from non-nullable to nullable, it goes from TeamId to Option<TeamId>, and consumers get a type error.

They could easily have declared TeamId as non-nullable in modern C# too.

The issue they had is:

(a) TeamId is legitimately nullable at creation, there are valid SlackEvents without a TeamId

(b) They wanted to assert "TeamId won't be null" from a certain point in the code path

(c) They didn't want to create a different data structure (TeamSlackEvent or something) to hold the definitely-has-a-TeamId events.

I strongly suspect (c) was a mistake and they were just too lazy to define a new (sub)-class or record.

I think I still prefer NotImplementedException. It also covers things that are logically reachable but not ready for use yet. The nuance is the string argument to the ctor.

It's useful to have a separate type since things like exception messages can get optimized out for size in some cases.

Different intention.

I do let my IDE register NotImplementedExceptions as TODOs, which is not really desired for things that should not be reachable.

It's not like you can't define your own exceptions in C#, so you always could do what the article is saying.

Even if exceptions might be handy, I wouldn't use exceptions for error handling for permance reasons. Instead I have another recommendation: never have void methods and use a Result type that packages the actual value along with an IsValid and Error property. So you will always return Result<T>.

That way, error handling is easy and doesn't cause much performance overhead.

I don't think you'd be hitting UnreachableException that much for that to be of performance concern. You actually halt the execution.

And what you are proposing, it's just a different way to handle errors. Maybe appropriate in critical path. However you still have exceptions from .NET framework. OutOfMemoryException, various IOExceptions etc.

> doesn't cause much performance overhead

Exceptions are slow when thrown.

If an error happens rarely, the corresponding exception is thrown rarely, so the performance impact is minimal.

OTOH, if an error can happen frequently, it is no longer "exceptional", and so is probably better handled without exceptions anyway.

C# needs closed enums

https://github.com/dotnet/csharplang/issues/3179

Off-topic slightly: does anything similar to what they are doing with that require extension method exist in Kotlin?

The expression part or the extension method part? (Obviously: No clue about Kotlin, just interested)

The extension part mainly, like the ability to add an extension method that can apply to 'everything' to an extent like the Require() here does

Is it more helpful if you will create a custom exception instead? E.g. class UnsupportedStateException : Exception

Generally, no. Only you will use it in your team.

UnreachableException has no special behaviour at all [0]. It's just a standardized 'tag' around regular Exception.

The only advantage of using a standard tag is that it will be recognized by other programmers, and maybe some supporting tooling might choose to give it special treatment.

E.g. a code analyzer could warn you if you raise a NotImplementedException (forgot to finish a feature?), but accept an UnreachableException as valid.

If you want to add a custom name (for easier grepping maybe?) or stuff some extra data in the exception, you can still inherit from UnreachableException.

[0] https://github.com/dotnet/runtime/pull/63922/files#diff-588c...

Is this like java.lang.UnsupportedOperationException?

No, that's System.NotSupportedException in .NET.

Which in turn is not for unreachable or even unexpected cases but expected cases you’re fully aware of, maybe even documented, but that you don’t support.

Yes, like Seek() on a NetworkStream.

In Java, usually an Error like AssertionError is used for those cases.

By example, for a switch on an enum, the compiler inserts a "throw new ...Error()" automatically when the default case is not specified.

Reply on news.ycombinator.com