This is gonna be rather long, so bear with me to the end, and sorry in advance.
Let's start with our old friend math kite, with only one method defined on
it named square, which will take a number and it's gonna return square of that
number:
The client for corresponding server will connect to it like this:
const { Kite } = require('kite.js')
const kite = new Kite({
// assuming that they are running on the same host.
url: 'http://0.0.0.0:7780',
autoReconnect: false,
autoConnect: false,
})
kite.on('open', () => {
kite.tell('square', 5).then(res => console.log('result:', res))
// => result: 25
})
kite.connect()
So what exactly is happening here?
Under the hood, Kite uses dnode-protocol to make calls to the each side
of the connection, but since dnode-protocol doesn't try to specify a
structure on how to send messages with extra/meta information (kite info,
authentication info, etc), we need to define our own structure.
We pack all the information in a temporary object(1).
We specify this object as the only argument on the dnode-protocol request.
We specifically name the callback as responseCallback in the temporary object.
Once the responder(server) gets this request, we read this object(1)
It first checks if this is a special request by checking that the first
argument on the request satisfies our rules.
If so, it first transforms this special object(1) to a valid dnode-protocol request, then since it's a normal dnode-protocol request right now, it lets it's proto to handle this request.
Therefore, the response will be another valid dnode-protocol message to be handled by the client's proto.
Problems with current implementation
1) Packed object has a withArgs property to define arguments even though the
dnode-protocol defines an arguments object.
2) Packed object has a responseCallback as its callback resulting in always
having callbacks: { '0': ['0', 'responseCallback'] } which means having to
send a string as to define which callback is gonna be called each time. (More
bytes over the wire)
3) withArgs is an array if there is more than one argument that needs to be
passed, while it's a simple value without an array if there is only one
argument needs passing (like our math kite's square method). This creates
an unnecessary inconsistencies between requests, and therefor we need to
handle this in a special way by checking if withArgs is an array or not.
4) The request is transformed into a regular dnode-protocol message before
it reaches to the handler defined on the api, so even though we send the
requester's kite info, the transformed object doesn't have this info.
Meaning that the handlers cannot know who is making the request.
5) We are wrapping response in an object with a shape of {error, result},
this requires first wrapping on the sender side, and de-wrapping on the
responder side, resulting in unnecessary computation, and also more bytes
over the wire.
6) Both requests and responses are being handled by the same method, namely
handleIncomingMessage, which violates Single Responsibility Pattern
resulting in more complex code, therefore more confusion.
Solution Proposal
As far as I can see, the only reason we are using a specially packed object
is to send the requester's info and authentication info.
Instead of using a specially packed object with redundant properties, let's
use dnode-protocol's regular arguments array, and let's send requester's
info and authentication details as last argument.
Instead of sending a responseCallback named property inside the object,
let's make sure that the argument before last one is a function definition.
Instead of sending an object with a shape of {error, result} as response
message, let's again use the arguments array, and make sure it only has 2
arguments, first is error and second is result.
The shape of request/response after proposed changes
This solves Problem 1, 2, 3, 4, 5 and the details of the implementation will
try to address Problem 6 as well, but since this problem is mostly related with
maintanence issues, it can be deferred to a later time.
Plan of action
Refactor the current implementation without introducing new classes/types and
make sure all the current tests are passing.
Create a KiteRequest class which is responsible for creation and validation
of request messages.
Create a KiteResponse class which is responsible for creation and
validation of response messages.
Create a KiteProtocol class which is going to delegate to KiteRequest and
KiteResponse classes for making sure that the connection and messages
between 2 kites are correctly transferred, denies the requests/responses if
something is wrong.
With these changes the connection between js version of kites will be
backwards compatible, but in order to be fully compatible and if we agree that
this is the way to go, golang implementation will need to adapt to these
changes as well.
Please let me know if there is something missing, or if I am assuming more than
I should, so that we can come up with a proper protocol definition.
This is gonna be rather long, so bear with me to the end, and sorry in advance.
Let's start with our old friend
math
kite, with only onemethod
defined on it namedsquare
, which will take a number and it's gonna return square of that number:The client for corresponding server will connect to it like this:
So what exactly is happening here?
Under the hood,
Kite
usesdnode-protocol
to make calls to the each side of the connection, but sincednode-protocol
doesn't try to specify a structure on how to send messages with extra/meta information (kite info, authentication info, etc), we need to define our own structure.It currently works like this:
Request from client:
Response from server:
The way it works right now:
dnode-protocol
request.responseCallback
in the temporary object.dnode-protocol
request, then since it's a normaldnode-protocol
request right now, it lets it'sproto
to handle this request.dnode-protocol
message to be handled by the client'sproto
.Problems with current implementation
1) Packed object has a
withArgs
property to define arguments even though thednode-protocol
defines anarguments
object. 2) Packed object has aresponseCallback
as its callback resulting in always havingcallbacks: { '0': ['0', 'responseCallback'] }
which means having to send a string as to define which callback is gonna be called each time. (More bytes over the wire) 3)withArgs
is an array if there is more than one argument that needs to be passed, while it's a simple value without an array if there is only one argument needs passing (like ourmath
kite'ssquare
method). This creates an unnecessary inconsistencies between requests, and therefor we need to handle this in a special way by checking ifwithArgs
is an array or not. 4) The request is transformed into a regulardnode-protocol
message before it reaches to the handler defined on the api, so even though we send the requester'skite
info, the transformed object doesn't have this info. Meaning that the handlers cannot know who is making the request. 5) We are wrappingresponse
in an object with a shape of{error, result}
, this requires first wrapping on the sender side, and de-wrapping on the responder side, resulting in unnecessary computation, and also more bytes over the wire. 6) Both requests and responses are being handled by the same method, namelyhandleIncomingMessage
, which violatesSingle Responsibility Pattern
resulting in more complex code, therefore more confusion.Solution Proposal
dnode-protocol
's regulararguments
array, and let's send requester's info and authentication details as last argument.responseCallback
named property inside the object, let's make sure that the argument before last one is afunction
definition.{error, result}
as response message, let's again use thearguments
array, and make sure it only has 2 arguments, first iserror
and second isresult
.The shape of request/response after proposed changes
Request from client:
Response from server:
The code needs to be written will be the same, except now the handlers will have information about the requester as their last argument:
This solves Problem 1, 2, 3, 4, 5 and the details of the implementation will try to address Problem 6 as well, but since this problem is mostly related with maintanence issues, it can be deferred to a later time.
Plan of action
KiteRequest
class which is responsible for creation and validation ofrequest
messages.KiteResponse
class which is responsible for creation and validation ofresponse
messages.KiteProtocol
class which is going to delegate toKiteRequest
andKiteResponse
classes for making sure that the connection and messages between 2 kites are correctly transferred, denies the requests/responses if something is wrong.With these changes the connection between
js
version of kites will be backwards compatible, but in order to be fully compatible and if we agree that this is the way to go,golang
implementation will need to adapt to these changes as well.Please let me know if there is something missing, or if I am assuming more than I should, so that we can come up with a proper protocol definition.
/cc @gokmen @cihangir @rjeczalik @szkl @ppknap @sinan