Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-4675

Throw AopInvocationException on advice returning null for primitive type

    Details

    • Last commented by a User:
      false

      Description

      Could we please throw an exception in a proxy if the return value is null and should be a primitive type?

      It is extremely hard to debug at present if you happen to intercept a method that returns a primitive type, and the proxy then returns null. The stack trace has no useful line numbers in it (since it comes from the $Proxy) and there is no way to know what you have done wrong. Wouldn't it be worth the cost of a check for null if the type is primitive (e.g. in JdkDynamicAopProxy after the call to invocation.proceed())? Probably it would be no more expensive than the existing special case there already for detecting "return this".

        Activity

        Hide
        jamestastic James Earl Douglas added a comment -

        I tried adding the following snippet to JdkDynamicAopProxy between the try and finally:

        } catch (NullPointerException nullPointerException) {

        Class<?> returnType = invocation.getMethod().getReturnType();
        if (null == retVal && returnType.isPrimitive())

        { NullPrimitiveException nullPrimitiveException = new NullPrimitiveException(); nullPrimitiveException.setType(returnType); throw nullPrimitiveException; }

        else

        { throw nullPointerException; }

        The problem I ran into is that you can't throw any Throwable that is not declared in the original method. I tested this by running a test getInt() method that returned a null Integer, and since that method did not declare any Throwables, when the code above tried to throw a NullPrimitiveException, the JDK threw a UndeclaredThrowableException, per the following link:

        http://java.sun.com/j2se/1.5.0/docs/api/java/lang/reflect/UndeclaredThrowableException.html

        If there is a way around this, I would love to learn more about it.

        Show
        jamestastic James Earl Douglas added a comment - I tried adding the following snippet to JdkDynamicAopProxy between the try and finally: } catch (NullPointerException nullPointerException) { Class<?> returnType = invocation.getMethod().getReturnType(); if (null == retVal && returnType.isPrimitive()) { NullPrimitiveException nullPrimitiveException = new NullPrimitiveException(); nullPrimitiveException.setType(returnType); throw nullPrimitiveException; } else { throw nullPointerException; } The problem I ran into is that you can't throw any Throwable that is not declared in the original method. I tested this by running a test getInt() method that returned a null Integer, and since that method did not declare any Throwables, when the code above tried to throw a NullPrimitiveException, the JDK threw a UndeclaredThrowableException, per the following link: http://java.sun.com/j2se/1.5.0/docs/api/java/lang/reflect/UndeclaredThrowableException.html If there is a way around this, I would love to learn more about it.
        Hide
        david_syer Dave Syer added a comment -

        If NullPrimitiveException is unchecked (extends RuntimeException) it should be fine, or am I missing something?

        Show
        david_syer Dave Syer added a comment - If NullPrimitiveException is unchecked (extends RuntimeException) it should be fine, or am I missing something?
        Hide
        jamestastic James Earl Douglas added a comment -

        That sounds feasible. I'll give it a shot tonight.

        Show
        jamestastic James Earl Douglas added a comment - That sounds feasible. I'll give it a shot tonight.
        Hide
        jamestastic James Earl Douglas added a comment -

        That seems to have solved it. I have attached some sample code in SPR-4675-src.zip which causes a null to be returned as an int, and a NullPrimitiveException is successfully thrown.

        Show
        jamestastic James Earl Douglas added a comment - That seems to have solved it. I have attached some sample code in SPR-4675 -src.zip which causes a null to be returned as an int, and a NullPrimitiveException is successfully thrown.
        Hide
        rstoya05-aop Rossen Stoyanchev added a comment -

        This issue has been resolved through a selective bulk update, as part of a larger effort to better manage unresolved issues. To qualify for the update, the issue was either created before Spring 3.0 or affects a version older than Spring 3.0 and is not a bug.

        There is a good chance the request was made obsolete, or at least partly outdated, by changes in later versions of Spring including deprecations. It is also possible it didn't get enough traction or we didn't have enough time to address it. One way or another, we didn't get to it.

        If you believe the issue, or some aspects of it, are still relevant and worth pursuing at present you may re-open this issue or create a new one with a more up-to-date description.

        We thank you for your contributions and encourage you to become familiar with the current process of managing Spring Framework JIRA issues that has been in use for over a year.

        Show
        rstoya05-aop Rossen Stoyanchev added a comment - This issue has been resolved through a selective bulk update, as part of a larger effort to better manage unresolved issues. To qualify for the update, the issue was either created before Spring 3.0 or affects a version older than Spring 3.0 and is not a bug. There is a good chance the request was made obsolete, or at least partly outdated, by changes in later versions of Spring including deprecations. It is also possible it didn't get enough traction or we didn't have enough time to address it. One way or another, we didn't get to it. If you believe the issue, or some aspects of it, are still relevant and worth pursuing at present you may re-open this issue or create a new one with a more up-to-date description. We thank you for your contributions and encourage you to become familiar with the current process of managing Spring Framework JIRA issues that has been in use for over a year.
        Hide
        david_syer Dave Syer added a comment -

        This seems like a valid request still and there is a patch attached. Re-opened.

        Show
        david_syer Dave Syer added a comment - This seems like a valid request still and there is a patch attached. Re-opened.
        Hide
        cbeams Chris Beams added a comment -

        Thanks, Dave. Would you be interested in submitting that patch as a pull request? Would help things along.

        Show
        cbeams Chris Beams added a comment - Thanks, Dave. Would you be interested in submitting that patch as a pull request? Would help things along.
        Hide
        david_syer Dave Syer added a comment -

        https://github.com/SpringSource/spring-framework/pull/99. It's not the patch that was supplied (which actually had missed the point I think).

        Show
        david_syer Dave Syer added a comment - https://github.com/SpringSource/spring-framework/pull/99 . It's not the patch that was supplied (which actually had missed the point I think).

          People

          • Assignee:
            pwebb Phil Webb
            Reporter:
            david_syer Dave Syer
            Last updater:
            Chris Beams
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since last comment:
              5 years, 23 weeks, 1 day ago