Skip to content

Add more error handling to mountAdaptiveCards#1176

Closed
corinagum wants to merge 3 commits intomicrosoft:masterfrom
corinagum:corinagum/ac-try-catch
Closed

Add more error handling to mountAdaptiveCards#1176
corinagum wants to merge 3 commits intomicrosoft:masterfrom
corinagum:corinagum/ac-try-catch

Conversation

@corinagum
Copy link
Copy Markdown
Contributor

#1125
@compulim could you let me know if this is what you were looking for for this fix? (also could you provide guidance on appropriate tests?)

@corinagum corinagum requested a review from compulim September 27, 2018 02:13
adaptiveCard.parse(cardWithoutHttpActions(this.props.jsonCard));
errors = adaptiveCard.validate();
} catch (error) {
errors.push(error);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you verify how errors array get displayed on the screen?

Think about the upstream and downstream when changing the code:

  • Downstream
    • Can the screen display Error?
    • Can it display IValidationError and Error at the same time?
    • What's the code over there? There is a command at MockBot v4 to force IValidationError, The command is called card broken and card broken:1
    • Also add a case to MockBot to make sure we capture this regression later
      • MockBot might already have this captured or not, I am not quite sure
  • Upstream: there is no upstreamer in this case, so it's good

Also, try it in v4 by visiting https://webchat-playground.azure-websites.net/. If there is a bug over there, file another issue.

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.

2 participants