NullReferenceException - Completely devoid of details

[vongillern]

I don't know if this necessarily the right place for this, but the payoff is big enough to warrant a possible failed attempt.

NullReferenceExceptions are the most common exceptions your average developer is going to encounter, and yet the only information that is included in the exception is the message that something was null and you may or may not get a line number in the stacktrace.

Take the following example:

_orderService.CreateOrder(items.ToList(), customer.CustomerId);

If I get a NullReferenceException on this line, there are three different objects that could have been null and caused the error (_orderService, items and customer). Knowing that it was specifically customer that was null would be incredibly helpful. Extending this concept further, having a small dump of all locally scoped variables when the exception was thrown would be incredibly useful, but I'll go with baby steps and just settle for knowing which variable/expression was the root cause of the null reference exception.

[peteraritchie]

It's kind of tricky because not all variable names are available when the exception is thrown. In your example it could be that only _orderService is available by name because it's a member variable (it seems). If items and customers are locals; they're names won't be known when the exception is thrown (unless the PDB is there--but if you have that you can debug it). I can't say for sure, but only providing the names some of the time might have been the impetus for not provide names at all.

[vongillern]

Agreed. I'm not sure it'd be "easy", but well worth the effort. If the name was not available, including its type in the error message would make it unambiguous 95% of the time. (i.e. NullReferenceException - expression of type 'SuperOrderService' evaluated to null).

My guess though, is that if the PDB is available, it would know the character ranges of the null expression. If i turn on "break on thrown exceptions" in the exceptions dialog box, it always breaks me in at the right point and I can inspect all of my variables. I just want a dump so I don't have to be actively debugging.

[svick]

@vongillern I think you can easily make that even better by including the name of the invoked method. Something like (mostly copying the existing message):

NullReferenceException: Object reference not set to an instance of an object when calling 'SuperOrderService.CreateOrder'.

This shouldn't be that hard to do, since the callvirt instruction that causes the vast majority of NREs knows which method it was.

[peteraritchie]

@svick that can be problematic too because the IL is really a representation of something like this:

var list = items.ToList();
var customerId = customer.CustomerId;
_orderService.CreateOrder(list, customerId);

And that's if the compiler(s) decided to put the retrieval of the param values next to the method call.

I expect there'd be a bit of work to associate those variables to a callvirt several instructions ahead of where the null value is actually found. And if that value got cached/reused, it would make it a bit more complex. I think "easily" is very optimistic. If were easy, I'm sure someone would have done it before now.

[svick]

@peteraritchie Yeah, I understand that associating callvirt with a variable may not be easy, but that's not what I'm talking about. What I meant is that the message would contain the type (as @vongillern suggested) and method that caused the NRE.

So, in the example, the message wouldn't tell you that _orderService is null, but it would tell you that the NRE happened when calling SuperOrderService.CreateOrder(), which should be sufficient to debug the issue.

Or maybe I misunderstood what you're saying?

[Alexx999]

