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

convo.repeat() in handler causes 'TypeError: Converting circular structure to JSON' in conversationState.saveChanges #1811

Closed
etiennellipse opened this issue Sep 13, 2019 · 6 comments
Assignees
Labels
Milestone

Comments

@etiennellipse
Copy link
Contributor

etiennellipse commented Sep 13, 2019

Are you sure this is an issue with the Botkit core module?

Yes

What are you trying to achieve or the steps to reproduce?

I am trying to repeat a step in a conversation. However, if a question was asked before, the repeat() method will cause a crash.

Reproduction steps:

  • Copy this skill in a web bot
  • Say "go" to fire the dialog
  • Answer the first question, for which the result will be saved in the conversation state
  • Then, say "repeat" which will fire convo.repeat() and crash
const { BotkitConversation } = require('botkit');

module.exports = function(controller) {
    const DIALOG_SUBJECT_ID = 'subject-dialog';

    const subjectDialog = new BotkitConversation(DIALOG_SUBJECT_ID, controller);

    subjectDialog.ask(`First question`, [], `first`);

    for (let i = 0; i < 30; i++) {
        subjectDialog.ask(`Message #${i}`,
            async (response, convo, bot) => {
                if (response === 'repeat') {
                    convo.repeat();
                }
            }, `conv-${i}`);
    }

    controller.addDialog(subjectDialog);

    controller.hears('go', ['message', 'direct_message'], async (bot, message) => {
        await bot.beginDialog(DIALOG_SUBJECT_ID);
    });

};

What was the result you received?

Error:

(node:97694) UnhandledPromiseRejectionWarning: TypeError: Converting circular structure to JSON
    at JSON.stringify (<anonymous>)
    at Object.calculateChangeHash (/Users/etienne/Code/botkit-example/node_modules/botbuilder-core/lib/storage.js:29:17)
    at BotkitConversationState.saveChanges (/Users/etienne/Code/botkit-example/node_modules/botbuilder-core/lib/botState.js:89:59)
    at Botkit.<anonymous> (/Users/etienne/Code/botkit-example/node_modules/botkit/lib/core.js:507:42)
    at Generator.next (<anonymous>)
    at /Users/etienne/Code/botkit-example/node_modules/botkit/lib/core.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/etienne/Code/botkit-example/node_modules/botkit/lib/core.js:3:12)
    at Botkit.saveState (/Users/etienne/Code/botkit-example/node_modules/botkit/lib/core.js:506:16)
    at Botkit.<anonymous> (/Users/etienne/Code/botkit-example/node_modules/botkit/lib/core.js:491:44)

What did you expect?

Repeat the previous message

Context:

  • Botkit version: 4.5.0
  • Messaging Platform: Web
  • Node version: 10.16.3
  • Os: MacOS

Inspecting the execution shows that in BotState.saveChanges the cached instance has a circular reference on the "first" value pointing to the "values" object, bot should contain the value of the answer given.

image

@etiennellipse
Copy link
Contributor Author

The comment #1673 (comment) refers to the same issue.

@etiennellipse
Copy link
Contributor Author

etiennellipse commented Sep 13, 2019

Investigating further.

When convo.repeat() is used, the BotkitConversation.onStep() method is called twice, the second time the step.result contains the whole step.values object. In the onStep method line 586 to 590, the result is captured into the step.values for the previous step; that's when things go wrong.

So I was looking at those calls in conversation.ts:

// did we just change threads? if so, restart this turn
                if (index !== step.index || thread_name !== step.thread) {
                    return await this.runStep(dc, step.index, step.thread, DialogReason.nextCalled, step.values);
                }

I don't understand why the step.values is passed as the result parameter here. The onStep method seems to expect the result from the last interaction. That's as far as I could go today!

@benbrown
Copy link
Contributor

benbrown commented Sep 13, 2019

Great digging! super helpful.

I will have to experiment and do more investigation - it may be incorrect to pass this in instead of simply forwarding result

@benbrown benbrown self-assigned this Sep 13, 2019
@benbrown benbrown added the bug label Sep 13, 2019
@benbrown benbrown added this to the 4.6 milestone Sep 13, 2019
@benbrown
Copy link
Contributor

OK I am able to reproduce this reliably.

It seems to be tied to having 2 calls to convo.ask in a row, and calling repeat from one of them...

If there is a call to say in between 2 asks, it is ok.

HRRM!

@benbrown
Copy link
Contributor

@etiennellipse I think your original diagnosis was correct!

@benbrown
Copy link
Contributor

will be fixed in 4.6

@benbrown benbrown closed this as completed Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants