Websockets #218

jakubkoci · 2021-03-30T14:55:01Z

First working solution with WebSockets. There is definitely still some space for improvement. Take it as minimal mergeable PR :)

Each edge agent communicates with its own mediator via WebSockets. The communication from Alice to Bob goes through Bob's mediator via HTTP. Then bob gets the message from the mediator via WebSockets and vice versa.

I basically copy-pasted e2e.test.ts and mediator.ts files to e2e.ws.test.ts respective mediator.ws.ts. We could have just one mediator, maybe even one e2e test file, after we add multiple inbound transporters which I would like to do as the next step.

Signed-off-by: Jakub Koci <jakub.koci@gmail.com>

TimoGlastra

Thanks for this Jakub! I think this is a long awaited PR :)

Most comments are thoughts and don't have to be fixed right now. I think a lot those would be resolved when multiple transports are added.

However the usage of socket.io is a blocker for me, and I don't think I can be convinced to approve this before we use a generic WS library.

TimoGlastra · 2021-03-31T09:32:37Z

package.json

+    "@types/socket.io": "^2.1.13",
+    "@types/socket.io-client": "^1.4.36",


Should be in devDependencies

TimoGlastra · 2021-03-31T09:33:25Z

src/agent/Agent.ts

@@ -38,6 +39,7 @@ export class Agent {
  protected messageReceiver: MessageReceiver;
  protected dispatcher: Dispatcher;
  protected messageSender: MessageSender;
+  protected transportService: TransportService;


I like the transport service

TimoGlastra · 2021-03-31T09:35:22Z

src/agent/MessageReceiver.ts

+      if (connection) {
+        if (transport) {
+          this.transportService.saveTransport(connection.id, transport);
+        }
+      }


Suggested change

if (connection) {

if (transport) {

this.transportService.saveTransport(connection.id, transport);

}

}

if (connection && transport)

this.transportService.saveTransport(connection.id, transport);

}

TimoGlastra · 2021-03-31T09:37:24Z

src/agent/MessageSender.ts

