fix stylecop issue and add message to exception #2750
Conversation
|
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.5.3 |
There was a problem hiding this comment.
arguable a null Action or BotMessagePreviewAction should be a bad request? currently it would be a NotImplemented i'm tempted to just leave this at this point...
I'm not sure I follow this (or I'm probably just missing something)...
We already throw a BadRequest if the original OnFileConsentAsync() gets to a point where it would Action might be null. Isn't this covered in the switch?
If there is no TurnContext.Activity.Value period, we definitely know the whole request is incorrect and throw an exception in the SafeCast().
All of that said, LGTM.
| @@ -315,24 +315,26 @@ private static T SafeCast<T>(object value) | |||
| var obj = value as JObject; | |||
| if (obj == null) | |||
| { | |||
| throw new Exception($"expected type '{value.GetType().Name}'"); | |||
| throw new InvokeResponseException(HttpStatusCode.BadRequest, $"expected type '{value.GetType().Name}'"); | |||
There was a problem hiding this comment.
I can check the TurnContext.Activity.Value with a truthy check, if it's null I'll throw a BadRequest... There's nothing else to be done on the type casting in JS.
@EricDahlvang stylecop was complaining about the exception class - while i was in there I added the message as we discussed and also made the SafeCast throw this exception.
arguable a null Action or BotMessagePreviewAction should be a bad request? currently it would be a NotImplemented i'm tempted to just leave this at this point...