openwallet-foundation / credo-ts

Typescript framework for building decentralized identity and verifiable credential solutions
https://credo.js.org
Apache License 2.0
275 stars 202 forks source link

Add support for multiple transports #268

Closed jakubkoci closed 3 years ago

jakubkoci commented 3 years ago

As a developer, I want to add more inbound/outbound transporters, so that an agent can receive/send messages from/to various transport protocols

I would just implement it in a way that we change properties inboundTransporter, outboundTransporter. We call start method for each inbound transporter.

Similarly, for outboundTransporter: a) we call sendMessage for each of them, but first, we should perhaps check if a transport canReply according to information in outboundMessageContext. b) we call sendMessage for each of them and process any error to find out if it canReply

By doing this in MessageSender we can remove the majority of the logic from resolveTransport. TransportService will just return if there is any inbound transport available that could be used for outbound communication.

TimoGlastra commented 3 years ago

Duplicate of #101

TimoGlastra commented 3 years ago

a) we call sendMessage for each of them, but first, we should perhaps check if a transport canReply according to information in

I'm not sure If I understand. Do you want to call sendMessage on all transporters and check if it returns true (or something similar?) so we know the message has been delivered?

jakubkoci commented 3 years ago

Duplicate of #101

Ouch, I'm sorry I overlooked that.

jakubkoci commented 3 years ago

I thought we could add canSend(outboundMessageContext) method, call it before sendMessage like a filtering mechanism.

Just an idea. The problem with this solution is that the sendMessage called later can fail even after canSend returning true. But then that failure should be caused by some other error.

For example, for WebSocket transport:

canSend(outboundCtx) {
  if (endpoint.startsWith('ws')) {
    return true
  }
}

There are also other questions that I haven't thought through yet. Should we try to send a message via all available transports? Or only via the first one which succeeds?

TimoGlastra commented 3 years ago

I thought we could add canReply(outboundMessageContext) method, call it before sendMessage like a filtering mechanism.

I think that's a good approach. I'm not sure if canReply is the best term. Maybe we're not replying but just sending a messing.

Just an idea. The problem with this solution is that the sendMessage called later can fail even after canReply returning true. But then that failure should be caused by some other error.

Yeah so it won't guarantee delivery, it just checks that it is theoretically possible to send the message. E.g. WS transport cannot deliver to HTTP endpoint.

Should we try to send a message via all available transports? Or only via the first one which succeeds?

Only the first one, sorted by priority of available services in the diddoc. Otherwise we'll send the message two (or more) times.


I think it would be good to write out the flow and define the requirements for this task. I think it will have a big impact on the flexibility of the framework. Looking at ACA-Py, the flow for transport is very flexible but also quite complex. Some things to consider coming to mind:

TimoGlastra commented 3 years ago

Last AFJ call we discussed how the transport flow should work in AFJ. I said I would write out the flow as implemented in ACA-Py.

ACA-Py has the same concept as inbound and outbound transports as AFJ. Internally it supports HTTP and WS for inbound and outbound out of the box. The transports define everything required for the transport, meaning no custom protocol (http, ws, ..) handling is needed in other parts of the framework.

ACA-Py has responder that you can call send on to send a message. This will make a lot of hops that are not necessarily important for the flow.

It finally ends up at the outbound message router. This will do the following:

  1. Check if there is already an inbound session open (WsInboundTransporter, HttpInboundTransporter, etc...) that can be replied to.
    • You do this by calling self.inbound_transport_manager.return_to_session(outbound) which return a boolean. If the operation fails it will return false. This avoids the problem of can_reply_on_session being true, while the operations will fail. It will only return true if the messages was actually sent to the session
  2. If it could not directly send the message over an open inbound session, it will fall back to outbound transporters
  3. It first fetches all the connection targets
    • This is a list of objects containing a did, endpoint, recipient keys, routing keys, etc...
    • If the connection is not complete yet it will take the info from the invitation
  4. Then it will try to send the messages to the targets using outbound transports.
    • If I remember correctly it will try the first target (endpoint), if it fails the second, etc...
  5. Finally if no outbound transporter supports the scheme fo the connection targets (e.g. with didcomm:transport/queue. it will store the messages in an in memory queue, waiting for pickup.
TimoGlastra commented 3 years ago

After our discussion last AFJ call I've done some more thinking about the architecture of supporting multiple transporters and how it should work with inbound and outbound transporters.

I think there was a small misunderstanding in what the inbound- and outbound transporters are. I took over the exact definition as it was implemented in ACA-Py, while in AFJ we use it a bit differently. Now that I understand how you're thinking about it, it makes a lot more sense. However I still think there's some limitations to overcome before we can fully take this approach.

I think the main problem I have with this approach is the intermixing of client vs server transport and and tight coupling of the inbound transporter to the outbound transporter. I'll try to explain.

To me the difference between inbound- and outbound transporters is more the difference between client and host transports. Client transports directly send messages to serviceEndpoints and can optionally receive messages if return_route is enabled. Host transports can't directly send messages to serviceEndpoints but host the endpoint itself. This means clients can directly send messages to them. If return_route is enabled they can respond over an already open socket. By following this concept inbound- and outbound transports don't share socket or transport session data. It is contained inside the transport class itself. This means the client HTTP library is not coupled at all to the server HTTP library.

This also means that in an environment that doesn't support all server transports (e.g. on mobile you can't host a WS or HTTP endpoint) you don't include those specific transports. So in React Native you would use an HTTP Client transport and a WS client transport, but no HTTP server transport or WS server transport.

