microsoft / botbuilder-js

Welcome to the Bot Framework SDK for JavaScript repository, which is the home for the libraries and packages that enable developers to build sophisticated bot applications using JavaScript.
https://github.com/Microsoft/botframework
MIT License
685 stars 281 forks source link

With bot framework sdk 4 and node.js the IIS server caches object UserState for all user sessions #1240

Closed ramcaltech2019 closed 5 years ago

ramcaltech2019 commented 5 years ago

With bot framework sdk 4 and node.js the IIS server caches object UserState, this object is used to store customer information, the first user will set this object with information, the second user that access the bot will have this UserState object populated with the information of the first user, the only way to remove this information is to reset IIS server that host the bot.

In an effort to resolve this issue the code was change not to inject the MemoryStorage object (see code below) in Index.js file:

const memoryStorage = new MemoryStorage(); conversationState = new ConversationState(memoryStorage); userState = new UserState(memoryStorage);

did not injected the memoryStorage object (see code below) but it does not resolve the IIS server cache problem.

conversationState = new ConversationState(); userState = new UserState();

The resolution may reside on a setting in the web.config or in code?

This bug prevent my bot for allow multiple concurrent users with independent sessions interact with the bot.

I am using Direct Line angular app as client to access the bot and also tried the web chat control, the IIS server still cached the userState globally for all user sessions.

stevkan commented 5 years ago

@ramcaltech2019, there appear to be a few things going on here that probably shouldn't be.

First, and this may not be an actual issue as I don't know your code or your intentions at this point, but MemoryStorage should only be used for testing. If you are planning on deploying this bot, then you should change the storage type to something persistent, such as CosmosDB, Mongo, or blob storage. Memory storage relies on just that, memory, making state volatile. Using MemoryStorage is unlikely the culprit here, but I wanted you to be aware.

Second, can you explain how (and why) you are using IIS to run your bot? IIS is not necessary. The bot, as a Node project, can run independently in a local environment. If you need to connect outside services (i.e. LUIS ,QnA, graph APIs, etc.), then you can connect via ngrok or relays to create a tunnel by changing the messaging endpoint in your Azure Bot Service bot to the respective endpoint either ngrok or relays provides.

ramcaltech2019 commented 5 years ago

@stevkan I developed a bot with nodejs using the bot framework and published to azure which is running nodejs in IIS, I interact with the bot through a angular page, the bot its based on a sample code for nodejs with the dispatcher, it is an ecommerce bot, during the conversation I saved to UserState object the name, phone number etc from the customer, if someone else access the bot it sees the information from the first customer (I send some of the userState properties to the console), If I have two concurrent customers accessing the bot, the userState properties changes for both customers, it seems like its sharing one UserState object for multiple concurrent users, that is a bug, each concurrent user should have his own instance of UserState, maybe my problem is that I am using the wrong object and the solution is to use nodejs variables instead, when I reset my azure bot hosting in IIS all the values in UserState are clear, but they are not even if the user refresh the page

stevkan commented 5 years ago

This doesn't appear to be a bug. I suspect you are passing a userId to Direct Line which, in this case, is the same userId when anyone connects to the page / bot. If this is true, then that would explain why details between users are being shared. The bot thinks it is conversing with the same user despite different instances.

Are you using the direct line or web chat secret to connect web chat to your bot or exchanging for a token? And, if exchanging, how are you doing so? Are you able to share your code for web chat (and for generating the token, if applicable)? I would like to see how userId is being passed in and where.

ramcaltech2019 commented 5 years ago

Hi, this makes sense yes I am using direct line with the angular sample app, I created also a direct line channel for the bot using the azure portal and got one of the secret keys and passed from the ng app, here is the code of the angular app, I passed in the postactivity a cookie token to welcome the user and the time zone, I use azure blob storage to store customer info, name etc, that's the reason of this code. Yes direct line always identify it self with the same secret key. The goal with this chat is to be accessible to customers with no personal authentication but the chat should be able to handle individual conversations, it this possible with direct line?
Let me know if you need additional info. Thank you very much for expert advise.

import { Component, ElementRef, OnInit, ViewChild } from "@angular/core";

