Action Entity send_stream_msg don't send all the chunks

[sleipnir]

Describe the bug
When making a request via stream, the first payload sent in the stream is not sent to the user function.

Example:

/home/sleipnir/go/bin/grpcurl -d @ --plaintext localhost:9000 cloudstate.tck.model.action.ActionTckModel/ProcessStreamed <<EOF
{
   "groups":{
      "steps":[
         {
            "reply":{
               "message":"The north face was first climbed on July 24, 1938"
            }
         }
      ]
   }
}

{
   "groups":{
      "steps":[
         {
            "reply":{
               "message":"The north face was first climbed on July 24, 1939"
            }
         }
      ]
   }
}

{
   "groups":{
      "steps":[
         {
            "reply":{
               "message":"The north face was first climbed on July 24, 1940"
            }
         }
      ]
   }
}

{
   "groups":{
      "steps":[
         {
            "reply":{
               "message":"The north face was first climbed on July 24, 1941"
            }
         }
      ]
   }
}
EOF

The logs resulting from this request is:

2022-02-13 06:41:27.112 [[email protected]]:[pid=<0.3878.0> ]:[debug]:Running client stream #Function<59.58486609/2 in Stream.transform/3>
2022-02-13 06:41:27.112 [[email protected]]:[pid=<0.3878.0> ]:[debug]:send_stream_msg %Cloudstate.Action.ActionCommand{metadata: %Cloudstate.Metadata{entries: []}, name: "ProcessStreamed", payload: nil, service_name: "cloudstate.tck.model.action.ActionTckModel"}
2022-02-13 06:41:27.113 [[email protected]]:[pid=<0.3878.0> ]:[debug]:send_stream_msg %Cloudstate.Action.ActionCommand{metadata: nil, name: "", payload: %Google.Protobuf.Any{type_url: "type.googleapis.com/cloudstate.tck.model.action.Request", value: "\n7\n5\n3\n1The north face was first climbed on July 24, 1939"}, service_name: ""}
2022-02-13 06:41:27.113 [[email protected]]:[pid=<0.3878.0> ]:[debug]:send_stream_msg %Cloudstate.Action.ActionCommand{metadata: nil, name: "", payload: %Google.Protobuf.Any{type_url: "type.googleapis.com/cloudstate.tck.model.action.Request", value: "\n7\n5\n3\n1The north face was first climbed on July 24, 1940"}, service_name: ""}
2022-02-13 06:41:27.114 [[email protected]]:[pid=<0.3878.0> ]:[debug]:send_stream_msg %Cloudstate.Action.ActionCommand{metadata: nil, name: "", payload: %Google.Protobuf.Any{type_url: "type.googleapis.com/cloudstate.tck.model.action.Request", value: "\n7\n5\n3\n1The north face was first climbed on July 24, 1941"}, service_name: ""}

As you can see from the payload: nil the first message goes with the payload attribute empty, and that's why in the reacquisition return only three messages are returned instead of four:

...
EOF
{
  "message": "The north face was first climbed on July 24, 1939"
}
{
  "message": "The north face was first climbed on July 24, 1940"
}
{
  "message": "The north face was first climbed on July 24, 1941"
}

To Reproduce
Steps to reproduce the behavior:

1. Execute docker user function docker run -it --rm -p 8090:8080 gcr.io/eigr-io/eigr-go-tck-action
2. Execute proxy MIX_ENV=prod USER_FUNCTION_PORT=8090 iex --name [email protected] -S mix
3. Via grpcurl run the request from the example above
4. See error

Expected behavior
That the response of all messages return in the console.

[marcellanz]

@sleipnir this might be the reason for what is observed here in this issue https://github.com/eigr-labs/akkaserverless-protocol/blob/main/cloudstateio-protocol/cloudstate-protocols-0.6.0/protocol/cloudstate/action.proto

// For streamed in and duplex streamed calls, the first command sent will just contain the service
// name and command name, but no payload. This will indicate that the action has been invoked.
// Subsequent commands on the stream will only have the payload set, the service name and command
// name will not be set.

[marcellanz]

@sleipnir I think I misunderstood this issue, while the proxy does not sent the first message, I found that from the user functions perspective, in detail, in perspective of its runtime support, the proxy has to send first a command without any payload at all. I though you describe this issue as an issue of the SDK implementation, which seems not to be.

[sleipnir]

@sleipnir this might be the reason for what is observed here in this issue https://github.com/eigr-labs/akkaserverless-protocol/blob/main/cloudstateio-protocol/cloudstate-protocols-0.6.0/protocol/cloudstate/action.proto

// For streamed in and duplex streamed calls, the first command sent will just contain the service
// name and command name, but no payload. This will indicate that the action has been invoked.
// Subsequent commands on the stream will only have the payload set, the service name and command
// name will not be set.

Hi @marcellanz, I think we have two things here.

Firstly, an implementation misconception that causes a message sent by the user to not have a complete response and second, and what led to the first mistake, a serious protocol design flaw (in my opinion), I don't know why this separation of first and other messages is necessary, especially regarding stateless functions such as Actions.
The fact that you don't send valuable information in every payload generates all sorts of work arounds in SDK implementations just to maintain this kind of state that should be co-located with the payload. I find this literally bizarre and completely unnecessary, I even tested it by making the proxy send the service_name, and name in all requests and the user role implemented with the Go SDK kept returning me without problems, that is, there is no validation on this on the user side, and neither should there be.
I think here we can clearly diverge from the original protocol.

[marcellanz]

there is more

// HandleStreamedIn handles a streamed in command. The first message in will
// contain the request metadata, including the service name and command name.
// It will not have an associated payload set. This will be followed by zero to
// many messages in with a payload, but no service name or command name set.
//
// If the underlying transport supports per stream metadata, rather than per
// message metadata, then that metadata will only be included in the metadata
// of the first message. In contrast, if the underlying transport supports per
// message metadata, there will be no metadata on the first message, the
// metadata will instead be found on each subsequent message.
//
// The semantics of stream closure in this protocol map 1:1 with the semantics
// of gRPC stream closure, that is, when the client closes the stream, the
// stream is considered half closed, and the server should eventually, but not
// necessarily immediately, send a response message with a status code and
// trailers.
// If however the server sends a response message before the client closes the
// stream, the stream is completely closed, and the client should handle this
// and stop sending more messages.
//
// Either the client or the server may cancel the stream at any time,
// cancellation is indicated through an HTTP2 stream RST message.

https://github.com/eigr/functions-go-sdk/blob/main/cloudstate/action/server.go#L110-L131

[sleipnir]

Yes, I had read that. Overall, it is unnecessary complexity and so far only justified by sending Metadata in the first message.

[marcellanz]

We can change that.

[marcellanz]

@sleipnir why does the current protocol and its description not make sense? Or why did it even made sense in the beginning?

Current AS still follows this protocol:
https://github.com/eigr-labs/akkaserverless-protocol/blob/main/akkaserverless-proxy-protocol/akkaserverless-proxy-protocol-0.8.6/akkaserverless/component/action/action.proto#L47

[sleipnir]

My question is that first if the user sends ten messages the user function should probably respond with ten messages, this is a point where the Cloudstate description is confusing by not making it clear that this is its intention, that is, the protocol should say that an empty message body containing only the metadata should be added to the beginning of the stream, being the first message that the user function would have to deal with. Second information like service_name and command_name for me are crucial in all sends and not just in the first message, making this a must makes the design of messages and sdks considerably simpler (at least in the sdks I had to implement).
How it ever made sense I don't know, I've always gone against the grain but James seems to have convinced everyone at the time that this was good design.

It seems to me that certain things in Cloudstate and Akkaserveless are the way they are because they better match the Akka/Scala way of doing things and not because they make general architectural sense.

[ralphlaude]

I fully agree with the point made by @sleipnir. Cloudstate and Akkaserveless are making thing that fits theirs Akka/Scala and not because they make general architectural sense. We should be able to remove things if they do not make sense. But also we should strive for a general architectural which makes sense