fix: didcomm message handler should attempt to pass message to other handlers #1064
Conversation
| // shouldn't throw an error if other handlers fail | ||
| let superHandled | ||
| try { | ||
| superHandled = await super.handle(message, context) |
There was a problem hiding this comment.
What I'm seeing here is that a message is parsed by this handler and it could be returned directly (which would also emit the validatedMessage event) but it can also be forwarded to other handlers in case they might be able to extract more value from it.
I like this idea a lot.
I wonder how we can incorporate it as the default way to handle all messages at some point.
Maybe message handlers can flag things as "good enough" so that the top-level handler knows not to throw an error at the end of the pipeline.
__tests__/shared/messageHandler.ts
Outdated
| @@ -59,6 +59,7 @@ export default (testContext: { | |||
| const allMessages = await agent.dataStoreORMGetMessages() | |||
| const count = await agent.dataStoreORMGetMessagesCount() | |||
| expect(allMessages.length).toEqual(count) | |||
| expect(count).toEqual(1) | |||
There was a problem hiding this comment.
This creates some coupling between the tests and the order in which they are executed.
Perhaps the expectation should be that the count is not zero, if another test in this suite produces a message, but not give it a specific value in case other suites create messages of their own.
| recipientDidUrl: sender.did, | ||
| }) | ||
| const messages = await agent.dataStoreORMGetMessagesCount({}) | ||
| expect(messages).toEqual(2) |
There was a problem hiding this comment.
testing message flow by count is prone to some weird test failures when multiple tests produce messages.
It would be better to use message IDs, or something else to check if they get processed correctly.
What issue is this PR fixing
fixes #944
What is being changed
DIDComm Message Handler should attempt to forward message to downstream handlers (but should not fail if none handle message)
Quality
Check all that apply:
yarn,yarn build,yarn test,yarn test:browserlocally.