/**
 * Declares the WebChat property on the window object.
 */
declare global {
    interface Window {
        WebChat: any;
    }
}

window.WebChat = window.WebChat || {};

@Component({
    selector: "app-root",
    templateUrl: "./app.component.html",
    styleUrls: ["./app.component.scss"]
})
export class AppComponent implements OnInit {
    @ViewChild("botWindow") botWindowElement: ElementRef;

    public ngOnInit(): void {
        const directLine = window.WebChat.createDirectLine({
            secret: "<secret token>",
           webSocket: false
        });

        window.WebChat.renderWebChat(
            {
                directLine: directLine,
                userID: "uvschatbot-003"
            },
            this.botWindowElement.nativeElement
        );

        directLine
            .postActivity({
                from: { id: genUniqueId() },
                localTimestamp:new Date(),
                localTimestamp1: new Date().toTimeString().slice(9),
                text: "requestWelcomeDialog",
                type: "event",
                value: "token"
            })
            .subscribe(
                id => console.log(`Posted activity, assigned ID ${id}`),
                error => console.log(`Error posting activity ${error}`)
            );

        function genUniqueId() {
               var user = checkCookie();
               if (user === ""){
                var user = "ChatBotUser" + Math.random().toString().replace('0.','');
                setCookie("uvs", user, 30);
                return user;
               }

               return user;
            }

            function setCookie(cname, cvalue, exdays) {
                var d = new Date();
               d.setTime(d.getTime() + (exdays*24*60*60*1000));
               var expires = "expires="+ d.toUTCString();
               document.cookie = cname + "=" + cvalue + ";" + expires + ";path=/";
             }

             function checkCookie() {
                var user = getCookie("uvs");
                console.log(user);

                return user;

              }

              function getCookie(cname) {
                var name = cname + "=";
                 var ca = document.cookie.split(';');
                for(var i = 0; i < ca.length; i++) {
                  var c = ca[i];
                  while (c.charAt(0) == ' ') {
                    c = c.substring(1);
                  }
                  if (c.indexOf(name)  == 0) {
                    return c.substring(name.length, c.length);
                   }
                }
                 return "";
              }
    }
}
stevkan commented 5 years ago

The issue is in your direct line object. As I mentioned, you need to randomize your userId. You can use any method you want, but as long as you keep passing the same value in, then every set of users who connect concurrently to your bot will have shared data being passed around.

A simple fix would look like this:

const userId = `${Date.now() + Math.random().toString(36)}`;

window.WebChat.renderWebChat(
  {
    directLine: directLine,
    userID: userId
  },
  this.botWindowElement.nativeElement
);

If you are using "Enhanced Authentication" in the Direct Line channel, then you will need to pre-pend the userId value with "dl_". It should look like this:

const userId = `dl_${Date.now() + Math.random().toString(36)}`;

This should solve the issue for you. I'm going to close this ticket as resolved. If the issue persists, please feel free to reopen the issue.

Hope of help! Steve.

ramcaltech2019 commented 5 years ago

Thank you Steve will follow your instructions and let you know the outcome.

ramcaltech2019 commented 5 years ago

sorry but it did not work, the bot still showing the data of the first user to multiple users. I refactored my bot code not to use UserState object but my own class, it works fine with the bot framework emulator, I deployed the new code to azure. I am not able to test it because the direct line post activity throws an error: failed to load resource: the server responded with a status of 401 () app.component.ts:45 Error posting activity Error: ajax error 401

I did not change any security of the bot in azure, the ConversationUpdate activities are going through but not the type event that I post with the direct line client side code below:

I did read that people are getting this 401 error as well with direct line and there is a patch but for .NET bots, my bot is nodejs

