[CSharp] DirectLine + WebSockets sample#86
Conversation
dandriscoll
left a comment
dandriscoll
left a comment
There was a problem hiding this comment.
Looks great overall, only a few small changes
| { | ||
| private static string directLineSecret = ConfigurationManager.AppSettings["DirectLineSecret"]; | ||
| private static string botId = ConfigurationManager.AppSettings["BotId"]; | ||
| private static string fromUser = "DirectLineSampleClientUser"; |
There was a problem hiding this comment.
Can you add the following comment above fromUser?
// fromUser is the field that identifies which user is sending activities to the Direct Line service.
// Because this value is created and sent within your Direct Line client, your bot should not
// trust the value for any security-sensitive operations. Instead, have the user log in and
// store any sign-in tokens against the Conversation or Private state fields. Those fields
// are secured by the conversation ID, which is protected with a signature.
|
|
||
| private static void WebSocketClient_OnMessage(object sender, MessageEventArgs e) | ||
| { | ||
| // avoid null reference exception when no data received |
There was a problem hiding this comment.
Change this comment to:
// Occasionally, the Direct Line service sends an empty message as a liveness ping. Ignore these messages.
There was a problem hiding this comment.
One other item I just thought of: can you add some code to the sample that exchanges the Direct Line secret for a token before using it?
There was a problem hiding this comment.
Done and added call to Tokens API to retrieve token, also added note to README
| <?xml version="1.0" encoding="utf-8"?> | ||
| <packages> | ||
| <package id="Microsoft.Bot.Connector.DirectLine" version="3.0.0" targetFramework="net461" /> | ||
| <package id="Microsoft.Rest.ClientRuntime" version="2.3.4" targetFramework="net461" /> |
There was a problem hiding this comment.
We've hit binary incompatibilities between minor versions of Microsoft.Rest.ClientRuntime. 2.3.2 is probably a better example to use.
| select x; | ||
| ```` | ||
|
|
||
| DirectLine v3.0 (unlike version 1.1) has support for Attachments (see [Adding Attachments to a Message](https://docs.botframework.com/en-us/core-concepts/attachments) for more information about attachments). Check out the `WebSocketClient_OnMessage` method in [Program.cs](DirectLineClient/Program.cs#L73-L90) to see how the Attachments are retrieved and rendered appropriately based on their type. |
There was a problem hiding this comment.
In printed text, Direct Line is normally two words.
It's OK to collapse them in filenames, though. ("DirectLineClient.cs" is OK)
|
Hi @dandriscoll , we've updated the PR with the latest feedback. |
4 participants
No description provided.