Serializer sometimes decodes a wrong class for RPC request

[ocervinka]

Serializer sometimes decodes a wrong class for RPC request

[ocervinka]

The simplest alternation of the code was to remove custom serializers and simplify Request and Response records. I also made them exactly the same.

public record Request (
        @Deserialize("label") String label
) {
    @Override
    @Serialize(order = 1)
    public String label() {
        return label;
    }
}

and

public record Response (
        @Deserialize("label") String label
) {
    @Override
    @Serialize(order = 1)
    public String label() {
        return label;
    }
}

those are now the only message types registered along with a single request handler.

RPC client sends Request. RPC server decodes Response (not expected) with the data being correct (label=testLabel) and then can't look up the handler (expected).

2023-04-28T10:28:38.869Z INFO io.activej.rpc.server.RpcServer [RPC Event Loop] Client connected on RpcServerConnection{address=/127.0.0.1, active=1, successes=0, failures=0}

2023-04-28T10:28:38.878Z WARN  hostName i.a.rpc.server.RpcServerConnection [RPC Event Loop] Exception while processing request ID 1
io.activej.common.exception.MalformedDataException: Failed to process request Response[label=testLabel]
	at io.activej.rpc.server.RpcServerConnection.serve(RpcServerConnection.java:72)
	at io.activej.rpc.server.RpcServerConnection.accept(RpcServerConnection.java:85)
	at io.activej.rpc.server.RpcServerConnection.accept(RpcServerConnection.java:39)
	at io.activej.datastream.AbstractStreamSupplier.send(AbstractStreamSupplier.java:153)
	at io.activej.datastream.csp.ChannelDeserializer.process(ChannelDeserializer.java:159)
	at io.activej.datastream.csp.ChannelDeserializer.onResumed(ChannelDeserializer.java:83)
	at io.activej.datastream.AbstractStreamSupplier.resume(AbstractStreamSupplier.java:142)
	at io.activej.datastream.AbstractStreamSupplier.asyncResume(AbstractStreamSupplier.java:135)
	at io.activej.datastream.csp.ChannelDeserializer.lambda$onResumed$1(ChannelDeserializer.java:117)
	at io.activej.promise.AbstractPromise$25.accept(AbstractPromise.java:1134)
	at io.activej.promise.AbstractPromise.complete(AbstractPromise.java:114)
	at io.activej.promise.SettablePromise.set(SettablePromise.java:68)
	at io.activej.csp.queue.ChannelZeroBuffer.put(ChannelZeroBuffer.java:83)
	at io.activej.csp.queue.ChannelQueue$1.doAccept(ChannelQueue.java:66)
	at io.activej.csp.AbstractChannelConsumer.accept(AbstractChannelConsumer.java:51)
	at io.activej.csp.ChannelSuppliers.lambda$streamToImpl$6(ChannelSuppliers.java:236)
	at io.activej.promise.AbstractPromise.complete(AbstractPromise.java:114)
	at io.activej.promise.SettablePromise.set(SettablePromise.java:68)
	at io.activej.net.socket.tcp.AsyncTcpSocketNio.onReadReady(AsyncTcpSocketNio.java:374)
	at io.activej.eventloop.Eventloop.onRead(Eventloop.java:883)
	at io.activej.eventloop.Eventloop.processSelectedKeys(Eventloop.java:630)
	at io.activej.eventloop.Eventloop.run(Eventloop.java:543)

[eduard-vasinskyi]

Hi, @ocervinka

You said that the same message types are used on client and server. But is the order of message types the same? If not, then this could cause the issue.

If you are sure that the order of message types is the same on the client and on the server, then could you please create a minimalistic demo that demonstrates the problem? Then I will try to help you with the issue.

[ocervinka]

@eduard-vasinskyi thanks for your help. Indeed the order of message types in List argument of RpcServer#withMessageTypes method wasn't guaranteed.

I suggest mentioning in Javadoc of that method that the order is important and must be the same for all application instances in order to communicate to each other.

When RpcServer and RpcClient are used directly there is less chance of making this mistake. However, the library kept message types in a Set for uniqueness because multiple subjects can register their KS IQ calls independently. Then it converted the Set to a List which caused the random deserialisation failures.

[eduard-vasinskyi]

@ocervinka
We will mention in Javadoc that the order of message types matters.

[ocervinka]

@eduard-vasinskyi do you want me to close the issue?

I can create PR with the Javadoc update, although the RpcServer#withMessageTypes is not in master anymore. It's been replaced by RpcServer#withSerializer so the Javadoc should be probably added to RpcMessageSerializer#ofmethods?

[eduard-vasinskyi]

@ocervinka
I have updated javadocs in the master-5.5 branch. These changes have been merged into the master branch.

[ocervinka]

Many thanks @eduard-vasinskyi .