search

microsoft / botframework-sdk

Bot Framework provides the most comprehensive experience for building conversation applications.
MIT License
7.5k stars 2.44k forks source link

[Feature Request] Response sequencing issue when using Session.delay() #4225

Closed dan-cohn closed 6 years ago

dan-cohn commented 6 years ago

Bot Info

Issue Description

In certain cases, users receive responses from the chatbot in the wrong order. Specifically, this occurs when typing ahead – i.e., sending a message to the bot before seeing the response from the previous one. I discovered the cause of this issue, and it relates to the use of Session.delay() to slow down the flow of responses, typically in conjunction with sendTyping().

Consider the following sequence:

  1. ChatConnector: message received
  2. Session.sendTyping()
  3. Session.delay(2000)
  4. Session.send("First response")
  5. Session.sendTyping()
  6. Session.delay(3000)
  7. Session.send("Second response")
  8. Session.sendBatch() sending 6 message(s)
  9. ChatConnector: message received
  10. Session.sendTyping()
  11. Session.delay(1000)
  12. Session.send("Third response")
  13. Session.sendBatch() sending 3 message(s)

Assume there’s no delay between the first and second utterances from the user because they were typed quickly. Here’s what should happen:

After 0 sec – typing dots appear After 2 sec – “First response” appears After 5 sec – “Second response” appears followed by typing dots After 6 sec – “Third response” appears

Here’s what actually happens:

After 0 sec – typing dots appear After 1 sec – “Third response” appears After 2 sec – “First response” appears followed by typing dots After 5 sec – “Second response” appears

The reason for this is that the third delay isn’t queued behind the first two. The framework treats it as a fresh message and sends it immediately.

Code Example

See above for details. I also have a proposed BotBuilder code fix for this. I'd be happy to share it. I'm currently applying a patch from my application code. See attached "patches.js" file.

patches.js.zip

Reproduction Steps

  1. Type two utterances one after the other. The first utterance should elicit a response or series of responses that include delays (e.g., for pacing). The second utterance should elicit a response or responses without delays or with a smaller delay.
  2. Observe that responses arrive out of order. If you look at the logs, you can see they are also sent in the wrong order.

This is reproducible on the emulator as well.

Expected Behavior

Responses should be sent in the correct order.

Actual Results

See attached video for an example of this. First I type “help” which has a 10-second response delay (for the sake for demonstrating the issue). While the clock is ticking, I type “bye” which has a 1-second response delay. As you can see, the “bye” response comes out first and eventually the two-part help response appears. Very confusing from a user standpoint.

Msg_ordering_issue.mov.zip

EricDahlvang commented 6 years ago

We recommend bots be developed in a way that is resilient and account for users sending messages quickly in succession. Your middleware solution looks good. Thank you for sharing.

dan-cohn commented 6 years ago

Thanks, @EricDahlvang. Would you like me to make a pull request, or is this something Microsoft plans to address?

EricDahlvang commented 6 years ago

It seems to me that this is something left to the bot developer by design. It is specific to the bot implementation, and does not necessarily apply to every bot. With that in mind, it feels like it should not be in the sdk directly. What are you thoughts?

dan-cohn commented 6 years ago

My view is that it's a defect. As a bot developer, I expect the framework to issue responses in the order I send them, regardless whether the messages are sent in response to one utterance or the next. Session.delay() isn't taking into account the fact that a previous delay may still be running.

Are you saying that some bot implementations may want to send a subsequent message so it arrives before a previous one? That seems odd to me, but even if that's truly the case, then I think there ought to be a separate API called sendImmediate() or something like that.

As I see it, the problem with the current SDK is that it behaves inconsistently. If I send multiple responses at once, the delay is cumulative. If I send a batch and then later send another batch, the delays are only cumulative within each batch.

fanidamj-zz commented 6 years ago

Can you please let us know if the issue still persists?

dan-cohn commented 6 years ago

I've been using a workaround, so I'm not sure. Has a fix been added to the framework?

EricDahlvang commented 6 years ago

Sorry @dan-cohn but we have not yet added something like this to the sdk. I've labeled this as a feature request.

JasonSowers commented 6 years ago

Thank you for opening an issue against the Bot Framework SDK v3. As part of the Bot Framework v4 release, we’ve moved all v3 work to a new repo located at https://github.com/microsoft/botbuilder-v3. We will continue to support and offer maintenance updates to v3 via this new repo.

From now on, https://github.com/microsoft/botbuilder repo will be used as hub, with pointers to all the different SDK languages, tools and samples repos.

As part of this restructuring, we are closing all tickets in this repo.

For defects or feature requests, please create a new issue in the new Bot Framework v3 repo found here: https://github.com/microsoft/botbuilder-v3/issues

For Azure Bot Service Channel specific defects or feature requests (e.g. Facebook, Twilio, Teams, Slack, etc.), please create a new issue in the new Bot Framework Channel repo found here: https://github.com/microsoft/botframework-services/issues

For product behavior, how-to, or general understanding questions, please use Stackoverflow. https://stackoverflow.com/search?q=bot+framework

Thank you.

The Bot Framework Team