botbuilder-python
botbuilder-python copied to clipboard
microsoft
Constructor fails to properly create TurnContext
Version
What package version of the SDK are you using. 4.14.1
Describe the bug
Constructing a new TurnContext from an existing TurnContext doesn't create a valid copy. Consider the following example:
To Reproduce
Steps to reproduce the behavior:
the following code will throw ConnectorClient error
newTurnContext = TurnContext(myTurnContext)
newTurnContext.activity.apply_conversation_reference(myConversationReference)
newTurnContext.send_activity(message)
while the following code will send a message correctly
newTurnContext = myTurnContext
newTurnContext.activity.apply_conversation_reference(myConversationReference)
newTurnContext.send_activity(message)
Expected behavior
TurnContext constructor should create a valid copy that can [among other things] send_activity correctly
Screenshots
n/a
Additional context
using apply_conversation_reference as recommended by proactive messaging example.
Hello @master0v, thanks for reaching out.
If you want to copy a TurnContext object we would recommend something like copy or deepcopy (depending your needs) over this method.
copy is insufficient, and deepcopy fails for the same reason the constructor fails (with the same exception). I believe the reason for that is a poor class design with references to global resources.
@master0v the idea is that the TurnContext is a transitory object that simply gives you immediate access to some of the facilities of the runtime.
The idea is the TurnContext is created by the Adapter - its really the programming model the application logic is presented with. It is, most significantly, how the application is given the inbound Activity and how the application sends the outbound Activity. Its also how you might save some conversation state that is associated with that particular inbound Activity.
As such there is no point in having a stand along TurnContext, so no point in creating one in isolation. It is only ever something that comes from the Adapter. What is meaningful to copy is the Activity it carries and potentially a ConversationReference derived from that Activity.
There is a slightly odd corner case where there is no inbound Activity - this is the proactive scenario. Here the Adaptive creates a (perhaps slightly fake) "Event" Activity and then creates a TurnContext around that. The point of this slight contrived arrangement is that the application basically sees the same programming model as if an Activity had arrived.
Tbh I'm not sure we would do this design again like this, nor am I sure this is a great model for Python, anyhow it is consistent with the other languages and self consistent with the non-proactive case.
Some more modern APIs also try and avoid classes like TunrContext that simply exist as mechanism, for example, in .NET teh observation has been made that with a better use of Dependency Injection the introduction of the TurnConext could have been avoided altogether.
Hope that makes sense.
@johnataylor thanks for a detailed and honest writeup. I am having a problem with this in exactly the "proactive" use case. Could you please perhaps add some code example, or point me to one that creates an "Event" Activity and then creates a TurnContext around that, that you are referring to?
@master0v we have a sample in C# here https://github.com/microsoft/BotBuilder-Samples/tree/main/samples/csharp_dotnetcore/16.proactive-messages
We added this sample to replace an earlier more complex one that got somewhat lost in building a contrived scenario. I say "somewhat lost" meaning almost all the sample code was just about contriving the scenario rather than simple illustration of the necessary APIs from the Bot Framework.
In writing the samples we tried to strike a balance between showing something fairly realistic and illustrating the API calls. Mostly I think we achieved this, but for proactive it is very difficult because so much depends on the external system: really proactive is almost all about the external system's behavior. In the proactive case we basically gave up on the idea of a realistic scenario and just illustrated the APIs with as little code as possible, we needed something to happen so we added a second "notify" endpoint. You presumably wouldn't have that in a realistic scenario, you'd have something else, but an endpoint was intended to be illustrative, perhaps that isn't explained very well in the sample now I look at it.
Specifically, the idea the original architects of Bot Framework had of proactive messaging is that the goal for the framework user should be to add their business logic to an instance of IBot. This means at runtime the IBot instance's OnTurn function is called, at which point it can spring into life and send Activities. (Note we added a useful abstract class called ActivityHandler that implements the IBot interface.) Specifically, OnTurn is called with a TurnContext that has been correctly set up such that when you call turnContext.Send with an Activity it goes to the right conversation. It also means if you get a conversation state handle from this turnContext and use it, the state will be appropriately correlated with the conversation. In an attempt to make things consistent the TurnContext you get given actually still has an Activity on it - except it's an Event Activity rather than the more typical Message Activity. This is very arbitrary and just a mechanism, nothing more. It's not hard to imagine a similar design but one that simply made the Activity property on the TurnContext null. To be fair to the design, the contrived or fake Event Activity avoids a bunch of special cases later in the processing, the downside, however, is that introducing the fake Activity is confusing and that's not good, simply having null would probably have been more self explanatory, at least in the sense that there was no inbound Activity because there was no inbound network call. For me, it's hard to be too critical of the fake Activity, after all null is rarely a good idea. The decision to include the IBot interface and TurnContext in the proactive case has itself been a source of a lot of confusion, I'm not leaping to its defense but I would observe the design has made a couple of significant things (sending Activities and saving state) consistent.
Anyhow, in order to get the Adapter to create this Event Activity and call the OnTurn on the IBot, something needs to be done to the Adapter. This is where the ContinueConversation call comes in. The external system must have saved out a ConversationReference from a previous inbound Activity and then the external system, when it wants some proactive behavior, pulls this ConversationReference out of its database and calls the ContinueConversation with it. In the sample, the role of the external system is modeled as the NotifyController and the role of the database is modeled by the shared ConcurrentDictionary of ConversationReferences.
You're probably going to say you wanted a sample in Python. This should be https://github.com/microsoft/BotBuilder-Samples/tree/main/samples/python/16.proactive-messages