Improve error for Action's method without proper return type
I've wrongly declared an Action like this:
public class TestingAction extends Action {
@GetMapping("/test")
public String test() {
System.out.println("Metadata: " + actionContext().metadata());
return "OK";
}
}
When booting up, discovery was fine, then it blew up on the first request with:
13:43:30.494 ERROR k.javasdk.impl.action.ActionsImpl - Failure during handling of command com.example.shoppingcart.TestingAction.Test
java.lang.ClassCastException: class java.lang.String cannot be cast to class kalix.javasdk.action.Action$Effect ...
It would be better to check the return type of the action's method and at least break on discovery.
An alternative would be to consider String to be the same as Effect<String>.
This will reduce quite a lot the boilerplate for cases where we want to return a simple response.
An alternative would be to consider
Stringto be the same asEffect<String>.This will reduce quite a lot the boilerplate for cases where we want to return a simple response.
That's a very interesting idea. I think we should consider and try it out.
I assume it would work for any type, be it String or not, if not an Effect then we would do a simple .reply with the returned value, right? This would also only apply for Actions, I guess.. since for entities there are other intermediate steps using the effects().
We could even take this a step further possibly:
- return (X != Effect) <=>
effects().reply - return CompletionStage<> <=>
effects().asyncReply - return DeferredCall<> <=>
effects().forward
Now, if you want side effects, emitting some events, etc, you would still use Effect directly.
But this would cover a big percentage of the most common cases.
To me, it's not so much about the boilerplate reduction (although it's also interesting from that perspective) but the fact that a newcomer does not need to understand effects from the get-go. I see this mainly as syntax sugar. When later you need to go out of the sweetened option, it can feel weird/confusing, but at that point you'll probably be more comfortable with the remaining concepts. It's not all at once, which can be overwhelming.
We raised the question the other day if it's a good idea to offer many options to do the same.
My take on that is that for the most straightforward cases, we should not need to require extra APIs. So, I personally find it a simplification and positive change.
I think that's quite easier for people to learn that they can go with a function A => B and when they need to pass more instructions to Kalix, they need to change to A => Effect[B].
I don't have any strong opinions, but personally, I would introduce Effect for users event for basic use cases, just to familiarize them with the concept, which they must understand anyway.
Effect will be practically unavoidable for Entities as without them you can't even start. So the most simple use case will already require the Effect API.
For read-only commands we can simply have return state instead of return effects().reply(state). But before even starting with read-only commands, users will already have to deal with Effect.