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

fix: can not open a bot if manifest url is invalid #3050

Merged
merged 4 commits into from
May 14, 2020

Conversation

zhixzhan
Copy link
Contributor

@zhixzhan zhixzhan commented May 14, 2020

Description

manifest url is extracted at bot open, fixed by passing this message as a notification to client.

Task Item

close #3047

Screenshots

Untitled1
image

@github-actions
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling d05dac4 on zhixzhan/skill-manifesturl-fix into 7c59824 on master.

@boydc2014
Copy link
Contributor

boydc2014 commented May 14, 2020

Also, like you said, once this url is not reachable and we classify as an error, there is really no good timing to trigger this re-check again, unless reopen the bot, which is weird.

I think the core issue is that, when we have invalid data, we don't show it in the table thus prevent any further explicit action on it. What i would expect is, even the url is invalid, we will still show the url in the table, and at the end the row, there is a warnning sign and a button to refresh, which seems fair to me.

@cwhitten @mareekuh any ideas?

@cwhitten cwhitten merged commit b904207 into master May 14, 2020
@cwhitten cwhitten deleted the zhixzhan/skill-manifesturl-fix branch May 14, 2020 16:32
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
* passing server error to client

* pretty error msg

Co-authored-by: Dong Lei <donglei@microsoft.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
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

Successfully merging this pull request may close these issues.

3 participants