-
Notifications
You must be signed in to change notification settings - Fork 135
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
DIDComm message handler breaks out of message handler flow #944
Comments
We found that our original message chaining design was confusing for a lot of folks and that's why we decided to try to change it. We didn't actually get to change the behavior of the other message handlers, leading to this bug. I think the fix you propose should be good to align the behavior of the handlers, but I'd still like to come up with a better solution for all. One of the ideas was to split message handlers into parsers and handlers Parsers can be chained, just like the the handlers of today, and they should return a message as soon as a message type is decoded. Then the message with a type can be broadcast to all the handlers, which can do further processing and rebroadcast a message with a different type, if for example there's a different layer that can be decoded. This seemingly more complicated design should accommodate handlers that can handle multiple messages (like a message forwarding handler) and messages that can be interpreted by multiple handlers ( a jwt string can be both a final message as well as an envelope for another protocol) Another reason for a redesign is the fact that our chaining logic today doesn't differentiate between an invalid signature and an exceptional failure (like failing to resolve a DID while trying to verify a signature) leading to very unhelpful "Unsupported message type“ errors. Any feedback here is very welcome! |
I submitted a PR for this fix. Maybe I've just gotten used to the current method of chaining, but I think that any confusion can be dealt with in more documentation and some more examples. Some apps may only need a few handlers, if they are working with a limited scope of messages, while other apps may need more handlers to process a wider variety of messages. The broadcast/rebroadcast idea may add more complexity than is needed. I'd rather see a move to a single package for message handling, with the various handlers being within that, so each new handler doesn't require a unique package/plugin. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Once a DIDComm v2 message is unpacked, no further messageHandlers are executed. Maybe this is intended behavior, but I was expecting to use subsequent message handlers to perform more processing on the message.
The fix would be easy, changing line 124 of the DidComm message-handler.ts from "return message" to "return super.handle(message, context)".
The text was updated successfully, but these errors were encountered: