getamis / alice

Hierarchical Threshold Signature Scheme
Apache License 2.0
375 stars 75 forks source link

Surface errors from `messageLoop` #292

Open blakeofwilliam opened 6 months ago

blakeofwilliam commented 6 months ago

Is this a BUG REPORT or FEATURE REQUEST?:

Feature Request

What happened:

Currently, when errors occur in the messageLoop or the Handler, they are completely swallowed. All that is provided is a StateChange with the new state of failed.

The errors are returned by both the messageLoop (seen here) and the Handler (seen here), but they are ignored by the Start function (seen here).

Here is an example of the log output:

INFO [03-12|10:36:28.892|message/msg_main.go:164] Change handler                           self=server-dkg msgType=0 fromId=client-dkg oldType=0 newType=1
INFO [03-12|10:36:28.892|message/msg_main.go:164] Change handler                           self=server-dkg msgType=1 fromId=client-dkg oldType=1 newType=2
INFO [03-12|10:36:28.893|message/msg_main.go:164] Change handler                           self=server-dkg msgType=2 fromId=client-dkg oldType=2 newType=3
INFO [03-12|10:36:28.895|message/msg_main.go:176] State changed                            self=server-dkg old=Init new=Done
2024/03/12 10:36:28 DKG state changed old Init new Done
INFO [03-12|10:36:36.336|message/msg_main.go:164] Change handler                           self=server-reshare msgType=0 fromId=client-reshare oldType=0 newType=1
INFO [03-12|10:36:36.336|message/msg_main.go:176] State changed                            self=server-reshare old=Init new=Failed

What you expected to happen:

When OnStateChanged is called, it'd be great if the argument passed to the OnStateChange contained both the newState and an error (if there is one).

Alternatively, the Start() function could return the error by passing an error channel to messageLoop and surfacing this error through the subsequent chain of functions (including the handler).

Either of these would allow for errors to be bubbled all the way up to the calling application.

markya0616 commented 4 months ago

Thanks for the feedbacks. The second solution is better for me. Can you create a PR for it?