I believe that NPE can be debugged in quite straightforward manner using crashdumps and WinDBG right now.
Adding some extra data is not that common for low-level exceptions, and NPEs are not the worst case (my personal favorite is "FileNotFoundException" when some assembly fails to resolve some dependency - you'll newer know which one actually failed).
Plus, it will surely hurt performance - exceptions are slow as it is, and NPEs are often caught.

[svick]

I believe that NPE can be debugged in quite straightforward manner using crashdumps and WinDBG right now.

I don't think that anything that involves crashdumps or WinDBG can be called straightforward. Especially if you consider that NRE is an exception that I think beginners often encounter.

NPEs are often caught

I hope that you are wrong about this. NRE pretty much always indicates a bug and catching it just hides that bug.

[stimms]

I certainly agree with @svick on this. One of the primary goals of .net should be to be as productive of an environment as possible. We should attempt to minimize the scenarios in which a developer needs to attach a debugger or examine a crash dump as they trend to be very time consuming operations.

With hundreds of thousands of developers encountering NPEs it doesn't take long for the time savings to really add up. I'm in favor of any action that gives us better error messages.

[jkotas]

Consider voting for http://visualstudio.uservoice.com/forums/121579-visual-studio/suggestions/2371587-better-nullpointerexception-error-message

[peteraritchie]

I'm not saying don't do it or that there's no value to it, just that it's not going to be that easy. As soon as you provide * some * detail, you have to provide detail in all circumstances or there will be lots of bugs logged

[Alexx999]

@svick well, I probably just got used to it. Anyway, I surely agree that exception messages are often bad.

[yaakov-h]

I agree with this. I've sometimes had to resort to checking the IL offset of the exception location and compare it with a disassembled assembly just to figure out exactly which reference was null.

The more information about an error, the better.

[akoeplinger]

+1, anything that can be done to add more details to this exception should be investigated.

@jkotas Thanks for the link, but that UserVoice is already 4 years old and has gotten a bunch of votes without an official statement from the team. Did you investigate a solution already? Did you run into problems or other concerns? Would love to learn more details :)

[jkotas]

The project management team looks at top uservoice requests regularly and works on getting them addressed. It does not matter that the UserVoice is 4 years old - it just means that it did not gather enough votes to be in the top list yet.

For example, UserVoice votes were one of the reasons for adding SIMD support to CLR - http://visualstudio.uservoice.com/forums/121579-visual-studio/suggestions/2212443-c-and-simd

[peteraritchie]

@jkotas is there someone on the team(s) that have looked at anything like this comment on this thread? Having some detail on what has been researched so far in terms of what would need to change in the code would give the community something to hit the ground running with for a potential PR.

[vongillern]

+1 @peteraritchie, a comment from the team would be helpful. I spent a little time looking at it and the project is obviously so massive, I had a hard time knowing where I could even start.

[jkotas]

@CarolEidt would you like to comment on this?

[peteraritchie]

@vongillern Right, it may be easy for the person on the CLR team that has spent the most time in that code; but that doesn't mean that person has the time to perform the change. (if that were the case, this may have been done already). For the community to do it, however acceptable that is to do, is likely not a quick task.

[CarolEidt]

@jkotas although I could imagine that the JIT could participate in a solution (through improved debug/unwind info if there are imprecisions), the main work would lie in getting System.Environment.GetStackTrace to report column numbers (which I think would give the desired granularity). @vongillern - System.Environment.GetStackTrace is in clr\src\mscorlib\src\System\Exception.cs. That might be a good place to start, though debug info (beyond the IL offsets that the JIT reports) is not really my area of expertise.

[peteraritchie]

👍

[mikedn]

The column number can be obtained by passing the exception object to the stack trace class, that's trivial. But there's no useful column number that you can get from a PDB, the sequence points generated by the C# (and likely any other) compiler usually track entire statements. One would need to first make the compiler emit sequence points for sub-expressions and that's problematic partly because the debug information would become larger, partly because sub-expressions can overlap and inevitably leads to overlapping sequence points. This may end up requiring changes to the debug format.

The JIT compiler too seems to track IL offsets at statement level (or an approximation of a statement).

To me this approach sounds like a non-starter. A lot of work for questionable benefit. And it's likely that it will only work properly with debug binaries.

Another approach might be to have the runtime figure out what was variable was stored in the base/index register of the memory operand that caused the access violation. The debugger can usually map a variable to a register to display its value so maybe doing the opposite wouldn't be too much trouble. But then again, this will probably work correctly only with debug binaries.

[jkotas]

Correct, the JIT and PDB do not have enough precision in debug info today to reliably identify the source of NullReferenceException. For example, the following C# statement:

        foo(a.ToString(), b.ToString(), c.ToString());

