OOB Multiuse invitations not emitting expected done webhook

[dbluhm]

Sometime between main and 0.11.0, we've lost a webhook of topic out_of_band, state done being emitted from ACA-Py when a connection has been completed through multiuse OOB invitations.

This is a breaking change that makes it difficult for controllers to determine when a connection was formed through an invitation intended to trigger a flow (e.g. with goal codes).

[swcurran]

@ianco — do you have some time to investigate this?

[ianco]

@ianco — do you have some time to investigate this?

I will take a look!

[ianco]

Would be nice if the integration tests also tested for receipt of webhooks ... I added a ticket for that awhile back (#2675)

[swcurran]

Needed for 0.12.0

[ianco]

@dbluhm can you confirm everything was working properly in 0.11.0?

I can only find 2 commits that may have impacted this, one of yours (to oob_manager) (I don't think this is causing an issue):

2fa5282

And (the more likely one) of mine, dealing with a different webhook issue:

62ec2c0

... although I can't see where a webhook that had been previously emitted has been removed ...

I'll pull the 0.11.0 version and try it out ...

[dbluhm]

I can do a quick test run and should be able to provide a link to an easy to run example

[dbluhm]

Minimal reproducible example of it working on 0.11.0 found here:

https://github.com/Indicio-tech/acapy-minimal-example/tree/test/oob-0.11.0/examples/simple

git clone https://github.com/Indicio-tech/acapy-minimal-example.git
cd acapy-minimal-example/examples/simple
git checkout test/oob-0.11.0
docker-compose run example
# cleaning up
docker-compose down -v

[dbluhm]

A quick log of the output from that branch attached.

Hopefully it's easy enough to parse!
0.11.0.log

[ianco]

@dbluhm thanks looking at this now ...

[dbluhm]

Might be relevant, I recently realized that in order for the queued events to be emitted from a transaction, the transaction must be explicitly committed; e.g.:

async with profile.transaction() as txn:
    # do some stuff...
    await rec.save(txn)
    await txn.commit()

Discovered this while doing the multi-use respond in kind fix

[ianco]

With the aca-py 0.11.0 version, Alice receives a request webhook, and then done.

With the latest aca-py Alice receives an init webhook, and then a request (and then no done).

So there's a new init webhook as well as the missing done.

[ianco]

Might be relevant, I recently realized that in order for the queued events to be emitted from a transaction, the transaction must be explicitly committed; e.g.:

Yes this is my commit that I referenced above.

[dbluhm]

Yes, the init webhook is one that I added here: https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/protocols/didexchange/v1_0/manager.py#L715-L719

With the open question of whether we should suppress the new webhook. Originally, the logic was resulting in a race condition that we worked around by adding a transaction for saving the conn rec and attaching the request.

[ianco]

Aha this commit is what caused this issue: 76a1d26

In oob_processor.py we check if the OOB record is multi-use, and if not then we emit the done event. Since the multi_use flag wasn't getting saved we always emitted the event.

I'll update the oob_processor to always emit the done event even for mult_use.

[ianco]

PS @dbluhm that simple example of yours was super useful. @swcurran we should link to this from aca-py somewhere, and I'm going to steal your method of checking for webhooks for the aca-py integration tests.