directline.botframework.com/v3/directline/conversations/7DMF9bahNDueWUFdJLh2x-5/activities:1 POST https://directline.botframework.com/v3/directline/conversations/7DMF9bahNDueWUFdJLh2x-5/activities 401 (Unauthorized)

   const userId = `${Date.now() + Math.random().toString(36)}`;
    // const userId =  genUniqueId();
    console.log(userId);
    window.WebChat.renderWebChat(
        {
            directLine: directLine,
            userID: userId
        },
        this.botWindowElement.nativeElement
    );

    directLine
        .postActivity({
            from: { id:  genUniqueId()},
            localTimestamp:new Date(),
            localTimestamp1: new Date().toTimeString().slice(9),
            text: "requestWelcomeDialog",
            type: "event",
            value: "token"
        })
        .subscribe(
            id => console.log(`Posted activity, assigned ID ${id}`),
            error => console.log(`Error posting activity ${error}`)
        );
ramcaltech2019 commented 5 years ago

this is the code from the web chat samples with direct line for an angular app, I used to run fine with no permissions issue, not it does not work window.WebChat.renderWebChat( { directLine: directLine, userID: "uvschatbot-003" }, this.botWindowElement.nativeElement );

    directLine
        .postActivity({
            from: { id: "USER_ID", name: "USER_NAME" },
            name: "requestWelcomeDialog",
            type: "event",
            value: "token"
        })
        .subscribe(
            id => console.log(`Posted activity, assigned ID ${id}`),
            error => console.log(`Error posting activity ${error}`)
        );
}
ramcaltech2019 commented 5 years ago

Sorry Steve, I had a problem of reference the old object right under the activity "event" sent by direct line, the message was 401 which confused me, after I replace the right reference the problem was fixed. I found the problem after I add my own class to replace the UserState, the problem is that with require in nodejs it creates a singleton by default, that causes and issue with concurrent users, I am trying to find a way to create a multiple instances of the class one not in the singleton pattern, any help of a sample code is greatly appreciated, below is the class that I want to convert to a non-singleton class so concurrent users don't break the bot. Thank you very much for your help.

function BotUserState(name, email, phone, userId, localTime, localTimeZone, timeRange, location, lastPrompt, skipGetReadyMsg, isRecurrentUser, recurrentUserName, spanishCustomerEnterData, isSpanishCustomer, vehicleNumber){ this.name = name; this.email = email; this.phone = phone; this.userId = userId; this.localTime = localTime; this.localTimeZone = localTimeZone; this.timeRange = timeRange; this.location = location; this.lastPrompt = lastPrompt; this.skipGetReadyMsg = skipGetReadyMsg; this.isRecurrentUser = isRecurrentUser; this.recurrentUserName = recurrentUserName; this.spanishCustomerEnterData = spanishCustomerEnterData; this.isSpanishCustomer = isSpanishCustomer; this.vehicleNumber = vehicleNumber;

}

BotUserState.prototype.setName = function (name) { this.name = name; }

BotUserState.prototype.setEmail = function (email) { this.email = email; }

BotUserState.prototype.setPhone = function (phone) { this.phone = phone; }

BotUserState.prototype.setUserId = function (userId) { this.userId = userId; }

BotUserState.prototype.setLocalTime = function (localTime) { this.localTime = localTime; }

BotUserState.prototype.setLocalTimeZone = function (localTimeZone) { this.localTimeZone = localTimeZone; }

BotUserState.prototype.setTimeRange = function (timeRange) { this.timeRange = timeRange; }

BotUserState.prototype.setLocation = function (location) { this.location = location; }

BotUserState.prototype.setLastPrompt = function (lastPrompt) { this.lastPrompt = lastPrompt; }

BotUserState.prototype.setSkipGetReadyMsg = function (skipGetReadyMsg) { this.skipGetReadyMsg = skipGetReadyMsg; }

BotUserState.prototype.setIsRecurrentUser = function (isRecurrentUser) { this.isRecurrentUser = isRecurrentUser; }

BotUserState.prototype.setRecurrentUserName = function (recurrentUserName) { this.recurrentUserName = recurrentUserName; }

BotUserState.prototype.setSpanishCustomerEnterData = function (spanishCustomerEnterData) { this.spanishCustomerEnterData = spanishCustomerEnterData; }

BotUserState.prototype.setIsSpanishCustomer = function (isSpanishCustomer) { this.isSpanishCustomer = isSpanishCustomer; }

BotUserState.prototype.setVehicleNumber = function (vehicleNumber) { this.vehicleNumber = vehicleNumber; }

module.exports = BotUserState;