[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(XXS): 8156226: DiagnosticCommandImpl::invoke throws NoSuchMethodException even if the method
From:       Kirill Zhaldbybin <kirill.zhaldybin () oracle ! com>
Date:       2016-05-09 18:34:18
Message-ID: 5730D82A.1000708 () oracle ! com
[Download RAW message or body]

David,

Thank you for reviewing the fix!

On 05/09/2016 03:08 AM, David Holmes wrote:
> Hi Kirill,
>
> On 7/05/2016 3:39 AM, Kirill Zhaldybin wrote:
>> Alexander,
>>
>> Thank you for reviewing the fix.
>>
>> On 06.05.2016 20:24, Alexander Kulyakhtin wrote:
>>> Is there a specification of what shall be thrown, or the justification
>>> of the changes done?
>>> One can think that the currently thrown NoSuchMethodException is
>>> correct since　methods, which differ in signature only, are still
>>> different methods.
>> I spent some time trying to figure out why I got NoSuchMethodException
>> for "help" despite such method actually existed.
>>
>> If you think that to throw NoSuchMethodException is better for wrong
>> signature I could just change error message in this case.
>
> The code doesn't really seem to be checking any detail of the 
> "signature" as such, simply that the passed in "signature" array has a 
> non-null zeroeth element of type String[]. Passing something else 
> seems like a bad call to me and so IllegalArgumentException would be 
> appropriate I think.
>
> But the logic of that entire block is suspect to me as it does not 
> handle the case where isEmpty() is true, but we pass in non-null 
> params and signature anyway.
I tried to make this fix as small and low-risk as possible - as far as 
the repo closes on Wednesday
The change does not affect "command execution" - it only throws 
different exceptions.

Thank you.

Regards, Kirill
>
> Aside: I'm not even convinced that NoSuchMethodException is 
> appropriate for the case where "actionname" doesn't exist. But that is 
> a different issue.
>
> Thanks,
> David
>
>
>> Could you please let me know your opinion?
>>
>> Thank you.
>>
>> Regards, Kirill
>>
>>>
>>> Best regards,
>>> Alexander
>>>
>>>
>>> ----- Original Message -----
>>> From: kirill.zhaldybin@oracle.com
>>> To: serviceability-dev@openjdk.java.net
>>> Sent: Friday, May 6, 2016 8:11:08 PM GMT +03:00 Iraq
>>> Subject: RFR(XXS): 8156226: DiagnosticCommandImpl::invoke throws
>>> NoSuchMethodException even if the method actually exists but
>>> parameters are wrong
>>>
>>> Dear all,
>>>
>>> Could you please review this small fix for 8156226?
>>>
>>> A case when a method exists but parameters' signature is wrong now
>>> causes new ReflectionException(new IllegalArgumentException()) thrown
>>> instead of new ReflectionException(new NoSuchMethodException()).
>>>
>>> WebRev:
>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8156226/webrev.00/
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8156226
>>>
>>> Thank you.
>>>
>>> Regards, Kirill
>>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]