If we strictly follow the inbound and outbound transport way of thinking, we would need four transport classes to handle Ws transport:

A client can both send and receive messages, and a server can also both send and receive messages. So instead of making the difference based on inbound- or outbound I'll like to go the route of separating transport logic based on Client and Host.

So whenever I talk about an inbound transport I actually mean a Host transport. And whenever I talk about an outbound transport I actually mean a Client transport.

Please let me know if this makes any sense.

@JamesKEbert @jakubkoci

JamesKEbert commented 3 years ago

I quite like your distinctions here.. I think I for the most part agree. I think the distinction/highlight I would make though is that the Inbound and Outbound transports (such as WsClientInboundTransport and WsClientOutboundTransport) from my point of view would be one class/transport, since they share context for both sending and receiving. For instance, in Websockets we need to maintain an active socket that is used for both listening for messages and sending messages.

So, I think it makes a lot of sense to focus our terminology on Client and Host transports. I think Inbound and Outbound makes sense when speaking about processing messages that we have received or are going to be sending--but that is mostly separated from the actual transports.

So, for Websockets you would need:

And I could definitely be wrong on some assumptions above, so, please counter my thoughts if I am.

TimoGlastra commented 3 years ago

I think the distinction/highlight I would make though is that the Inbound and Outbound transports (such as WsClientInboundTransport and WsClientOutboundTransport) from my point of view would be one class/transport

Exactly that's my thinking also. The distinction between the four types was to highlight that splitting on inbound/outbound doesn't give us the right distinction.

TimoGlastra commented 3 years ago

aries-framework-javascript-Flow (1)

jakubkoci commented 3 years ago

Maybe we don’t have to change current naming for inbound/outbound transports if we just make clear that

InboundTransport can receive inbound messages and send outbound messages OutboundTransport can send outbound messages and receive inbound messages

We remove PollingInboundTransporter and WsInboundTransporter from edge agents because they doesn’t make sense anymore. If we eventually add Bluetooth or NFC type of transport, we will add BluetoothInboundTransport and BluetoothOutboundTransport.

We move the logic of sending outbound message via WebSocket transport session from outbound transport to inbound transport.

Let me just review the whole process of how we want to implement transport again. I still have some open questions:

Send outbound message via inbound transport

  1. Inbound message has return_route and there is an inbound transport session for outbound message connection, send the outbound message via inbound transport, otherwise throw an error. What if there is no inbound transport session for the given connection but there is and inbound session for the inbound message?
  2. Inbound message doesn’t have return_route but there is an inbound transport session for outbound message connection (forward to existing WebSocket session for example), send the outbound message via inbound transport session.
  3. Inbound message doesn’t have return_route and there is no inbound transport session for outbound message connection, fallback to outbound transports.

Open issues/Questions:

I’m still not sure how to design the InboundTransport interface to call sendMessage from the framework on it. Currently, I have some problems when making the framework work with the Nest.js framework. It could be partially because of my lack of knowledge of Nest.js.

One of ideas is to have an interface Session with method sendMessage. Then we wouldn’t have dependency from Dispatcher or MessageSender to InboundTransport but we could call session.sendMessage(outboundMessageContext)

Implementation of Session interface could be something like this:

class WebSocketSession implements TransportSession {
  constructor(socket) {
    this.socket = socket
  }

  sendMessage(outboundMessageCtx) {
    this.socket.send(outboundMessageCtx.payload)
  }
}

class HttpSession implements TransportSession {
  constructor(req, res) {
    this.req = req
    this.res = res
  }

  sendMessage(outboundMessageCtx) {
    this.res.send(outboundMessageCtx.payload)
  }
}

Send outbound message via outbound transport

  1. Find all available services for a given connection. If the connection is not complete yet it will take the info from the invitation.
  2. Try to pack and send the messages to the services using outbound transports. Try the first service (endpoint), if it fails the second, etc… Should we validate if the transport is capable to send the message even before packing the message?
  3. Finally if no outbound transport supports the scheme fo the connection targets (e.g. with didcomm:transport/queue. it will store the messages in an in memory queue, waiting for pickup.

Open issues/Questions:

TimoGlastra commented 3 years ago
  1. Inbound message has return_route and there is an inbound transport session for outbound message connection, send the outbound message via inbound transport, otherwise throw an error. What if there is no inbound transport session for the given connection but there is and inbound session for the inbound message?

I wouldn't throw an error. return_route means an agent MAY hold onto the connection and use it to return messages as designated. I would try outbound in that case.

TimoGlastra commented 3 years ago

BTW -- would be great if we could also fix the /msg being appended to the endpoint as part of this issue. This is done internally and seems weird to me.

I think we can solve this by providing this as input to the inbound transporter. This gives the control of the endpoint and path to the framework user. In addition we should expand the endpoint property to be an array, and remove the host/port config. This will make it possible to register endpoints for https, wss, etc...