Botkit: convo.ask and convo.addQuestion is not waiting for response in latest version

[gcfabri]

bot.createConversation(message, (err, convo) => {
  convo.addQuestion('is it a bug?', (response, convo) => {
    if (response.text) {
      convo.say('No');
      convo.next();
    } else {
      convo.say('Yes: ' + response.text);
      convo.next();
    }
  });
  convo.activate();
});

Output: Yes:

The question appears then the 'else' condition is satisfied without waiting for a response.

[benbrown]

On what messaging platform are you experiencing this issue?

Can you share some code that exhibits this behavior?

[gcfabri]

Just faced with Facebook Messenger. Not tested with other platform.

[benbrown]

@gcfabri Can you upgrade to 0.6.2 and see if your issue is resolved?

[gcfabri]

@benbrown Tested with 0.6.2, same behavior. Now I am working with version 0.5.8 and this not happens.

[benbrown]

Very strange. I just tested your code _exactly_ with a bot on Facebook running 0.6.2 and it worked as expected - waiting for user input, and responding with "No."

What webhook event subscriptions does your bot have? Perhaps some _other event_ is being received that causes the conversation to move forward improperly.

[Taichou]

Hello guys,
I'm facing the same problem with a bot on Facebook (running 0.6.2) and I'm thinking the conversation is triggered by the message_delivery.
So, I tried reproducing it with a straightforward bot, and change the require_delivery value:

const Botkit = require("botkit");
const controller = Botkit.facebookbot({
  stats_optout: process.env.BOTKIT_OPTOUT,
  validate_requests: process.env.BOTKIT_VALIDATE_REQUEST, 
  access_token: process.env.FB_PAGE_TOKEN,
  verify_token: process.env.FB_VERIFY_TOKEN,
  app_secret: process.env.FB_APP_SECRET,
  debug: true,
  require_delivery: true
});

const bot = controller.spawn({});

controller.setupWebserver(process.env.PORT || 3000, function(err, webserver) {
  controller.createWebhookEndpoints(controller.webserver, bot, function() {
    console.log("This bot is online!!!");
  });
});

controller.hears(["hello"], "message_received", function(bot, message) {
  bot.reply(message, "Hey there.");
});

controller.hears(["question"], "message_received", function(bot, message) {
  bot.createConversation(message, (err, convo) => {
    convo.addQuestion("is it a bug?", (response, convo) => {
      if (response.text) {
        convo.say("No");
        convo.next();
      } else {
        convo.say("Yes: " + response.text);
        convo.next();
      }
    });
    convo.activate();
  });
});

• In case require_delivery=false:
  -- Whenever the user sends Hello I get the reply. (Just a check).
  -- Sending "question" leads to reproducing @gcfabri's issue
  screen: Not waiting

• In case require_delivery=true:
  -- Hello still working.
  -- Sending "question" stops everything, even sending a hello afterward doesn't work:
  screen: No answer
  I still receive the messages server-wise but no message is sent:

  debug: RECEIVED MESSAGE
  debug: CUSTOM FIND CONVO 1** 1**
  debug: FOUND EXISTING CONVO!
  debug: HANDLING MESSAGE IN CONVO

  then nothing happens.