+    if (transport) {
+      outboundPackage.transport = transport;
+    } else {
+      outboundPackage.transport = new HttpTransport(outboundMessage.endpoint);


This should look at the scheme of the endpoint instead of creating a hardcoded transport. Ideally the outboundMessage.endpoint is not set here yet. As we could then dynamically select the transport to use based on available endpoints of the the connection and available transports.

TimoGlastra · 2021-03-31T09:38:26Z

src/agent/MessageSender.ts

+    if (transport) {
+      outboundPackage.transport = transport;
+    } else {
+      outboundPackage.transport = new HttpTransport(outboundMessage.endpoint);


TimoGlastra · 2021-03-31T09:43:29Z

src/modules/routing/RoutingModule.ts

@@ -120,4 +131,5 @@ interface MediatorConfiguration {
  verkey: Verkey;
  invitationUrl: string;
  alias?: string;
+  transport?: Transport;


I don't think we should save a transport with the mediator. We're moving more towards mediator coordination protocol in which case the flow probably looks something like this:

Mediator creates invitation

Mediatee receives invitation, sends request (probably HTTP)

Mediatior sends response with DIDDoc containing both ws and http endpoint.

We make a preference to use WS, but could also use HTTP. So it should be more dynamic I think.

I see. The transport here is passed mainly to say what transport should be used for this step:

Mediatee receives invitation, sends request (probably HTTP)

I assumed that it should be possible to do it either via HTTP or WS.

Maybe, we should select the transport channel based on the service endpoint in the invitation? I see that 0. Invitation to Connect defines just one serviceEndpoint but Aries RFC 0434: Out-of-Band Protocol 1.1 shows an example with services as an array.

Maybe, we should select the transport channel based on the service endpoint in the invitation?

I think for creating the connection with the mediator it should use the one from the invitation, just like we do for any other connection. After that it should use the transport(s) as defined in the received did doc

TimoGlastra · 2021-03-31T09:45:22Z

src/agent/TransportService.ts

+export interface Transport {
+  type: TransportType;
+}


I think it would be nice to add a canReply method to this interface

TimoGlastra · 2021-03-31T09:46:27Z

src/agent/TransportService.ts

+  public constructor(socket: Socket) {
+    this.socket = socket;
+  }
+}


When the socket (http or ws) will be closed canReply can be set to false.

Also when we already replied with a message on the http channel we can also set canReply to false

TimoGlastra · 2021-03-31T09:47:43Z

src/agent/Agent.ts

-  public async receiveMessage(inboundPackedMessage: unknown) {
-    return await this.messageReceiver.receiveMessage(inboundPackedMessage);
+  public async receiveMessage(inboundPackedMessage: unknown, transport?: Transport) {
+    return await this.messageReceiver.receiveMessage(inboundPackedMessage, transport);


I think it would be nice to add transport to the InboundMessageContext

TimoGlastra · 2021-03-31T09:50:59Z

src/agent/TransportService.ts

@@ -0,0 +1,41 @@
+import { Socket } from 'socket.io';


I've mentioned this before, but AFAIK socket.io uses non-standard communication protocols. This means this won't work with ACA-Py as a mediator.

I have objections to merging this with socket.io instead of a general WS server. If you can make it such that socket.io can communicate with non-socket.io agents that's also fine

As @TimoGlastra suggests, Javascript WebSocket works with ACA-py. I have tested this in my own code and works. https://developer.mozilla.org/en-US/docs/Web/API/WebSocket

jakubkoci · 2021-04-05T07:34:12Z

Thanks @TimoGlastra and @JamesKEbert for the helpful discussion and explanation of the mediation protocol in our last call. I went through the code and specs again and I understand it a little bit more. I still have some questions but also some ideas about the implementation.

First of all, I read the Aries RFC 0025: DIDComm Transports and it says:

“POST requests are considered transmit only by default. No agent messages will be returned in the response. This behavior may be modified with additional signaling.”
“Websockets are considered transmit only by default. Messages will only flow from the agent that opened the socket. This behavior may be modified with additional signaling.”

Do I understand correctly that “This behavior may be modified with additional signaling.” means Aries RFC 0092: Transports Return Route?

Regarding the implementation, we should probably solve the problem with sendAndReceiveMessage and return_route before we do the change from socket.io to WebSocket implementation.

I implemented the current solution in a way to achieve some sort of synchronicity to establish a connection with a mediator. I realized that we can achieve the same with the same or similar concept like returnWhenIsConnected. That would mean just create and send a message and wait for the rest to be handled by the framework.

// Simplified code
const connReq = await this.connectionService.createRequest(connection.id);
await this.messageSender.sendMessage(connReq);
await returnWhenIsConnected(conn)

This change will require some changes in HttpOutboundTransporter and downloadMessages implementation. Before that, I just want to be sure I understand the DIDComm Transports and return_route correctly.

If return_route is set, then a request message sender wants to receive a response message via the same transport channel as it sent the message. It means that the mediator will send an outbound message via HTTP response for HTTP request and via the same socket ID for WebSocket connection. If the return route is not set, then an outbound message should be send according to the information from a connection DIDDoc. An agent should decide outbound transport channel based on information from the DIDDoc (http endpoint or indication to store the message in case of didcomm:transport/queue endpoint) together with current transport state in transport service (connected socket ID in case of ws transport).

An edge agent connected to a mediator can set return_route and get message response via HTTP response or as WebSocket message. If the edge agent doesn’t set return_route the mediator just stores messages and the edge agent doesn’t get them until it sends batchPickup message with return_route.

Is my understanding correct? :)

TimoGlastra · 2021-04-06T15:10:54Z

Do I understand correctly that “This behavior may be modified with additional signaling.” means Aries RFC 0092: Transports Return Route?

Yep!

Is my understanding correct? :)

Yeah seems about right

TimoGlastra · 2021-05-07T08:08:31Z

@jakubkoci can this be closed now that we have #256?

jakubkoci · 2021-05-12T14:31:48Z

Replaced by #250

jakubkoci added 4 commits March 30, 2021 14:39

Add websockets transports with socket.io

547aa10

Signed-off-by: Jakub Koci <jakub.koci@gmail.com>

Introduce websocket and http transport class

fa8eb60

Signed-off-by: Jakub Koci <jakub.koci@gmail.com>

Align edge agent inboud transporters

b340967

Signed-off-by: Jakub Koci <jakub.koci@gmail.com>

Return back test logger instead of console logger

13aa1dc

Signed-off-by: Jakub Koci <jakub.koci@gmail.com>

jakubkoci requested a review from a team as a code owner March 30, 2021 14:55

TimoGlastra requested changes Mar 31, 2021

View reviewed changes

jakubkoci mentioned this pull request Apr 11, 2021

refactor: make a connection with mediator asynchronously #231

Merged

icc-romeu mentioned this pull request Apr 16, 2021

Invitation from aca-py agent with aca-py mediator between #230

Closed

jakubkoci closed this May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Websockets #218

Websockets #218

jakubkoci commented Mar 30, 2021

TimoGlastra left a comment

TimoGlastra Mar 31, 2021

TimoGlastra Mar 31, 2021

TimoGlastra Mar 31, 2021

TimoGlastra Mar 31, 2021

TimoGlastra Mar 31, 2021

TimoGlastra Mar 31, 2021

jakubkoci Apr 6, 2021

TimoGlastra Apr 6, 2021

TimoGlastra Mar 31, 2021

TimoGlastra Mar 31, 2021

TimoGlastra Mar 31, 2021

TimoGlastra Mar 31, 2021

icc-romeu Mar 31, 2021

jakubkoci commented Apr 5, 2021

TimoGlastra commented Apr 6, 2021

TimoGlastra commented May 7, 2021

jakubkoci commented May 12, 2021

		"@types/socket.io": "^2.1.13",
		"@types/socket.io-client": "^1.4.36",

Websockets #218

Websockets #218

Conversation

jakubkoci commented Mar 30, 2021

TimoGlastra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubkoci commented Apr 5, 2021

TimoGlastra commented Apr 6, 2021

TimoGlastra commented May 7, 2021

jakubkoci commented May 12, 2021