Revisit all exception usage in ClrMD before 2.0 release

[leculver]

This is a reminder to revisit all exception throws within ClrMD and see if there's a more appropriate exception/add more detailed message and data.

[ryanmolden]

Old issue, but I would add, see if you can replace exceptions with TryX type methods, normally throwing/catching exceptions isn't a huge problem except:

1) It makes debugging super annoying both because perf (debuggers cough VS cough can slow down considerably when handling exception notifications, even if you aren't breaking on FCE) and general noise (if you want to break on something like InvalidOperationException in your own code it can be annoying if the framework is throwing 800,000 of them (I know now VS allows you to suppress breaking by the module the exception came from, but none of this is 'free' (see previous perf complaint))

2) Reporting non-fatal things via exceptions is kind of against the idea of most exceptions (I know, horse, barn, .NET code does this egregiously)

3) It makes analyzing perf traces looking for 'interesting'/unexpected exceptions a lot harder when there are a slew of them from the framework.

[leculver]

@ryanmolden I'm onboard with less exceptions, but I need more concrete advice on where you are seeing them today.

My hope for exceptions are:

• Startup codepaths will throw. There's just too much that can go wrong.
• Change ClrObject/ClrException/ClrValueType/ClrArray to stop throwing in normal operation. Tracked with https://github.com/microsoft/clrmd/issues/617 (though the issue isn't very descriptive). There will be some exceptions, for example if you call ClrObject.AsArray and don't call IsArray first then you will get an exception. Maybe that should be named "ToArray" because "As" would imply null on failure...
• Most operations will validate parameters and throw as appropriate (IndexOutOfBounds, ArgumentNull) but those should be 100% avoidable if you follow the conventions.
• I plan to make QueryInterface deep in the implementation stop throwing/catching.

Aside from the things listed, is there something where you have to really worry about exceptions right now? It's entirely possible I just forgot about a piece of the library which throws often.

[ryanmolden]

Last I checked it was immense amounts from QI throwing.

I have this (rather old) comment in some code, not sure if it is still legit in 2.0, but possibly

            catch (Exception ex) when (ex.GetType() == typeof(System.Exception))
            {
                // Sadly CLRMD throws System.Exception at times, specifically here it is when the debug metadata doesn't have full info on the field we are requesting,
                // it can throw, we don't want our callers to necessarily have to deal with that so we swallow it here and return failure.
                //
                // TODO: File a bug for them to STOP throwing System.Exception, making me catch THAT specific exception makes me catch ALL exceptions or have a filter
                // like here to ensure that the REAL type of the Exception is System.Exception (not simply that it derives from it).

This is around accessing a field Offset. I think the problem is if any field in the list has an Unknown type or unknown size it can end up throwing, which is kind of annoying, I guess in the case of offset the only way to avoid that is say return a clearly invalid offset like -1, and that would need to be opt-in or a different API likely since old code that expected failure to == exeption wouldn't do well if no exception + returned FUBAR offset :P

EnumerateReferencesWithFields could throw an InvalidOperationException, but I believe you already fixed that one (would have to dig back through email/Teams log to check)

Asking for a field Size could throw an Exception (potentially covered by the same / similar fix for the Offset issue discussed above).

Getting fields for a type can throw IndexOutOfRangeException (this def still happens on 2.0, its in my debugger right now, I can get repros with 'the usual dump' if you want to look into this)

[leculver]

At this point I'm satisfied with where we are for 2.0. I will be happy to continue fixing any other exception issues since they do not change the public callable surface area of the library.

Last I checked it was immense amounts from QI throwing.

This is now fixed.