Actor proxy for methods declared not as Completion<X> should fail faster

[bwehrle]

Currently when a proxy is generated for an actor interface there are two cases:

• Returns Completes
• Returns instance of T

In the first case everything works correctly. In the second case the code generated will always return null, which is a trap for the unwary developer. My suggestions would be to disallow this case- all return types must of type Completion (could be Completion).

Here is an example of the generated code by which I observed this (working out of the branch in my fork using 0.7.9)

 public io.vlingo.examples.ecommerce.model.OrderInfo doesSyncOperatorWork() {
    if (!actor.isStopped()) {
      final java.util.function.Consumer<Order> consumer = (actor) -> actor.doesSyncOperatorWork();
      if (mailbox.isPreallocated()) { mailbox.send(actor, Order.class, consumer, null, doesSyncOperatorWorkRepresentation5); }
      else { mailbox.send(new LocalMessage<Order>(actor, Order.class, consumer, doesSyncOperatorWorkRepresentation5)); }
    } else {
      actor.deadLetters().failedDelivery(new DeadLetter(actor, doesSyncOperatorWorkRepresentation5));
    }
    return null; //<---  Always return null
  }

[VaughnVernon]

@bwehrle I understand. The problem with changing it is, in a few special cases we do allow immediate actor state accessors, such as with Stoppable::isStopped(). See Stoppable__Proxy.

[bwehrle]

@VaughnVernon I think there are two case here -one is the hand generated proxy (wherein all is fair because its your proxy), and the other is the dynamic proxy, which can't know these details. Is that correct? If so, I would say for the latter case the proxy creation should fail faster (because otherwise it will simply fail, but further along in the execution of the code).

[kmruiz]

@VaughnVernon If there is a list of special protocols, like Stoppable? Maybe we can apply the suggestions that @bwehrle is sharing with us only for any protocol that is not special.

I personally agree that the generator should be smarter and be able to produce more reasonable errors.

[VaughnVernon]

@kmruiz I do support "special" cases but those are purposely hand coded. Under normal circumstances we should never allow the Actor messaging to be bypassed by direct method invocations.

[kmruiz]

It means that the proxy generator shouldn't consider any special change, so it will be as simple as only allowing methods that return void or Completes<T> and fail in any other case. Is that right?

[VaughnVernon]

Yes, ok, I agree with fail fast on the proxy generation if there is a non-void return type that is not a Completes<T>. Sorry, I thought you were suggesting to support special cases with direct access into the Actor instance.

[kmruiz]

This commit: https://github.com/vlingo/vlingo-actors/commit/dced01a692a9e4ddaefed249d3f22f3f49e06a99

should improve the error handling and throw an exception with a proper message:

io.vlingo.actors.InvalidProtocolException: For protocol io.vlingo.actors.ProtocolWithPrimitive
    In method `public boolean shouldNotCompile()`: 
        method return type should be either `void` or `Completes<T>`. The found return type is `boolean`. Consider wrapping it in `Completes<T>`, like `Completes<Boolean>`.
    In method `public java.util.List<java.lang.Boolean> shouldNotCompileEither()`: 
        method return type should be either `void` or `Completes<T>`. The found return type is `java.util.List<java.lang.Boolean>`. Consider wrapping it in `Completes<T>`, like `Completes<java.util.List<java.lang.Boolean>>`.

[bwehrle]

Cool! This will really help prevent some frustration for new developers. For the documentation, it would be good to call out theses cases; the generated code proxy and its limitations, and the hand-coded case, etc.

Regards, Brian

On Sat, Feb 23, 2019, 17:47 Kevin Mas Ruiz notifications@github.com wrote:

This commit: dced01a https://github.com/vlingo/vlingo-actors/commit/dced01a692a9e4ddaefed249d3f22f3f49e06a99

should improve the error handling and throw an exception with a proper message:

io.vlingo.actors.InvalidProtocolException: For protocol io.vlingo.actors.ProtocolWithPrimitive In method public boolean shouldNotCompile(): method return type should be either void or Completes<T>. The found return type is boolean. Consider wrapping it in Completes<T>, like Completes<Boolean>. In method public java.util.List<java.lang.Boolean> shouldNotCompileEither(): method return type should be either void or Completes<T>. The found return type is java.util.List<java.lang.Boolean>. Consider wrapping it in Completes<T>, like Completes<java.util.List<java.lang.Boolean>>.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vlingo/vlingo-actors/issues/39#issuecomment-466668049, or mute the thread https://github.com/notifications/unsubscribe-auth/AOjeNAASni9mfb1aN8ALlVBdnfqHJRjpks5vQXCVgaJpZM4aUFBh .

[VaughnVernon]

@bwehrle @kmruiz Can you please confirm that this can be closed? Thanks!

[kmruiz]

It should be on the latest version of vlingo 0.8.2 and snapshots :D. So if @bwehrle can take a look and confirm that it works for him, it can be closed.

[VaughnVernon]

I have confirmed fixes as have others. Although @bwehrle has not confirmed, I am closing this.

[bwehrle]

My apologies, gentlemen, for not confirming this fix sooner- I just wrote a small test with this and it works great and the error message is concise and helpful. Thanks much!

Caused by: java.lang.IllegalArgumentException: Actor proxy io.vlingo.examples.ecommerce.model.TestProto not created because: For protocol io.vlingo.examples.ecommerce.model.TestProto
    In method `public java.lang.Object doSomething()`: 
        method return type should be either `void` or `Completes<T>`. The found return type is `java.lang.Object`. Consider wrapping it in `Completes<T>`, like `Completes<java.lang.Object>`.