-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: improve sending error handling #1045
feat: improve sending error handling #1045
Conversation
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #1045 +/- ##
==========================================
- Coverage 88.74% 88.73% -0.02%
==========================================
Files 522 523 +1
Lines 12156 12176 +20
Branches 1913 1913
==========================================
+ Hits 10788 10804 +16
- Misses 1364 1368 +4
Partials 4 4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice change! Seems rather useful for any non-happy flow.
-- I also like the TSDocs you added :D
Just thinking here, but what if instead of handing this in the module layer, we hanle this in the message layer? That way we don't have to support this in every module, and new modules get this functionality out of the box. One issue is that the module doesn't have access to the associated record, but maybe we can add some optional metadata to the outbound message (such as associatedRecord). |
Good idea! Other modules will still need to add associatedRecord when calling to MessageSender, but their logic will remain the same. And fewer lines modified means less potential bugs being introduced :-) I think the simplest approach for this would be adding associatedRecord as optional field in options for |
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
So I finally added it to OutboundMessage because I think it makes more sense there (and also it could be useful to use in other flows like message reception and handlers). In |
cause: error, | ||
}) | ||
} | ||
outboundMessage.associatedRecordId = basicMessageRecord.id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the id but not the type can make it hard to get the record type in a generic context. A possibility is attachign the whole record, but I like the id appraoch better.. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was also hesitating on using the whole record or just the id. Opted to go for the id in order to make this whole outbound message more 'lightweight' and considering that the context would know the type of record to search for. But I don't have a very strong opinion on this. Can change to the whole record if we foresee that it's more convenient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think my preference would go to attaching the record
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
* feat: OOB public did (#930) Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> * feat(routing): manual mediator pickup lifecycle management (#989) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * docs(demo): faber creates invitation (#995) Signed-off-by: conanoc <conanoc@gmail.com> * chore(release): v0.2.3 (#999) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(question-answer): question answer protocol state/role check (#1001) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: Action Menu protocol (Aries RFC 0509) implementation (#974) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * fix(ledger): remove poolConnected on pool close (#1011) Signed-off-by: Niall Shaw <niall.shaw@absa.africa> * fix(ledger): check taa version instad of aml version (#1013) Signed-off-by: Jakub Koci <jakub.koci@gmail.com> * chore: add @janrtvld to maintainers (#1016) Signed-off-by: Timo Glastra <timo@animo.id> * feat(routing): add settings to control back off strategy on mediator reconnection (#1017) Signed-off-by: Sergi Garreta <sergi.garreta@entrust.com> * fix: avoid crash when an unexpected message arrives (#1019) Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> * chore(release): v0.2.4 (#1024) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * style: fix some lint errors Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: use did:key flag (#1029) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * ci: set default rust version (#1036) Signed-off-by: Sai Ranjit Tummalapalli <sairanjit5@gmail.com> * fix(oob): allow encoding in content type header (#1037) Signed-off-by: Timo Glastra <timo@animo.id> * feat: connection type (#994) Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com> * chore(module-tenants): match package versions Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: improve sending error handling (#1045) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: expose findAllByQuery method in modules and services (#1044) Signed-off-by: Jim Ezesinachi <jim@animo.id> * feat: possibility to set masterSecretId inside of WalletConfig (#1043) Signed-off-by: Andrii Uhryn <an.ugryn@gmail.com> * fix(oob): set connection alias when creating invitation (#1047) Signed-off-by: Jakub Koci <jakub.koci@gmail.com> * build: fix missing parameter Signed-off-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> Signed-off-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: conanoc <conanoc@gmail.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: Niall Shaw <niall.shaw@absa.africa> Signed-off-by: Jakub Koci <jakub.koci@gmail.com> Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Sergi Garreta <sergi.garreta@entrust.com> Signed-off-by: Sai Ranjit Tummalapalli <sairanjit5@gmail.com> Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com> Signed-off-by: Jim Ezesinachi <jim@animo.id> Signed-off-by: Andrii Uhryn <an.ugryn@gmail.com> Co-authored-by: Iskander508 <pavel.zarecky@seznam.cz> Co-authored-by: conanoc <conanoc@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Niall Shaw <100220424+niallshaw-absa@users.noreply.github.com> Co-authored-by: jakubkoci <jakub.koci@gmail.com> Co-authored-by: Timo Glastra <timo@animo.id> Co-authored-by: Sergi Garreta Serra <garretaserra@gmail.com> Co-authored-by: Sai Ranjit Tummalapalli <34263716+sairanjit@users.noreply.github.com> Co-authored-by: KolbyRKunz <KolbyKunz@yahoo.com> Co-authored-by: Jim Ezesinachi <ezesinachijim@gmail.com> Co-authored-by: an-uhryn <55444541+an-uhryn@users.noreply.github.com>
* feat: OOB public did (openwallet-foundation#930) Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> * feat(routing): manual mediator pickup lifecycle management (openwallet-foundation#989) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * docs(demo): faber creates invitation (openwallet-foundation#995) Signed-off-by: conanoc <conanoc@gmail.com> * chore(release): v0.2.3 (openwallet-foundation#999) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(question-answer): question answer protocol state/role check (openwallet-foundation#1001) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: Action Menu protocol (Aries RFC 0509) implementation (openwallet-foundation#974) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * fix(ledger): remove poolConnected on pool close (openwallet-foundation#1011) Signed-off-by: Niall Shaw <niall.shaw@absa.africa> * fix(ledger): check taa version instad of aml version (openwallet-foundation#1013) Signed-off-by: Jakub Koci <jakub.koci@gmail.com> * chore: add @janrtvld to maintainers (openwallet-foundation#1016) Signed-off-by: Timo Glastra <timo@animo.id> * feat(routing): add settings to control back off strategy on mediator reconnection (openwallet-foundation#1017) Signed-off-by: Sergi Garreta <sergi.garreta@entrust.com> * fix: avoid crash when an unexpected message arrives (openwallet-foundation#1019) Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> * chore(release): v0.2.4 (openwallet-foundation#1024) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * style: fix some lint errors Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: use did:key flag (openwallet-foundation#1029) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * ci: set default rust version (openwallet-foundation#1036) Signed-off-by: Sai Ranjit Tummalapalli <sairanjit5@gmail.com> * fix(oob): allow encoding in content type header (openwallet-foundation#1037) Signed-off-by: Timo Glastra <timo@animo.id> * feat: connection type (openwallet-foundation#994) Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com> * chore(module-tenants): match package versions Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: improve sending error handling (openwallet-foundation#1045) Signed-off-by: Ariel Gentile <gentilester@gmail.com> * feat: expose findAllByQuery method in modules and services (openwallet-foundation#1044) Signed-off-by: Jim Ezesinachi <jim@animo.id> * feat: possibility to set masterSecretId inside of WalletConfig (openwallet-foundation#1043) Signed-off-by: Andrii Uhryn <an.ugryn@gmail.com> * fix(oob): set connection alias when creating invitation (openwallet-foundation#1047) Signed-off-by: Jakub Koci <jakub.koci@gmail.com> * build: fix missing parameter Signed-off-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: Pavel Zarecky <zarecky@procivis.ch> Signed-off-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: conanoc <conanoc@gmail.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: Niall Shaw <niall.shaw@absa.africa> Signed-off-by: Jakub Koci <jakub.koci@gmail.com> Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Sergi Garreta <sergi.garreta@entrust.com> Signed-off-by: Sai Ranjit Tummalapalli <sairanjit5@gmail.com> Signed-off-by: KolbyRKunz <KolbyKunz@yahoo.com> Signed-off-by: Jim Ezesinachi <jim@animo.id> Signed-off-by: Andrii Uhryn <an.ugryn@gmail.com> Co-authored-by: Iskander508 <pavel.zarecky@seznam.cz> Co-authored-by: conanoc <conanoc@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Niall Shaw <100220424+niallshaw-absa@users.noreply.github.com> Co-authored-by: jakubkoci <jakub.koci@gmail.com> Co-authored-by: Timo Glastra <timo@animo.id> Co-authored-by: Sergi Garreta Serra <garretaserra@gmail.com> Co-authored-by: Sai Ranjit Tummalapalli <34263716+sairanjit@users.noreply.github.com> Co-authored-by: KolbyRKunz <KolbyKunz@yahoo.com> Co-authored-by: Jim Ezesinachi <ezesinachijim@gmail.com> Co-authored-by: an-uhryn <55444541+an-uhryn@users.noreply.github.com>
Currently, when an outbound message is sent as a result of calling a module/API method and a transport error happens, an exception is thrown without any retry mechanism (unless handled externally by the outbound transporter object). In the particular case of Basic Messages, when such situation happens, a record is created but there are no means from the calling application to know its id and delete it or mark it as failed in the UI.
This PR addresses this issue by throwing in
BasicMessageModule.sendMessage
a new kind of error:MessageSendingError
, which includes the message itself and the associated record. The reason of adding the message is to open the possibility for the calling application to retry sending if it wants (not something so straightforward though, but feasible).The same strategy could potentially be extended to methods from other modules that also create records and send messages in a single step (e.g. sendQuestion, proposeCredential, offerCredential).
Signed-off-by: Ariel Gentile gentilester@gmail.com