Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

conversationState with StorageKey containing "[object Object]" when using conversation properties #1849

Closed
etiennellipse opened this issue Nov 6, 2019 · 6 comments
Assignees

Comments

@etiennellipse
Copy link
Contributor

We are using the web adapter, and in order to pass along our User entity along the handlers inside the bot, we put it inside the conversation.properties which can contain custom properties, as documented here https://github.com/microsoft/botbuilder-js/blob/master/libraries/botframework-schema/src/index.ts#L146

So far, so good. Although I would like a simpler way of accessing my user entity all along the middlewares and handlers, but that is another story.

So my problem here is that when saving the conversationState, Botkit takes all the fields inside the conversation object and creates a key with them. See: https://github.com/howdyai/botkit/blob/master/packages/botkit/src/conversationState.ts#L27

My storage key looks like: websocket/conversations/7-[object Object]-7/

So, whenever the properties field is used, there will always be a [object Object] token in the key...

I think building the key this way can lead to some unpredictability, so I would suggest that the key should be built with specific fields in a way that, for example if I remove or add properties in between releases of my chatbot it won't break the conversation storage.

Context:

  • Botkit version: 4.6
  • Messaging Platform: Web
  • Node version: v10.16.3
  • Os: MacOS / Linux
@benbrown
Copy link
Contributor

Ah! Interesting problem, and good point.

The reason we use all the keys is that some platforms (slack) take 4 or 5 pieces of info to properly define the conversation, and some only 1.

I don't really want to have to make the state driver platform-aware.

How about always excluding the properties field from the key?

On the other subject, you can theoretically pass any fields you want down the web adapter and they will end up in the resulting message object in your botkit app.

@benbrown benbrown self-assigned this Nov 11, 2019
@stale
Copy link

stale bot commented Jan 10, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 10, 2020
@etiennellipse
Copy link
Contributor Author

Excluding the properties field can be a good solution!

@stale stale bot removed the stale label Jan 10, 2020
@stale
Copy link

stale bot commented Mar 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 11, 2020
@etiennellipse
Copy link
Contributor Author

Not stale, still an issue.

@stale stale bot removed the stale label Mar 11, 2020
@benbrown
Copy link
Contributor

Fixed in the next release -> #1929

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants