runtime: Exception message for AmbiguousMatchException should include the name

Fundamentally, this is an indication of a bad argument, without telling you what the bad value is, which usually demands that you fire up the debugger to figure it out. It would be helpful to know what the ambiguous name actually was.

System.Reflection.AmbiguousMatchException : Ambiguous match found.
Stack Trace:
    at System.RuntimeType.GetMethodImpl(String name, BindingFlags bindingAttr, Binder binder, CallingConventions callConv, Type[] types, ParameterModifier[] modifiers)
    at System.Type.GetMethod(String name)

About this issue

Original URL
State: closed
Created 8 years ago
Reactions: 6
Comments: 23 (22 by maintainers)

Most upvoted comments

A similar change to ArgumentException thrown from Dictionary<K, V>.Add() was proposed in https://github.com/dotnet/corefx/issues/1187 and merged in https://github.com/dotnet/coreclr/pull/1452. Is this any different?

Personally, I think clearer exceptions messages are worth some code bloat.

svick on Aug 22, 2016

re parallel strings, we do this for IO, eg.,

  <data name="UnauthorizedAccess_IODenied_NoPathName" xml:space="preserve">
    <value>Access to the path is denied.</value>
  </data>
  <data name="UnauthorizedAccess_IODenied_Path" xml:space="preserve">
    <value>Access to the path '{0}' is denied.</value>
  </data>

and of course try our best to only use the one with the more info, but they are both there.

Edit: BTW, we try to put single quotes around replacement markers unless they’re numbers. Not start with a replacement marker. and end with a period.

danmoseley on Apr 7, 2023

I think an area owner like @steveharter should advise.

danmoseley on Apr 7, 2023

I don’t own this area but I think you’d be welcome to take it @IDisposable. do you want me to assign to you?

danmoseley on Apr 7, 2023

My 2c. Many times issues first show up in logs. A debugger may not be available - nor dump file - nor even a repro. If by changing some localized places to add some value that we have easily available into that message, we can make some of these diagnosable without further work. It seems a place where relatively small work in a library is highly leveraged. This is why we added keys to key not found exceptions, paths to file not found exceptions. Of course there are cases where it is not worth it but there are still plenty of cases we we do not have a good reason to not do it.

GetMethodImpl is just one of the places where the exception gets thrown. In order to get a consistent behavior,

I don’t think consistency for its own sake is important in these messages. In making changes of this sort in the past, I just skipped cases where it is difficult/impossible to supply the value. Eg., some of the IO exception paths. Most of our exception types were originally designed to have an optional “parameter” – they already come in helpful and less helpful forms. We just prefer the more helpful form if we can.

danmoseley on Apr 8, 2020

Let’s tentatively try to make this a little better in 5.0 for debuggability. Though it’s low-priority compared to other reflection work so it may still slip below the cut line.

Edit: We shouldn’t strive for 100% coverage. Just target scenarios where we don’t have to contort the code to make the exception message a little better.

GrabYourPitchforks on Apr 8, 2020