jersey
  1. jersey
  2. JERSEY-1738

NPE with AsyncInvoker.get() when InvocationCallback implementation is itself generic

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0-m12
    • Fix Version/s: 2.0-rc1, 2.0
    • Component/s: core
    • Labels:
      None

      Description

      The NPE originates from org.glassfish.jersey.internal.util.ReflectionHelper#getParameterizedTypeArguments, line 768 (ClassTypePair is null) when calling get(InvocationCallback) on an AsyncInvoker.

      I have a concrete class implementing InvocationCallback, that is itself generic. When trying to infer the parameterized type arguments, the actual type argument is a TypeVariable. Both p.concreteClass and p.declaringClass are my generic, concrete class.

      public class MyCallback<V> implements InvocationCallback<V> {
        @Override
        public void completed(V response) {
          // whatever
        }
        @Override
        public void rejected(Throwable throwable) {
          // whatever
        }
      }
      
        MyCallback<Foo> callback = new MyCallback<Foo>();
        asyncInvoker.get(callback);
      

      While I understand why it needs the generic type (there should probably be a get(InvocationCallback<T>, Class<T>) overload) and why it fails, I believe it should better handle the error.

      FYI, my generic concrete implementation is a bridge between JAX-RS 2 and a promise-like API (also think Guava's ListenableFuture). The class is both an InvocationCallback<V> for its JAX-RS facet, and a Promise<V>. The way I create an instance of my InvocationCallbackPromise doesn't currently allow subclassing (final class with create() static factory) that would engrave the type arguments in the type; so I'll have to change my API to allow subclassing, similar to GenericType; not ideal in my book, but I can live with that.

        Activity

        Hide
        t.broyer added a comment -

        Note: this behavior seems obvious in retrospect, but it's not clearly documented (apologies if I missed it). So it's half a spec bug, half a request to better handle the error.

        BTW, please move priority from Major down to Minor. I had selected Major before actually fully debugging the issue, before I realized the failure was expected. Sorry about that.

        Show
        t.broyer added a comment - Note: this behavior seems obvious in retrospect, but it's not clearly documented (apologies if I missed it). So it's half a spec bug, half a request to better handle the error. BTW, please move priority from Major down to Minor. I had selected Major before actually fully debugging the issue, before I realized the failure was expected. Sorry about that.
        Hide
        Marek Potociar added a comment -

        The exception thrown has changed to IllegalArgumentException with a better description of the root cause.

        Show
        Marek Potociar added a comment - The exception thrown has changed to IllegalArgumentException with a better description of the root cause.

          People

          • Assignee:
            Marek Potociar
            Reporter:
            t.broyer
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 3 hours
              3h
              Remaining:
              Remaining Estimate - 0 minutes
              0m
              Logged:
              Time Spent - 1 hour Time Not Required
              1h