Subscriptions wise this basic bot had every and any possible subscription (I didn't give it too much thought):

Selected events: messages, messaging_postbacks, messaging_optins, message_deliveries, message_reads, messaging_payments, messaging_pre_checkouts, messaging_checkout_updates, messaging_account_linking, messaging_referrals, message_echoes, standby, messaging_handovers, messaging_policy_enforcement

Not sure if all this helps, in a nutshell: I reproduce the same bug :p

[vnxdev]

I'll add my 5 cents.
When using 0.5.8 I get this debug message:
debug: Successfully subscribed to Facebook events: {"success":true}

When using the latest version I don't see it.

Also, look at this sample code:

controller.on('message_received', (bot, message) => {
  bot.createConversation(message, (err, convo) => {
    convo.addQuestion('What is your name?', (response, convo) => {
      convo.say('Your name is ' + response.text)
      convo.next()
    })
    convo.activate()
  })
})

I'm not quite sure if it was fixed already or if it's directly related to this issue, but when using 0.5.8 and validate_requests: true, I get error:

info: ** Invalid X-HUB signature on incoming request!
debug: ** X-HUB Validation Error: { TypeError: Key must be a buffer
    at new Hmac (crypto.js:115:16)
    at Object.Hmac (crypto.js:113:12)
    at getSignature (../node_modules/botkit/lib/Facebook.js:741:27)
    at verifyRequest (../node_modules/botkit/lib/Facebook.js:732:26)
    at ../node_modules/body-parser/lib/read.js:104:9
    at invokeCallback (../node_modules/raw-body/index.js:224:16)
    at done (../node_modules/raw-body/index.js:213:7)
    at IncomingMessage.onEnd (../node_modules/raw-body/index.js:273:7)
    at emitNone (events.js:105:13)
    at IncomingMessage.emit (events.js:207:7)
  expose: true,
  statusCode: 403,
  status: 403,
  body: <Buffer 7b 22 6f 62 6a 65 63 74 22 3a 22 70 61 67 65 22 2c 22 65 6e 74 72 79 22 3a 5b 7b 22 69 64 22 3a 22 32 37 38 38 36 39 39 35 39 32 36 34 31 34 37 22 2c ... >,
  type: 'entity.verify.failed' }

When I remove validate_requests: true conversation works fine. The same code on the latest version doesn't show any errors but reproduces the bug stated in this issue.

[Taichou]

Actually.... I'm not sure it's a bug :p ! I think I got it:

it's triggered by "message_delivered" and "message_read" both contain a text field that's empty which is a falsy value, so it goes into the "else" and ends the conversation with a "Yes".
And even if they didn't contain the text field, it will have the same result since null and undefined are both falsy values too.

The code should be something like this:

    convo.addQuestion("is it a bug?", (response, convo) => {
      if (response.text) {
        convo.say("No");
        convo.next();
      } else {
        console.log(
          "Not a bug: we've just received a " +
            response.type +
            ". So, no convo.next() here if we wanna still wait for an answer"
        );
      }
    });

When I run this code I get this log:
Not a bug: we've just received a message_read. So, no convo.next() here if we wanna still wait for an answer

So, yea, it did wait for an answer just not the one we're expecting.

Or I guess you can also unsubscripe to the webhooks message_deliveries, message_reads (not what I would do though).

That being said, this runs if _require_delivery=false_ in case it's true there is no answer but I'm guessing that's another issue (see here: #643 ).

[Taichou]

Oh and I've been wondering why the response.text is an empty string in my console log while it's not even defined in the debug log "debug: HANDLING MESSAGE IN CONVO ...".

It's the capture cb that adds the text (comes after the botkit.debug):

botkit.middleware.capture.run(that.task.bot,` response, that, function(err, bot, response, convo) {
                if (response.text) {
                    response.text = response.text.trim();
                } else {
                    response.text = '';
                }

(in CoreBot.js)

And I still have no idea why/how the same code worked in 0.5.8... (if I got something wrong, please tell me, it'll help)

[vnxdev]

Can confirm, I rolled back to 0.5.8, set require_delivery: false and validate_requests: false and now everything works like a charm

[Taichou]

Ok, I checked the 0.5.8 and I think I get the difference:

In the 0.5.8:
The "facebook_botkit.handleWebhookPayload handles" the request and calls the receiveMessage if it contains a message, otherwise if it contains optin, delivery, read... we trigger the appropriate bot 'action'.
The "find convo" only comes later on in the reveiveMessage, so only in the case of the message really containing a 'message'.

In the 0.6.2:
In the "categorize step", facebookBot uses a handleOptIn to define the type of the message:

  // handle message sub-types
  facebook_botkit.middleware.categorize.use(function handleOptIn(
    bot,
    message,
    next
  ) {
    if (message.optin) {
      message.type = "facebook_optin";
    }
    if (message.delivery) {
      message.type = "message_delivered";
    }
    if (message.read) {
      message.type = "message_read";
    }
    if (message.referral) {
      message.type = "facebook_referral";
    }
    if (message.account_linking) {
      message.type = "facebook_account_linking";
    }
    if (message.is_echo) {
      message.type = "message_echo";
    }

    next();
  });

and then the cb calls receiveMessage :( (so, in all cases):

```javascript
botkit.categorize = function(bot, message) {
message._pipeline.stage = 'categorize';
botkit.middleware.categorize.run(bot, message, function(err, bot, message) {
if (err) {
console.error('An error occured in the categorize middleware: ', err);
return;
}

        botkit.receiveMessage(bot, message);
    });
};

and of course the receiveMessage, looks for a convo and doesn't call the appropriate trigger if convo is found:

  ```javascript
  botkit.receiveMessage = function(bot, message) {
        message._pipeline.stage = 'receive';
        botkit.middleware.receive.run(bot, message, function(err, bot, message) {
            if (err) {
                console.error('An error occured in the receive middleware: ', err);
                return;
            } else {
                botkit.debug('RECEIVED MESSAGE');
                bot.findConversation(message, function(convo) {
                    if (convo) {
                        convo.handle(message);
                    } else {
                        botkit.trigger(message.type, [bot, message]);
                    }
                });
            }
        });
    };

A quick fix would be to modify this last function to check for convos only if we're in a message_receive type of message, so:

`if (convo && message.type =="message_received")

It works in this case but not sure what will happen for postbacks (I'm not even sure what we want for postbacks).

Again, not sure at all if this helps in any way, It just happens that I was trying to understand the code better and stumbled on all this, so I'm posting my analysis in case it helps save some Debug time for others.

[benbrown]

@Taichou good investigation!

I believe the solution is to _exclude_ the message_delivered events from conversations. This will allow the other event types to be included.

Are there other event types that should ALWAYS be excluded from a conversation?

[benbrown]

Or vice versa - are message_received and facebook_postback the only events we DO want included?

[ouadie-lahdioui]

Yes, i think we should include only message_delivered and message_read events into conversations and ignore everything else. i guess it is still possible to handle other events separately even that the bot wait for an answer !

This is the list of all available fb events

[Taichou]

Hi guys,

Yea I'd definitely go for an "include only" solution, and I've been thinking about what to include.

But before that, reading your two comments I guess we should first clear what we mean by exclude or include (since It subject to confusion, to me at least). Starting from this code:

              bot.findConversation(message, function(convo) {
                  if (convo) {
                      convo.handle(message);
                  } else {
                      botkit.trigger(message.type, [bot, message]);
                  }
              });

My understanding of "include only (type_A)" means that the said (type_A) would be handled by the convo (if any) and thus would not launch the trigger event.
aka:

if (convo && message.type == type_A) { ... convo.handle ... } else { ... trigger ... }

Ok, based on this I'll definitely include message_received (duh :p ! or we won't be having any conversation at all).

I would definitely not include message_delivered and message_read or the corresponding events will never be triggered in the case of a conversation and we'll be stuck with conversation never delivered (which is the origin of this issue).
@ouadie-lahdioui I'm guessing u meant exclude (or I could have understood something wrong)

As for The facebook_postback I was very tempted to include it, but then there is this bit of code:

  // For backwards-compatability, support the receive_via_postback config option
  // this causes facebook_postback events to be replicated as message_received events
  // allowing them to be heard without subscribing to additional events
  if (facebook_botkit.config.receive_via_postback) {
    facebook_botkit.on("facebook_postback", function(bot, message) {
      facebook_botkit.trigger("message_received", [bot, message]);
    });
  }

(facebook.js 0.6.2 line 42)

So, it means that if we include facebook_postback it will never trigger this "message_received" if a convo exists and thus break backward compatibility in the case receive_via_postback.
But again if we don't include facebook_postback it means that a conversation will never 'hear' postbacks :( (which is sad)...

We can imagine the same scenario happening with message_echo in the future (which was not handled in 0.5.8)

After all this, I started to get quite the headache :p, we can't handle every case alone, so I'm suggesting a very basic approach "conversations should handle only received messages" if the user wants more, he can listen to an event trigger and redirect to the conversation So, I'd advise changing the CoreBot.js receiveMessage to:

    botkit.receiveMessage = function(bot, message) {
        message._pipeline.stage = 'receive';
        botkit.middleware.receive.run(bot, message, function(err, bot, message) {
            if (err) {
                console.error('An error occured in the receive middleware: ', err);
                return;
            } else {
                botkit.debug('RECEIVED MESSAGE');
                bot.findConversation(message, function(convo) {
                    if (convo && message.type =="message_received") {
                        convo.handle(message);
                    } else {
                        botkit.trigger(message.type, [bot, message]);
                    }
                });
            }
        });
    };

and for the postbacks, in facebook.js changing:

  if (facebook_botkit.config.receive_via_postback) {
    facebook_botkit.on("facebook_postback", function(bot, message) {
      facebook_botkit.trigger("message_received", [bot, message]);
    });
  }

to

  if (facebook_botkit.config.receive_via_postback) {
    facebook_botkit.on("facebook_postback", function(bot, message) {
      message.type= "message_received";
      facebook_botkit.receiveMessage(bot, message);
    });
  }

that way, the postback will first trigger a postback event (even if a conversation exists) then be treated as a message received and go into the machine again to be handled by either a convo or a trigger.

and if the user wants to handle more cases in his conversation, well he can always listen for the specific event and call .receiveMessage.

I'm not sure if it's the best approach but it's all I could come up with for now, and I'm open to suggestions ^^.

PS: I was also thinking of doing the "message_received" check before the bot.findConversation which will save some milliseconds but it's not exactly the same thing and some users may want to do something special if no conversation is found and even if it's not a message_received... so I'm leaving it at that...

PS2: I haven't tested these suggestions, it's all theoretical, it looks good to me but you never know... (Murphy and all :p )

[vnxdev]

Any updates on this?

[vnxdev]

@benbrown Sorry to bother, but any plans on making updates on this issue?

[jsalwen]

this may be related to the #1212 issue

[gcfabri]

Please, any updates on this?

[vnxdev]

Seems like Botkit is Slack only now
Don't use it for any other platform or you'll have to wait for months for something like this to be fixed.

[benbrown]

Thanks for your patience, folks. I'll include a fix for this issue in the next release.

[mralanlee]

Thanks for the update @benbrown