has just one sequence point, and so it is not possible to identify from just the NullReferenceException location and debug info which one of a, b or c was null. If you would like to see the good error message even with JIT optimizations on, it is even harder because of the debug info tracking is not guaranteed to be preserved by the JIT optimizations today.

I think that the first step to attack this problem would be to write a design proposal about possible approaches. The ones identified by @mikedn would be a good start. Focus on:

• The rough sketch of extra information that would flow between the different components in the system
• The user experience - how good and reliable would be the exception messages
• The estimated overhead in disk footprint, RAM and CPU cycles

I know it is a lot of work...

[EmJayGee]

Wouldn't this feature overall be better cast as a language/compiler feature
rather than of the runtime? The end goal is easily mapping back to the
source; debug info is designed to assist that but if we want things like
the identifier/expression text, wouldn't this best be handled by the
compiler?

I'm assuming that this kind of information being available only in debug
builds with symbols available to the runtime will be a non-starter.

Mike

On Wed, Feb 4, 2015 at 2:31 PM, Jan Kotas notifications@github.com wrote:

Correct, the JIT and PDB do not have enough precision in debug info today
to reliably identify the source of NullReferenceException. For example, the
following C# statement:

    foo(a.ToString(), b.ToString(), c.ToString());

has just one sequence point, and so it is not possible to identify from
just the NullReferenceException location and debug info which one of a, b
or c was null. If you would like to see the good error message even with
JIT optimizations on, it is even harder because of the debug info tracking
is not guaranteed to be preserved by the JIT optimizations today.

I think that the first step to attack this problem would be to write a
design proposal about possible approaches. The ones identified by @mikedn
https://github.com/mikedn would be a good start. Focus on:

• The rough sketch of extra information that would flow between the
  different components in the system
• The user experience - how good and reliable would be the exception
  messages
• The estimated overhead in disk footprint, RAM and CPU cycles

I know it is a lot of work...

—
Reply to this email directly or view it on GitHub
https://github.com/dotnet/coreclr/issues/25#issuecomment-72954338.

[peteraritchie]

@EmJayGee Starting to sound a lot like contracts, or the very least code weaving. I think if were approached form the compiler-side it may never get done. Keep in mind there's 2-3 compilers in play for any piece of code: C#/VB, JIT 32-bit, and JIT 64-bit.

[OtherCrashOverride]

Exceptions should be exceptional. If you think something may be null, you should check that condition in code to ensure proper operation. I think this is more an issue of coding practice than a feature for the runtime. The Common Language Runtime is, as the name implies, common among many languages so it would likely require strategies for what makes sense for each language to produce a more useful message targeted at the developer using that language.

Therefore, I suggest the proper place for this is the IDE where both source code and language are known. NullReferenceException should be a rare occurrence if good coding practices are observed.

