5 Replies Latest reply on Jun 12, 2008 11:38 AM by Ales Justin

    Installing instance of wrong class gives weird error

    Carlo de Wolf Master

      Installing an instance into MC while specifying another class in the bean meta data yields:

      java.lang.IllegalArgumentException: Wrong arguments. start for target org.jboss.ejb3.stateful.StatefulContainer@1f8f8c8 expected=[] actual=[]

      instead of:
      java.lang.IllegalArgumentException: object is not an instance of declaring class

      (The last one could also use some decoration.)

      ReflectionUtils should actually have the method as context parameter and not just the name. It should then check whether the declaring class is assignable from the target class.

        • 1. Re: Installing instance of wrong class gives weird error
          Ales Justin Master

           

          "wolfc" wrote:
          It should then check whether the declaring class is assignable from the target class.

          I'll add this check to ReflectionUtils:
           protected static void checkMember(Object target, Member member, String memberType) throws Throwable
           {
           if (member == null)
           throw new IllegalArgumentException("Null " + memberType);
          
           if (target != null)
           {
           Class<?> declaringClass = member.getDeclaringClass();
           if (declaringClass.isInstance(target) == false)
           throw new IllegalArgumentException("Target (" + Strings.defaultToString(target) + ") doesn't match " + memberType + " 's (" + member + ") declaring class.");
           }
          
           }
          


          • 2. Re: Installing instance of wrong class gives weird error
            Adrian Brock Master

            That's not correct. It's now doing the check twice.
            Once when you do it and once inside the JDK reflection api.

            That's horribly inefficient, since these reflection calls are used everywhere.

            The whole point of ReflectionUtils is give more reasonable error messages
            after reflection calls fail, because the JDK is usually hopeless.

            e.g. you'll get something like

            java.lang.IllegalArgumentException: wrong type java.lang.String
            


            Without it telling you what invocation is being attempted or which
            parameter is wrong.

            What's required is to fix "handleErrors" to also check whether IllegalArgumentException
            is due to the wrong target object being passed for non-status invocations
            and not just assuming that it is due to an invalid parameter being passed.

             public static Throwable handleErrors(String context, Object target,
            + Class<?> clazz,
            + Object instance,
             Class<?>[] parameters, Object[] arguments, Throwable t) throws Throwable
             {
             if (t instanceof IllegalArgumentException)
             {
             if (target == null)
             throw new IllegalArgumentException("Null target for " + context);
            + if (clazz != null && instance != null && clazz.isAssignableFrom(instance) == false)
            + throw new IllegalArgumentException("Wrong target. " + instance.getClass().getName() + " is not a " + clazz.getName());
             ArrayList<String> expected = new ArrayList<String>();
             if (parameters != null)
             {
             for (int i = 0; i < parameters.length; ++i)
             expected.add(parameters.getName());
             }
             ArrayList<String> actual = new ArrayList<String>();
             if (arguments != null)
             {
             for (int i = 0; i < arguments.length; ++i)
             {
             if (arguments == null)
             actual.add(null);
             else
             actual.add(arguments.getClass().getName());
             }
             }
             throw new IllegalArgumentException("Wrong arguments. " + context + " for target " + target + " expected=" + expected + " actual=" + actual);
             }
             else if (t instanceof InvocationTargetException)
             {
             throw ((InvocationTargetException) t).getTargetException();
             }
             throw t;
             }
            


            and

             public static Object invoke(Method method, Object target, Object[] arguments) throws Throwable
             {
             if (method == null)
             throw new IllegalArgumentException("Null method");
             try
             {
             return method.invoke(target, arguments);
             }
             catch (Throwable t)
             {
             throw handleErrors(method.getName(), Strings.defaultToString(target),
            + method.getDeclaringClass(), target,
             method.getParameterTypes(), arguments, t);
             }
             }
            


            An alternative fix would be to produce one generic error message for both
            that says something

            Invalid invocation TargetClass.methodName(ActualParamType1, ActualParamType2, ...)
            for MethodDeclaringClass.methodName(MethodParam1, MethodParam2, ...)

            • 3. Re: Installing instance of wrong class gives weird error
              Ales Justin Master

              With the current methods on ReflectUtils this only comes in play with method and field invocation, so I only added that code there - and not to handleError method.

              • 4. Re: Installing instance of wrong class gives weird error
                Adrian Brock Master

                The point is that it shouldn't do anything at all unless there is an error.

                You effectively have:

                public void doSomething(X parameter)
                {
                 check(parameter);
                 doIt(parameter);
                }
                
                private void doIt(parameter)
                {
                 check(parameter);
                 ...
                }
                


                The correct code - since we don't implement doIt() and it gives bad error messages should be:

                public void doSomething(X parameter)
                {
                 try
                 {
                 doIt(parameter);
                 }
                 catch (Exception e)
                 {
                 throw fixErrorMessage(e);
                 }
                }
                


                • 5. Re: Installing instance of wrong class gives weird error
                  Ales Justin Master

                   

                  "adrian@jboss.org" wrote:
                  The point is that it shouldn't do anything at all unless there is an error.

                  That's what I do.
                  "that code" in my last post is meant your error handling code. ;-)