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

[Botskills] Tracking of known issues #1246

Closed
9 of 10 tasks
dfavretto opened this issue May 3, 2019 · 11 comments · Fixed by #1351
Closed
9 of 10 tasks

[Botskills] Tracking of known issues #1246

dfavretto opened this issue May 3, 2019 · 11 comments · Fixed by #1351
Assignees
Labels
Type: Bug Something isn't working

Comments

@dfavretto
Copy link
Contributor

dfavretto commented May 3, 2019

Project

Name: Botskills

Language: TypeScript

Description

There are some known bugs with the first implementation, here is a list of them for tracking the ones being work on, and the ones already fixed.

  • Should stop execution if .lu file is not found ([botskills] CLI operation should fail if it can't find LU file #1232).
  • Error in authentication when connecting a Skill ([botskills] az cli error when adding authentication connection #1230).
  • The list command is showing the skills name instead of the ids. This leads to confusion when user tries to disconnect a skill using the names listed with the list command.
  • The commands executed for the authentication process in the connect command are not being printed when using --verbose.
  • When adding a skill that has no authentication connections the botskills CLI shows the following error: Checking for authentication settings ... Could not configure authentication connection automatically. Skill adds fine however but confusing to user. This step should just be skipped.
  • When connecting say the weather skill botskills connect -b vabreakoutspeechdemo -r http://bfweatherskill.azurewebsites.net/api/skill/manifest --cs it reports success but doesn't actually add to the Dispatch model. If you supply luisFolder it works, it should report an error if it can't luisfolder and not add the skill.
  • If there's no .dispatch file when the dispatch add command is executed, it triggers the dispatch init prompting the user for the name of the Dispatch file, instead of throwing an error for not finding the said file.
  • When requiring the skills.json file, if it's malformed (i.e. no skills property) should throw a verbose error. EDIT: A workaround was implemented in PR [Botskills] Fix authentication issue and other minor ones #1293 so there's no need to throw an error.
  • Validate manifest id for special character.
  • When executing async functions, if an error is thrown it's not being handled correctly.
@dfavretto dfavretto added the Type: Bug Something isn't working label May 3, 2019
@dfavretto dfavretto self-assigned this May 6, 2019
@darrenj
Copy link
Contributor

darrenj commented May 7, 2019

@dfavretto added a new -luisfolder issue above

@aishwaryaumachandran
Copy link

I'm trying to connect the assistant template with the skill template:

The assistant has the services set up and it is up and running.
The skill has the manifest and the other json files populated. The skill has been deployed on the app service too. I am trying to connect the assistant and the skill using the "botskills connect" and I have the following error:

What could be the potential issue?

(node:16800) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'find' of undefined
at Object.connectSkill (C:\Users\User\AppData\Roaming\npm\node_modules\botskills\lib\functionality\connectSkill.js:132:25)
at process._tickCallback (internal/process/next_tick.js:68:7)
(node:16800) 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: 1)
(node:16800) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@darrenj
Copy link
Contributor

darrenj commented May 7, 2019

@aishwaryaumachandran Could you share your manifest file if possible please? Alternatively you could also try the deployment\scripts\add_remote_skill.ps1 file within a pwsh.exe console session as a workaround whilst we look at the manifest.

@aishwaryaumachandran
Copy link

{
"id": "SkillSample",
"msaAppId": "",
"name": "SkillSample",
"endpoint": "http://localhost:3978/api/skill/messages",
"description": "Skill Sample",
"authenticationConnections": [
{
"id": "",
"serviceProviderId": "",
"scopes": ""
}
],
"actions": [
{
"id": "skillsample_va_test_skill",
"definition": {
"description": "Skill Sample",
"slots": [
{
"name": "Sample",
"types": [
"string"
]
}
],
"triggers": {
"utterances": [
{
"locale": "en",
"text": [
"demo dialog"
]
}
],
"utteranceSources": [
{
"locale": "en",
"source": [
"va_test_skill#Sample"
]
}
]
}
}
}
]
}

@aishwaryaumachandran
Copy link

Let me know if you need any other details

@aishwaryaumachandran
Copy link

aishwaryaumachandran commented May 7, 2019

Do you have an update on the manifest?

I executed the PowerShell command to connect. It says unable to load Manifest file.

I was able to validate the Manifest file

@darrenj
Copy link
Contributor

darrenj commented May 7, 2019

This command should work (within pwsh.exe). Run from within your bot folder. Replace the URL with your skill manifest endpoint and the luisfolder should point at the /Deployment/Resources/LU/en folder of the skill you've created.

.\Deployment\scripts\add_remote_skill.ps1 -botName "YOUR_BOT_NAME" -manifestUrl https://YOUR_SKILL.azurewebsites.net/api/skill/manifest -luisFolder [path]

If not let me know what the script shows when executing?

@dfavretto
Copy link
Contributor Author

Hey @darrenj, thanks for adding that issue, I think it's highly related to the issue about not finding the .lu file, so I'll be fixing those both issues in the same PR.

Hi @aishwaryaumachandran, that issue seems like a problem with the content of the skills.json of your assistant. Looking at the C# sample, that file is not prepared to be used with the Botskills since it has no skills property with an empty array.

In the PR #1293 we implemented a workaround in case that property is not present. Maybe the Botskills' published version doesn't have that yet, so I'd suggest you copy the skills.json located in the TypeScript version.

@aishwaryaumachandran
Copy link

aishwaryaumachandran commented May 7, 2019

Thank you @darrenj @dfavretto I was able to get the assistant and the skill connected using powershell.

I'll try that solution of taking from the TypeScript version and check.

@darrenj
Copy link
Contributor

darrenj commented May 8, 2019

Thanjs @dfavretto good spot. Could we have botskills cope with a missing skills property? (Ie add it) I'll pr adding this but think it's reasonable for it to be missing especially if someone adds skills support to their own bot.

@dfavretto
Copy link
Contributor Author

@darrenj actually the botskills is already taking this situation into account since PR #1293 when we modified this line in the connect command, so if the skills property is missing, it is created in the moment of adding a new Skill.

I'm not quite sure if the published version has this changes yet, but if it doesn't, I'd suggest waiting before publishing another version, since I'll be creating a PR for fixing the issues with .lu files and .dispatch files not being validated before trying to update the Dispatch model.

@lauren-mills lauren-mills removed Status: Committed This has been confirmed for the next release. Status: In Progress This work item in underway. labels Jul 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants