Botbuilder-js: [Bug, Transcripts] TranscriptLoggerMiddleware should check for responses before checking responses.length

[piyushkacha7]

Versions

package version of the SDK: 4.6.0
nodejs version are you using: 12.13.0 and 10.17.0

Describe the bug

Below I shared the image, every message throw error.
error in botbuilder-core\lib\transcriptLogger.js:56:43 line package.

I'm testing inside the ms-team channel.

Screenshots

[bug]

[samirkatyar]

Im Also facing the same issue. after update 4.5.3 To 4.6.0

[sagarmodhavaniya]

i am also getting the same error after update 4.5.3 to 4.6.0
@MicrosoftIssueBot

[dkbhadeshiya]

I am facing the same issue as well.

[christopheranderson]

We're looking at this now.

[stevengum]

@piyushkacha7, @samirkatyar, @sagarmodhavaniya, @dkbhadeshiya

What is the bot trying to do when it fails while trying to log the sent activities? Is it trying to create a new conversation in MS Teams?

[stevengum]

I have a test that properly fails without a check for responses, but I'd like to understand more about the scenarios that you're all facing.

Do you have multiple SendActivitiesHandlers registered in your bot(s)? Each SendActivitiesHandler should return the value from next().

---

Branch & Example test:

    it(`should not error for sent activities if no ResourceResponses are received`, async () => {
        class NoResourceResponseAdapter extends TestAdapter {
            constructor(logic) {
                super(logic);
            }

            // Send activities but don't pass the ResourceResponses to the TranscriptLoggerMiddleware
            async sendActivities(context, activities) {
                await super.sendActivities(context, activities);
            }
        }

        let conversationId = null;
        const transcriptStore = new MemoryTranscriptStore();
        const adapter = new NoResourceResponseAdapter(async (context) => {
            conversationId = context.activity.conversation.id;
            const typingActivity = {
                type: ActivityTypes.Typing,
                relatesTo: context.activity.relatesTo
            };

            await context.sendActivity(typingActivity);
            await context.sendActivity(`echo:${context.activity.text}`);

        }).use(new TranscriptLoggerMiddleware(transcriptStore));

        await adapter.send('foo')
            .assertReply(activity => assert.equal(activity.type, ActivityTypes.Typing))
            .assertReply('echo:foo')
            .send('bar')
            .assertReply(activity => assert.equal(activity.type, ActivityTypes.Typing))
            .assertReply('echo:bar');

        const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId);
        assert.equal(pagedResult.items.length, 6);
        assert.equal(pagedResult.items[0].text, 'foo');
        assert.equal(pagedResult.items[1].type, ActivityTypes.Typing);
        assert.equal(pagedResult.items[2].text, 'echo:foo');
        assert.equal(pagedResult.items[3].text, 'bar');
        assert.equal(pagedResult.items[4].type, ActivityTypes.Typing);
        assert.equal(pagedResult.items[5].text, 'echo:bar');
        pagedResult.items.forEach(a => {
            assert(a.timestamp);
        });
    });

[stevengum]

I've added another test for scenarios where other TurnContext registered SendActivitiesHandlers do not return next() which also causes this error to occur. The order of the registered SendActivitiesHandlers matters in this scenario.

class NoResourceResponseMiddleware {
    async onTurn(context, next) {
        context.onSendActivities(async (context, activities, next) => {
            // In SendActivitiesHandlers developers are supposed to call:
            //      return next();
            // If this is not returned, then the next handler will not have the ResourceResponses[].
            const responses = await next();
        });

        // Run the bot's application logic.
        await next();
    }
}

const adapter = new TestAdapter() // etc.

adapter.use(new TranscriptLoggerMiddleware(transcriptStore));
adapter.use(new NoResourceResponseMiddleware());

[piyushkacha7]

while

@stevengum I'm using TranscriptLoggerMiddleware for generating a transcript. It's not Channel specific issue. any channel does not get a response of bot. dialogs are beginning but bot sent response time throw this error.

[stevengum]

Leaving this issue open as it needs to be cherry-picked against master.

[stevengum]

@piyushkacha7, @samirkatyar, @sagarmodhavaniya, @dkbhadeshiya, this fix has gone out in 4.6.1 which is available on npm.

After #1426 is merged to master this issue will be closed.

[stevengum]

The logic in master is different from the logic in the 4.6 branch. They should have been same prior to #1409. As such, this issue will not actually be closed, and instead #1426 has been closed for now as we regroup to do what's right for the R7 timeframe.

My comment from #1426:

_Closing as the current logic in master was not released in 4.6. Specifically the logic to stamp over the activity.id with the actual resourceresponse.id. In master we will pick up theses tests and revisit the logic in the TranscriptLoggerMiddleware for R7._

[stevengum]

The next step to take regarding this issue is comparing transcript logs using the 4.5 branch and the master branch to determine what steps to take.