Skip to content
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

Can webhooks be generalized? Or a different solution? #327

Closed
dbluhm opened this issue Jan 15, 2020 · 3 comments · Fixed by #1063
Closed

Can webhooks be generalized? Or a different solution? #327

dbluhm opened this issue Jan 15, 2020 · 3 comments · Fixed by #1063
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@dbluhm
Copy link
Contributor

dbluhm commented Jan 15, 2020

While writing the plugin for the toolbox, I'm finding that being able to tap into the information emitted to webhooks without having to have a separate web server running to receive it would be very useful.

Would it be possible to generalize the send_webhook system to more of a notify or observer style system of which the webhook sender is just one consumer?

To talk through my use case in a little more detail, I would like to be able to asynchronously emit a DIDComm message to the toolbox connection when a credential state changes, update a toolbox-plugin-specific record when a credential is issued or a presentation received, etc, etc. From a plugin angle, I might be able to accomplish the same goal by supplying my own AdminServer implementation but tapping into notifications emitted by the agent core feels cleaner and is closer to my intent; however, I am open to other solutions.

I'm happy to make contributions to see this implemented if we find it a good idea.

@dbluhm dbluhm added enhancement New feature or request question Further information is requested labels Jan 15, 2020
@dbluhm
Copy link
Contributor Author

dbluhm commented Feb 17, 2021

This recently became a priority for us for improving the flow of messages to the toolbox and other agents utilizing the toolbox protocols. I have some preliminary stuff that I'm feeling pretty good about but I've run into something that feels quirky and I'm not sure whether it is expected behavior or not.

Consider the send_webhook method in BaseRecord:

    async def send_webhook(
        self, session: ProfileSession, payload: Any, topic: str = None
    ):
        """Send a standard webhook.

        Args:
            session: The profile session to use
            payload: The webhook payload
            topic: The webhook topic, defaulting to WEBHOOK_TOPIC
        """
        if not payload:
            return
        if not topic:
            topic = self.webhook_topic
            if not topic:
                return
        responder = session.inject(BaseResponder, required=False)
        if responder:
            await responder.send_webhook(topic, payload)

I've been hooking in at roughly this point and trying things out and I've discovered that session.inject(BaseResponder) will return something different from session.profile.inject(BaseResponder). I wanted to be able to use the profile from this session and subsequently create a new session but the difference in the responders means that messages are not being encrypted. I believe this is because the session.profile.inject(BaseResponder) is returning the AdminResponder created on startup rather than the Responder associated with the (Admin) Request Context.

Is this expected behavior?

Mentioning @andrewwhitehead in case this issue has fallen in between the sofa cushions lol. Thanks in advance.

@dbluhm
Copy link
Contributor Author

dbluhm commented Feb 18, 2021

After some additional testing, this is not always the case. When send_webhook is triggered by the root context (such as creating an invitation on startup and a new connection record is created), both the session and the profile have the same responder, the AdminResponder from the root context. When it is triggered by a message handler, session.inject(BaseResponder) returns a DispatcherResponder and session.profile.inject(BaseResponder) returns the same AdminResponder from the root context.

@dbluhm
Copy link
Contributor Author

dbluhm commented Apr 27, 2021

This issue is addressed by #1063.

@dbluhm dbluhm linked a pull request Apr 27, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants