You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Client & Server messages' handling are not very readable: multiple types of messages are mixed (aggregator's with network's with orgchestration's) and are handled out of order by reling on EventEmitter.on('msg') callbacks. discussions on #775 gave some idea on how to potentially fix that.
let's first remark that many parts requires a different set of message (federated vs decentralized, secure aggregator vs some other) yet are still used in the same fashion. so it seems that we can plug a class with various implementations.
we can have an abstract NetworkClient with various implementation, one federated, one decentralized (and one local). theses advertise their supported messages by exposing a isValidMessage(raw: unknown): raw is Message (with Message depending on the implementation). that ensure that the type of messages we are receiving are typed correctly. it then gets passed an async queue of messages (such as reactive-channel's one).
same idea for AggregatorClient
Client should do more, and not be an abstract class but be constructed with NetworkClient, AggregatorClient, …. it should take care of splitting the message stream, ie read next msg and put it the queue where the corresponding isValidMessage succeeded.
that helps by having a local set of messages per implementation, and only care about these ones. we can also better check that the server is not weirdly behaving by failing when the next messages is unexpected. and message order is preserved because async channels are also ordered and there is a single reader of WebSocket.
with that, it forces us to check that we don't receive an unexpected message in the stream. that can be a bit bothering in the long run (I've other ways to fix that such as type-encoding a state-transition table, but I might be going a bit far for now). but overall, I do think that can greatly help us understand where messages are coming from and in which state we are.
Client & Server messages' handling are not very readable: multiple types of messages are mixed (aggregator's with network's with orgchestration's) and are handled out of order by reling on
EventEmitter.on('msg')
callbacks. discussions on #775 gave some idea on how to potentially fix that.let's first remark that many parts requires a different set of message (federated vs decentralized, secure aggregator vs some other) yet are still used in the same fashion. so it seems that we can plug a class with various implementations.
we can have an abstract
NetworkClient
with various implementation, one federated, one decentralized (and one local). theses advertise their supported messages by exposing aisValidMessage(raw: unknown): raw is Message
(withMessage
depending on the implementation). that ensure that the type of messages we are receiving are typed correctly. it then gets passed an async queue of messages (such asreactive-channel
's one).same idea for
AggregatorClient
Client
should do more, and not be an abstract class but be constructed withNetworkClient
,AggregatorClient
, …. it should take care of splitting the message stream, ie read next msg and put it the queue where the correspondingisValidMessage
succeeded.that helps by having a local set of messages per implementation, and only care about these ones. we can also better check that the server is not weirdly behaving by failing when the next messages is unexpected. and message order is preserved because async channels are also ordered and there is a single reader of WebSocket.
with that, it forces us to check that we don't receive an unexpected message in the stream. that can be a bit bothering in the long run (I've other ways to fix that such as type-encoding a state-transition table, but I might be going a bit far for now). but overall, I do think that can greatly help us understand where messages are coming from and in which state we are.
Originally posted by @tharvik in #775 (comment)
The text was updated successfully, but these errors were encountered: