Fix connection record response for mobile

[ianco]

Signed-off-by: Ian Costanzo [email protected]

See issue: #1441

This is a bit of a shot in the dark, the connection auto-replies aren't following the same convention as other protocols.

[TimoGlastra]

@ianco Does this fix #1372? I don't see any code related to this issue

[codecov-commenter]

Codecov Report

Merging #1469 (eec51a1) into main (26d23bf) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1469      +/-   ##
==========================================
- Coverage   95.73%   95.71%   -0.03%     
==========================================
  Files         527      527              
  Lines       32375    32372       -3     
==========================================
- Hits        30994    30984      -10     
- Misses       1381     1388       +7     

[ianco]

@ianco Does this fix #1372? I don't see any code related to this issue

Hi @TimoGlastra I think you're right. I thought it may be related because you're dealing with connections but upon review I feel that it's a different issue

@seriousManual @ntsbs do you think this will resolve your issue #1441 ?

I've changed the PR to Draft status because I think the issue may be more complicated. Responses can be initiated in the routes class (called from admin api), one of the handlers (response to a message from another agent) or within the manager class (common to both).

I think the issue here was that the response initiated within the manager class explicitly referenced the mediator, whereas the response initiated within the handler "knows" that there is an open session to return the response.

This issue may come up for other types of transactions as well, @swcurran @andrewwhitehead @shaangill025 any thoughts?

[seriousManual]

@seriousManual @ntsbs do you think this will resolve your issue #1441 ?

Honestly I don't know.
I'm not really familiar with Python in general and the aca-py code base, so I can't tell if this is related at all.

[ianco]

@seriousManual @ntsbs do you think this will resolve your issue #1441 ?

Honestly I don't know. I'm not really familiar with Python in general and the aca-py code base, so I can't tell if this is related at all.

Can we test this out somehow? Can you build an aca-py docker image with the updated code? Or if not maybe I can build/publish a docker image for you?

[seriousManual]

Hi @ianco , if you could an image that would be awesome!

[ianco]

Hi @ianco , if you could an image that would be awesome!

Hi @seriousManual you can try running with this image:

https://hub.docker.com/layers/anonsolutions/aries-cloudagent/py36-1.16-1_0.7.2-rc1/images/sha256-2f880b93bb09d0ff6dad79d190c85bec80e21ee7df3c0192d3aae4bb642e609f?context=explore

If you want to build an image yourself, I can send some instructions, just let me know.

[seriousManual]

Hey Ian,
we deployt the Image and ran it aganst the wallet app, the error persists unfortunately.

[ianco]

Hey Ian, we deployt the Image and ran it aganst the wallet app, the error persists unfortunately.

OK thanks, back to the drawing board ...

[dbluhm]

This actually solves a problem we're experiencing with the toolbox and ACA-Py 0.7.2. With the session -> profile changes made wherever possible, I found that the BaseResponder instance now available for use in the ConnectionsManager was an AdminResponder rather than the DispatcherResponder from the message currently being processed so it lost the reply_to_verkey that is held in the DispatcherResponder and made it unable to respond to the HTTP session.

[dbluhm]

This instance appears to be the only time where a BaseResponder instance is injected and send_reply is attempted so in theory there shouldn't be other lurking problems. However, this feels like a gotcha that's going to get me again in the future lol. What dependencies are available in what injection context and when has lost a significant amount of clarity for me since we now have contexts, profiles, and sessions of various types to consider.

[dbluhm]

This also appears to impact the auto_accept condition of the DID Exchange manager receive_request.

[dbluhm]

Unfortunately, I think this is a critical bug for using ACA-Py 0.7.2 with agents without endpoints (mobile agent, toolbox, etc.) as it renders them unable to connect.

[ianco]

Hey @dbluhm thanks for the comments, I wasn't sure if this was an issue, it was a stab in the dark to fix a different issue (which it didn't even fix)

[dbluhm]

These changes have fixed errors we were experiencing; however, seems like it would be beneficial to get someone else's eyes on this since I contributed some of these changes.

[shaangill025]

Looks good, may be tests can be updated to cover recent changes made to:

• connections/v1_0/handlers/connection_request_handler.py
• didexchange/v1_0/handlers/request_handler.py