credo-ts icon indicating copy to clipboard operation
credo-ts copied to clipboard

RN crash when an unexpected didcomm message arrives from a connection

Open Iskander508 opened this issue 3 years ago • 2 comments
trafficstars

We've run into this case a few times with different didcomm messages, all ending up with an app crash.

The source of the problem is probably this root message handler: https://github.com/hyperledger/aries-framework-javascript/blob/main/packages/core/src/agent/Agent.ts#L140 It doesn't handle properly the case when a particular message handler raises an exception.

  • One example we can reproduce relatively easy: Deleting a credential exchange record, while the credential issuance process is underway. When the issue-credential message arrives then the record is not found, raising an exception, popping up to the root message handler crashing the app.
  • Another example we ran into recently with aca-py. The aca-py (issuer) didn't have the credential-definition properly stored on the ledger, and when trying to issue a credential, instead a problem-report message was sent to the wallet. Since there was no record about the credential yet (it was the first message coming via the connection), an exception was raised and popped up to the root message handler crashing the app.

reproduced on iOS

"@aries-framework/core": "0.2.3",
"@aries-framework/react-native": "0.2.3",
"react-native": "0.67.2"

Iskander508 avatar Sep 08 '22 11:09 Iskander508

I'm wondering if this is actually an error or a feature :stuck_out_tongue:, as looking at the Dispatcher code I can see that the exception is actually handled and explicitly thrown.

Anyway, if this makes the framework to stop processing messages just for the reception of an unexpected message, it's clearly an issue. In such case maybe would be nice to inform the application about this abnormal situation through an event instead.

genaris avatar Sep 08 '22 12:09 genaris

An event could be interesting. We have the agent message processes event. Currently that is only emitted on succes, but maybe that should be extended to also include errors?

TimoGlastra avatar Sep 08 '22 12:09 TimoGlastra

Closing as a lot has changed. If the issue still persists with latest 0.5.0 alphla, node 18/20 and 0.2.0 of the shared components, please open a new issue. Thanks!

TimoGlastra avatar Feb 14 '24 06:02 TimoGlastra