Botbuilder-dotnet: MS Teams channel is giving exception when Comsmos DB is used for state managment in Enterprise Bot Template V4

[sw-ms-roshanparmar]

Github issues should be used for bugs and feature requests. Use Stack Overflow for general "how-to" questions.

Version

Microsoft Bot Builder 4.1.5 , C#

Describe the bug

Description

Currently I am using Enterprise Bot Template v4 SDK C# for developing the bot. I am using Cosmos DB for user state management. Everything works fine with enterprise bot template , but when configure the bot on MS Teams channel , at that point of time Bot fails to write the state data to Cosmos DB and gives an error

What is actually happening..
Cosmos DB(document DB) handles every individual row/data uniquely by "id" column. Cosmos DB is having a limitation of the length of the "Id" which is 255 characters.
Now, when bot tries to store the user state for MS Teams channel in cosmos db , at that point of time Enterprise bot template/ bot framework v4 automatically creates an unique id which has length more than 255 characters. So this gives an exception in code.

Please someone look into this issue as this is a blocking issue for ms teams.

As a workaround of this issue i managed to store the state in blob storage and deployed my bot.

To Reproduce

Steps to reproduce the behavior:

1. Create a bot with bot builder SDK 4.0.7 and with enterprise bot template
2. configure cosmos db for state management
3. deploy it on azure and enable MSTeams channel
4. try to get and set something in the private conversation state.
5. here you will get an exception in your startup.cs exception handler.

Expected behavior

It should save the state.

Exception call stack

Message: {"Errors":["The input name 'msteams2fconversations2fa:1de_GDChOYejATRlAuinJGMmbe2YZQ3SdEB7EPvQ_WOIUSTuU64GB5HP9BX8ol2oo1WPck-s-RpJWbMo3X9fayx-OFmyF3LnBgs7jDYj79WniZvCMqbZZy8M5RPLh2MXm2fusers2f29:1RNNKZBhoBQm0hQVbvY5NwyFWEsXABeNQUxZEHPwUYkjfn_whINgbuk6cnw21K1zFSF_ozHXBkBRJfXTLfMgbXA' is invalid. Ensure to provide a unique non-empty string less than '255' characters."]}
ActivityId: ad22f7b1-1572-406f-9440-b77f328239a0, Request URI: /apps/c8f5225c-4725-4992-9a92-6dd3f3d1510b/services/82820cba-3446-437f-8667-0a3eac3be57f/partitions/db603d54-4640-4e74-a0bc-aa19ce2198a3/replicas/131862185541974409p/, RequestStats:
RequestStartTime: 2018-11-29T12:08:33.5809299Z, Number of regions attempted: 1
, SDK: Microsoft.Azure.Documents.Common/2.1.0.0, Windows/10.0.17134 documentdb-netcore-sdk/1.9.1

[bug]

[jwiley84]

Closing as a duplicate of #27

[dmvtech]

Reopening here as this is the better repo to address the issue.

[dmvtech]

Confirming that this does look like it is a bug.

Azure Cosmos DB does indeed have a limit of 255 bytes for Row Keys:
https://docs.microsoft.com/en-us/azure/cosmos-db/faq#table

It was previously implimented in v3 to truncate the key:
https://github.com/Microsoft/BotBuilder-Azure/blob/master/CSharp/Library/Microsoft.Bot.Builder.Azure/DocumentDbBotDataStore.cs#L300

It does not look like this is implimented in v4:
https://github.com/Microsoft/botbuilder-dotnet/blob/master/libraries/Microsoft.Bot.Builder.Azure/CosmosDbStorage.cs

[sw-ms-roshanparmar]

Thanks @dmvtech for the details. Is there any workaround which i can implement until this is fixed in new version of bot builder sdk.

[dmvtech]

@sw-ms-roshanparmar Not that I know of at this point. I will update if I come up with one.

[dmvtech]

Hi @sw-ms-roshanparmar.

Sorry for the delay. Please find a workaround below that you should be able to use until this is fixed otherwise.

I wasn't able to fully test with a large conversation ID, but I just changed my MAX_KEY_LENGTH to be 50 and was able to confirm the general functionality works. What you need to do; is add a new class to your bot and inherit from the BotState class (just as the ConversationState class does here. Then, modify the GetStorageKey() method to check for large conversationId and then truncate it if so.

namespace Microsoft.BotBuilderSamples
{
    public class DMVConversationState : BotState
    {
        /// <summary>
        /// Initializes a new instance of the <see cref="ConversationState"/> class.
        /// </summary>
        /// <param name="storage">The storage provider to use.</param>
        public DMVConversationState(IStorage storage)
            : base(storage, nameof(ConversationState))
        {
        }

        internal const int MAX_KEY_LENGTH = 254;

        protected override string GetStorageKey(ITurnContext turnContext)
        {
            var channelId = turnContext.Activity.ChannelId ?? throw new ArgumentNullException("invalid activity-missing channelId");
            var conversationId = turnContext.Activity.Conversation?.Id ?? throw new ArgumentNullException("invalid activity-missing Conversation.Id");

            if((channelId == "msteams") && (conversationId.Length > MAX_KEY_LENGTH)) //not necessary to check for *just* Teams
            {
                var hash = conversationId.GetHashCode().ToString("x");
                conversationId = conversationId.Substring(0, MAX_KEY_LENGTH - hash.Length) + hash;
            }
            return $"{channelId}/conversations/{conversationId}";
        }
    }
}

Then update your Startup and Bot.cs code to change references from ConversationState to your new class and you should be good to go.

[sw-ms-roshanparmar]

Thanks @dmvtech ,

This solved my problem. However, I have to tweak the code little bit to incorporate the PrivateConverstionState Id in my implementation.

Thanks.

[cleemullins]

@sw-ms-roshanparmar I'm not sure there's an easy answer to this. The workaround that @dmvtech provided is something I was about to recommend.

The core problem is that we cons up the keys, and require them to be unique. If we truncate the key, as was done in v3, then guarantees around uniqueness are gone.

We could hash the key if Length > 255. That would probably provide a better uniqueness guarantee.

The problem is this only applies to CosmosDB. Other data providers all have different restrictions.

For now, I think the if (length >255) then hash() seems like the best overall mechanism. PR coming shortly.

[cleemullins]

@sw-ms-roshanparmar I've got PR #1370 pending that resolves this.