Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

Update activity after migration to 0.8 is not working #3393

Closed
tommyJimmy87 opened this issue May 20, 2020 · 18 comments
Closed

Update activity after migration to 0.8 is not working #3393

tommyJimmy87 opened this issue May 20, 2020 · 18 comments
Assignees
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. Type: Bug Something isn't working

Comments

@tommyJimmy87
Copy link

What project is affected?

Virtual Assistant Project and Skill Project.

What language is this in?

Typescript.

What happens?

From MS Teams Channel, when trying to update an activity from a Skill, the Virtual Assistant will error out.

What are the steps to reproduce this issue?

  1. Migrate the VA and Skill to the latest version 0.8 of Typescript
  2. In the VA we have to implement a new Class "CustomSkillHandler" which extends the class "ChannelServiceHandler" (we need to implement this because the updateActivity function is not implemented in the provided "SkillHandler")
  3. Inside the "CustomSkillHandler" implement the onUpdateActivity :
    protected async onUpdateActivity(claimsIdentity: ClaimsIdentity, conversationId: string, activityId: string, activity: Activity): Promise<ResourceResponse> {
        return await this.updateActivity(claimsIdentity, conversationId, activityId, activity);
    }

and the updateActivity function looks like this:

private async updateActivity(claimsIdentity: ClaimsIdentity, conversationId: string, replyToActivityId: string, activity: Activity): Promise<ResourceResponse> {

        let skillConversationReference: SkillConversationReference;
        try {
            skillConversationReference = await this.conversationIdFactory.getSkillConversationReference(conversationId);
        } catch (err) {
            // If the factory has overridden getSkillConversationReference, call the deprecated getConversationReference().
            // In this scenario, the oAuthScope paired with the ConversationReference can only be used for talking with
            // an official channel, not another bot.
            if (err.message === 'Not Implemented') {
                const conversationReference = await this.conversationIdFactory.getConversationReference(conversationId);
                skillConversationReference = {
                    conversationReference,
                    oAuthScope: JwtTokenValidation.isGovernment(this.channelService) ?
                        GovernmentConstants.ToChannelFromBotOAuthScope :
                        AuthenticationConstants.ToChannelFromBotOAuthScope
                };
            } else {
                // Re-throw all other errors. 
                throw err;
            }
        }

        if (!skillConversationReference) {
            throw new Error('skillConversationReference not found');
        }
        if (!skillConversationReference.conversationReference) {
            throw new Error('conversationReference not found.');
        }

        const activityConversationReference = TurnContext.getConversationReference(activity);

        const callback = async (context: TurnContext): Promise<void> => {
            const adapter: BotFrameworkAdapter = (context.adapter as BotFrameworkAdapter);
            // Cache the ClaimsIdentity and ConnectorClient on the context so that it's available inside of the bot's logic.
            context.turnState.set(adapter.BotIdentityKey, claimsIdentity);
            context.turnState.set(this.SkillConversationReferenceKey, activityConversationReference);
            activity = TurnContext.applyConversationReference(activity, skillConversationReference.conversationReference) as Activity;
            const client = adapter.createConnectorClient(activity.serviceUrl);
            context.turnState.set(adapter.ConnectorClientKey, client);

            context.activity.id = replyToActivityId;
            await context.updateActivity(context.activity);
            return;
        };
        AppCredentials.trustServiceUrl(skillConversationReference.conversationReference.serviceUrl);

        await (this.adapter as BotFrameworkAdapter).continueConversation(skillConversationReference.conversationReference, skillConversationReference.oAuthScope, callback);
        return { id: uuid() };
    }
  1. In the index change the SkillHandlerdeclaration :
// Register the request handler.
const handler: CustomSkillHandler = new CustomSkillHandler(adapter, bot, skillConversationIdFactory, credentialProvider, authenticationConfiguration);
const skillEndpoint = new ChannelServiceRoutes(handler);
skillEndpoint.register(server, '/api/skills');

  1. Test an update activity from the Teams Channel.

What were you expecting to happen?

The Card is updated in Teams

Can you share any logs, error output, etc.?

The error is shown when the VA tries to update the activity from the previous described function onUpdateActivity.

(node:13410) UnhandledPromiseRejectionWarning: Error: Failed to decrypt conversation id
    at new RestError (/Users/xxxx/Desktop/repository/xxx/virtual-assistant/node_modules/botframework-connector/node_modules/@azure/ms-rest-js/dist/msRest.node.js:1397:28)
    at /Users/xxxx/Desktop/repository/xxx/virtual-assistant/node_modules/botframework-connector/node_modules/@azure/ms-rest-js/dist/msRest.node.js:1849:37
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:13410) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 10)

Any screenshots or additional context?

Channel Teams

@tommyJimmy87 tommyJimmy87 added Needs Triage Needs to be triaged for assignment Type: Bug Something isn't working labels May 20, 2020
@Batta32 Batta32 self-assigned this May 20, 2020
@Batta32
Copy link
Collaborator

Batta32 commented May 20, 2020

Thanks @tommyJimmy87 for reporting this issue, we are still reviewing it. As soon as we have any update we will be back to you 😊!

@Batta32
Copy link
Collaborator

Batta32 commented May 22, 2020

Hi @tommyJimmy87, I need you to give an extra repro steps of what you're doing in order to figure out what's wrong as we couldn't reproduce the issue that you are facing.

  1. What do you mean with The Card is updated in Teams?
  2. Are you using the Virtual Assistant and Skill in TypeScript version?
  3. Can you provide the connection/communication steps between the Virtual Assistant & Skill and how are you testing the CustomSkillHandler?
  4. Are you using the generator-bot-virtualassistant@1.0.0 to generate the Virtual Assistant and Skill?

Here is the branch with our latest changes that we made to reproduce this issue.

Environment

Repro steps

  1. Deploy a VA & Skill using the generator-bot-virtualassistant
  2. Connect the VA & Skill using botskills connect command
  3. Create a new CustomSkillHandler class extending from ChannelServiceHandler
  4. Add the onUpdateActivity and updateActivity methods on the CustomSkillHandler
  5. Modify the index file to receive the CustomSkillHandler
  6. Test the connection between the Virtual Assistant and Skill using Teams channel

We will be attentive to your answer!

@tommyJimmy87
Copy link
Author

Hi @Batta32, will try to answer your questions :

  1. I mean that I'm trying to update an adaptive card in MS Teams channel and the result should be that the card is updated after I did some action (the card is within the skill, so basically the test is the updateActivity from the skill to the VA).

  2. Yes, both VA and Skill are in Typescript

  3. In the skill I have an adaptive card, after an action from the user this card should be updated with different content. From the skill I call the updateActivity:

dc.context.updateActivity(previouslySentActivity);

previouslySentActivity it's the activity that we want to update, we save the activity in the state in order to be able to update it later. This use to work in the previous version, but the flow of sending and update activity as far as I understand changed with the new version.

This will call the VA endpoint (this is working) which should then update the activity. The call comes into the CustomSkillHandler in the onUpdateActivity method but then I get the error when the activity is sent to MS Teams.

  1. No, unfortunately our project is already in an advanced status and I needed to migrate to the new version maintaining all the custom stuff that we did, basically, we migrated manually. Which is not great but necessary.

Let me know if you need more context, meanwhile, maybe I can try this out also on your branch and check if I have the same error.

@tommyJimmy87
Copy link
Author

Hi @Batta32, I tested it from the branch you are using to test. I created a fork here: https://github.com/tommyJimmy87/botframework-solutions/tree/feature/southworks/conversationid-issue
I changed your branch to reproduce the same error. Within the testSkillHandler I added a try-catch to get the error :

            try{
                await context.updateActivity(activity);
            }catch(e){
                console.log(e);
            }

This is the actual error I'm getting from the updateActivity call :

{ body: '{"error":{"code":"ServiceError","message":"Unknown"}}',
     headers: HttpHeaders { _headersMap: [Object] },
     status: 400 },
  body: { error: { code: 'ServiceError', message: 'Unknown' } } }

Which is the same I get in my project.
This is the test I did:

  1. I created a new Dialog inside the sample-skill testUpdateActivityDialog where in the beginDialog I send a TestCard with a text. In the onContinueDialogI send the same card but with a different text;

  2. Connect the VA to the Skill.

  3. Call the VA with some intent that will call the Skill and start the Test Dialog and show the card;

  4. Call the VA with some intent that will call the Skill and the continueDialog of the Test Dialog;

  5. The skill will call the VA in order to update the card and then you get the error.

@Batta32
Copy link
Collaborator

Batta32 commented May 26, 2020

Great @tommyJimmy87 ! We will review it with this new information, thank you very much 😊.

@darrenj darrenj added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. labels May 27, 2020
@DiegoCardozo94
Copy link
Contributor

DiegoCardozo94 commented Jun 4, 2020

Hi @tommyJimmy87! We successfully reproduced the scenario using your latest changes and the mentioned repro steps.

We will continue researching about this issue. As soon as we have any update, we will back to you 😊.

We tested the connection between the VA & Skill bots detecting the same issue when the skills sends the activity information to the VA
image

@tommyJimmy87
Copy link
Author

tommyJimmy87 commented Jun 10, 2020

Hi @DiegoCardozo94,

Thanks for the update.

Do you have any idea when this might be released?
Do you think this can be released before the 17th, since we have a strictly release from our side we would like just to understand if the 17th date is feasible or not ? :)

Thanks

@Batta32
Copy link
Collaborator

Batta32 commented Jun 18, 2020

Hi @tommyJimmy87, sorry for the delay.

We managed to validate that this issue happens in C# as well so it appears not to be related to TypeScript only.

We found some many related issues:

  1. SkillHandler doesn't return ResourceResponse when forwarding activities botframework-sdk#5919: it details that the SkillHandler generates a fake ResourceResponse when it forwards activities from the skill to the channel. This means that operations like update and delete won't work.
  2. Update Activity method from Virtual Assistant  botbuilder-dotnet#3686: it details that the SkillHandler class does not implement OnUpdateActivityAsync.
    If the Skill is run without a Virtual Assistant the cards are correctly updated, but when run connected to a Virtual Assistant, the cards are not updated.
    This issue has a comment saying that this problem should be fixed on the next release. You can track the state of the work in this I can Update and Delete activities from a Skill botframework-sdk#5788.
  3. MS bot with Ms teams channel gives 400 error OfficeDev/BotBuilder-MicrosoftTeams#124: it details that using a V3 bot in Teams Channels it fails with HTTP 400 Bad Request (similar to this issue).

Last but not least, we validated the following scenarios using the mentioned steps:

  • [works] TypeScript Skill alone
  • [issue reproduced] TypeScript Virtual Assistant connected TypeScript Skill
  • [issue reproduced] TypeScript Virtual Assistant connected C# Skill
  • [issue reproduced] C# Virtual Assistant connected TypeScript Skill
  • [issue reproduced] C# Virtual Assistant connected C# Skill

@darrenj, we replicated this issue using C# bots too.

Our test environment:

  • Branch with our modifications
  • C# Virtual Assistant Sample
  • C# Skill Sample

Our repro steps:

  1. Deploy both samples
  2. Setup the Virtual Assistant with Ngrok
  3. Connect the Virtual Assistant with Microsoft Teams
  4. Connect the Skill with the Virtual Assistant
  5. Run both samples
  6. In Microsoft Teams, send hello to the Virtual Assistant and complete the OnboardingDialog
  7. Send run sample dialog, the Virtual Assistant will respond with a card with the text Send Activity
  8. Send another message to the Virtual Assistant, it will throw an exception trying to update the activity

The Virtual Assistant will throw an exception when trying to update an activity
image

TypeScript Skill successfully running in Microsoft Teams Channel, updating the card which was previously sent
image

@gabog
Copy link
Contributor

gabog commented Jul 13, 2020

Hi, microsoft/botframework-sdk#5919 has nee addressed on all 3 supported languages, you can try if that solution helps using one of the daily builds starting tomorrow.

@tommyJimmy87
Copy link
Author

Hi @Batta32, here it's something that you have to update first right? It's not something I can try already?

@Batta32
Copy link
Collaborator

Batta32 commented Jul 16, 2020

Hi @tommyJimmy87! Yes, we will be validating what @gabog mentioned for TypeScript and C#.

We will be updating this branch adding the last updates and incorporating the botbuilder-v4-js-daily daily build of the SDK that contains the microsoft/botbuilder-js#2489 and microsoft/botbuilder-dotnet#4264 .

As soon as we have any update, we will back to you 😊.

@lauren-mills lauren-mills removed the Needs Triage Needs to be triaged for assignment label Jul 16, 2020
@Batta32
Copy link
Collaborator

Batta32 commented Jul 21, 2020

Hi @tommyJimmy87, we successfully confirmed that applying the changes of microsoft/botbuilder-js#2489 in the testSkillHandler this issue is solved and the activity is correctly updated using Microsoft Teams channel.

Check this commit in order to identify the changes that you should apply in your testSkillHandler, specifically in the processActivity method.

@darrenj & @gabog - we successfully validated the changes of microsoft/botbuilder-dotnet#4264 using C# bots too.

Our test environment:

  • Branch with our modifications
  • TypeScript Virtual Assistant Sample
  • TypeScript Skill Sample

Repro steps:

  1. Deploy both samples
  2. Setup the Virtual Assistant with Ngrok
  3. Connect the Virtual Assistant with Microsoft Teams
  4. Connect the Skill with the Virtual Assistant
  5. Run both samples
  6. In Microsoft Teams, send hello to the Virtual Assistant and complete the OnboardingDialog
  7. Send run sample dialog, the Virtual Assistant will respond with a card with the text Send Activity
  8. Send another message to the Virtual Assistant, it will update the card with the text Update Activity

More details

Changes made on the processActivity method of testSkillHandler
image

TypeScript Virtual Assistant successfully running in Microsoft Teams Channel, updating the card which was previously sent
image

We will be attentive to your answer!

@tommyJimmy87
Copy link
Author

Hi @Batta32, thanks for the update, we will try this out asap. Just one question from my side: is any library version changed or it's just the commit you posted above? Thanks

@Batta32
Copy link
Collaborator

Batta32 commented Jul 21, 2020

@tommyJimmy87 - Just the commit we've posted above!

@tommyJimmy87
Copy link
Author

Hi again @Batta32 :) Also this one looks like solved! I will do some more deep tests tomorrow but seems to be ok with the changes in the Test Skill Handler class you provided. Thanks!

@Batta32
Copy link
Collaborator

Batta32 commented Jul 29, 2020

Thanks @tommyJimmy87! As soon as you can confirm this, we can close this issue 😊.

@tommyJimmy87
Copy link
Author

Hi @Batta32! We can gladly close this issue :)

@Batta32
Copy link
Collaborator

Batta32 commented Jul 31, 2020

Thanks @tommyJimmy87 for confirming this!

@darrenj - we can close this issue as it was solved by the SDK team!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants