fix: no return routing and wait for ping

[TimoGlastra]

This PR adds some improvements to return routing. Mainly it adds two things:

1. Messages returned as a http response are now emitted using an event, meaning the sendMessage await won't wait for the returned message to be processed. This could lead to a chain of send / receive messages that would take a long time to resolve. Especially if the socket was kept open because no response was returned this could lead to unwanted behaviour. (see fix: return if return route but no response hyperledger/aries-cloudagent-python#1853 for more context on socket staying open)
2. Do not include the return route decorator if not response is expected. The return route decorator is automatically added if we don't have an endpoint. This only happens with the mediator in most cases. What would happen is that the trust ping was sent to complete the connection, and we didn't receive a response. This would lead to the socket staying open for 15 seconds, before the mediation request message was sent. Now if the return route is already set (in this case to none) we won't add it in the message sender..

As mentioned in hyperledger/aries-cloudagent-python#1853, this reduces the connection + request mediation flow time from ~17 seconds to around 1.5 seconds :)

[codecov-commenter]

Codecov Report

Merging #946 (cf22cdb) into main (60ee0e5) will decrease coverage by 0.00%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##             main     #946      +/-   ##
==========================================
- Coverage   87.68%   87.67%   -0.01%     
==========================================
  Files         475      475              
  Lines       11406    11414       +8     
  Branches     1937     1938       +1     
==========================================
+ Hits        10001    10007       +6     
- Misses       1338     1340       +2     
  Partials       67       67              

Impacted Files	Coverage Δ
.../core/src/modules/connections/ConnectionsModule.ts	68.50% <33.33%> (-0.86%)	⬇️
...ckages/core/src/transport/HttpOutboundTransport.ts	13.63% <50.00%> (+2.00%)	⬆️
packages/core/src/agent/MessageSender.ts	84.61% <100.00%> (ø)
.../connections/handlers/ConnectionResponseHandler.ts	82.35% <100.00%> (+1.10%)	⬆️
...connections/handlers/DidExchangeResponseHandler.ts	81.81% <100.00%> (+0.86%)	⬆️
.../modules/connections/services/ConnectionService.ts	87.14% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60ee0e5...cf22cdb. Read the comment docs.

[JamesKEbert]

This is great Timo!
You mentioned during the WG call this morning that it does seem a little hacky. Would it be appropriate to add a new decorator or add to the return route decorator to address this specific use case, as that way it might not be as hacky? Thoughts?

[TimoGlastra]

Would it be appropriate to add a new decorator or add to the return route decorator to address this specific use case, as that way it might not be as hacky?

Could you elaborate a bit on what you mean?

[JamesKEbert]

Would it be appropriate to add a new decorator or add to the return route decorator to address this specific use case, as that way it might not be as hacky?

Could you elaborate a bit on what you mean?

Yeah, so let's say that in theory the other agents wants to send us a message after a trust ping/connection completion, they potentially could use the HTTP reply, so it's not that we don't want them sending a response it's that they aren't closing the socket in a timely manner when they don't want to send a response.
So, it might be more appropriate/less-hacky to add to the ~transport decorator with a field like "keep_alive" or "keep_socket_alive" with a boolean value:

"~transport": {
    "return_route": "all",
    "keep_alive: "false"
}

This field to indicate that we don't want the other party to keep the socket open unless they intend to send a response/another message.
May or may not be a good idea. Thoughts?

[TimoGlastra]

Interesting. Well the question arises for me, what does keep-alive mean then. This implies a message is always instantly processed and a message may be returned directly, or the socket will be closed. Maybe we should approach it more as the keep alive header in HTTP: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive?

I think ACA-Py's implementation shouldn't hang on to the http connection after a message has been processed, which means this wouldn't be an issue anymore.

For now I'll merge this as is as it's an improvement in itself. Let's keep looking at other options.

[JamesKEbert]

Good thoughts!

Maybe we should approach it more as the keep alive header in HTTP: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive?

Are you saying we should approach it as in the meaning behind the "keep_alive" field in the transport decorator should more mimic the HTTP keep alive header or as in we should be specifying this at the HTTP level (and standardize when using DIDComm over HTTP)?

I think ACA-Py's implementation shouldn't hang on to the http connection after a message has been processed, which means this wouldn't be an issue anymore.

For now I'll merge this as is as it's an improvement in itself. Let's keep looking at other options.

Agreed on both accounts