(For an example, see F#'s use of null versus C#)

[rafabertholdo]

In my opinion, null.something should return null instead of raising an exception.
http://visualstudio.uservoice.com/forums/121579-visual-studio/suggestions/5728581-add-and-assembly-attribute-for-the-compiler-to-byp

Fortunately, C# 6.0 brings an operator to make it happen, but I think the code will be dirty. This should be a compiler attribute.

[peteraritchie]

@rafabertholdo Wouldn't that just defer a null exception to higher-up? That seems like it would make the root cause of a null reference exception even harder to find.

[yaakov-h]

Propagating null instead of erroring out (non-explicitly) is a bad idea, IMO. The collective experience of almost every single Objective-C developer stands against that behaviour.

[kangaroo]

Issue ping. Is there a concrete action here?

[jkotas]

A number of issues in this repo track hard problems with unclear solutions. This is one of them. Our intent is to keep them open to facilitate discussion about solutions. I have created "hard problem" label to tag them.

cc @richlander

[dsplaisted]

It looks like VS 2017 will show the information you want when targeting .NET Framework 4.6.2 or higher: https://blogs.msdn.microsoft.com/devops/2016/03/31/using-the-new-exception-helper-in-visual-studio-15-preview/

I'm not sure whether it's enabled in .NET Core

[alexsorokoletov]

This feature is indeed available in VS 2017, article says it is not available for .NET Core/UWP, only available for .NET 4.6.2.

Would be really great to have that not in the VS but in .NET Core/Mono so that we can have that information in the crash reports rather than when debugging in VS.

[inlineHamed]

Is this available in .Net Core 3.0 ?

[terrajobst]

Is this available in .Net Core 3.0 ?

The debugger experience is:

But the exception message is unchanged.

[strich]

Is there a plan to also update the exception message? Surely that is really the important part of this?

[soundarmoorthy]

I am not part of Microsoft / .NET team, and i wonder if it's even possible to do this consistently. Even in the case when the compiler emit enough information about identifiers (all language compilers) and the runtime consume it, the way the .NET project gets compiled won't be consistent given the fact that any .NET project that i can think of reference binaries from community / NuGet. In that case you will have part of the code reporting identifier names and the other part that doesn't. Also what happens to the generated types (for example IEnumerable) The runtime can figure out and report that IEnumerable.Current is null, but what is null is the underlying object in the container, which still doesn't give the answer. You walk the stack and figure out the underlying object and fix it, which is the case even without the information.

The way i think of this is trying to infer context from identifiers than the methods. Identifiers tell you what is null, but often you need to figure out why it is null because the programmer didn't anticipate the object to be null, and he has to walk the stack back to figure out the issue.

[EgorBo]

Related: https://openjdk.java.net/jeps/358 JEP 358: Helpful NullPointerExceptions

[swythan]

@jkotas Is any type information available at the point the exception is generated?

Even if the variable name is unavailable, there are a number of scenarios where knowing the expected type of the null reference would be helpful (even the compile-time type) .

A (very) contrived example:

this.authorisedUsers
    .FirstOrDefault() 
    .Manager
    .ContactDetails
    .Address
    .Postcode
    .ToUpper();

If the NRE told you the type was string, then you'd know the problem was Postcode. If it told you that it was Person then it's ambiguous, but it's at least narrowed down the possibilities.

Obviously it wouldn't help with something like

foo(a.ToString(), b.ToString(), c.ToString());

[jkotas]

@jkotas Is any type information available at the point the exception is generated?

This information is not available today. Adding it would be same problem as #3858 (comment)

[madhub]

@jkotas Is any type information available at the point the exception is generated?

This information is not available today. Adding it would be same problem as #3858 (comment)

Any exploration of how Java implemented this feature in Java 14

[michael-hawker]

I think this and FileNotFoundException not saying which file wasn't found (and the path where it was looking) are two of the most frustrating things to debug.

[InCerryGit]

Has there been any progress on this topic? I think it would be very helpful to introduce more detailed exceptions like Java does. For example code like this.

String s = null;
String s1 = s.toLowerCase();

Java will output a very helpful exception message.

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "String.toLowerCase()" because "s" is null
	at org.jdk17.App.main(App.java:14)

The exact method and reason for the exception are clear at a glance. If there are multiple methods and variables in a line of code, you can quickly locate the problem.

[madhub]

Has there been any progress on this topic? I think it would be very helpful to introduce more detailed exceptions like Java does. For example code like this.

String s = null;
String s1 = s.toLowerCase();

Java will output a very helpful exception message.

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "String.toLowerCase()" because "s" is null
	at org.jdk17.App.main(App.java:14)

The exact method and reason for the exception are clear at a glance. If there are multiple methods and variables in a line of code, you can quickly locate the problem.

This feature is present since Java 14 , its JEP 358: Helpful NullPointerExceptions. Very useful feature to find exact object throwing Null reference. Not sure why this feature is not a priority , it greatly helps production code debugging.

[OneB1t]

yes this will be very helpful to have +1

[pedoc]

just ping