botbuilder-js: Invoke: healthCheck activity will throw on DirectLine Speech
Versions
What package version of the SDK are you using. 4.9.3 What nodejs version are you using 12.16.1 (but irrelevant to this bug) What browser version are you using irrelevant What os are you using irrelevant
Describe the bug
An invoke activity with the name healthCheck will fail on DirectLine Speech on this line: https://github.com/microsoft/botbuilder-js/blob/1908c683c3fc9c74260fb26494c9da4c4a3169f3/libraries/botbuilder/src/botFrameworkAdapter.ts#L1385
(TypeError: Cannot read property 'credentials' of undefined)
context.turnState.get(this.ConnectorClientKey) is gonna be undefined for a WebSocket connection as processActivity is never gonna be called, where this value is set.
To Reproduce
Send a {"type": "invoke", "name": "healthCheck"} to any bot written in JS and observe the error.
Expected behavior
Check for undefined on getting the connectorClient from turnState first.
Screenshots
Additional context
[bug]
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Comments: 21 (20 by maintainers)
@stevengum
So, we’re only interested in a DLS check, yes. We’ve had an outage recently where all of our backend was healthy but the DLS connection didn’t work after an SSL and private key update on the Bot Service side. We received WebSocket connectsions, but they failed to finish establishing, which such a healthcheck would have alerted on.
@johnataylor yes, that’s the gist of it. We started looking into this kind of healthcheck and before defining a custom activity type, we investigated and found that the botbuilder SDK already supports such a type and “invoke” sounds like the right solution for this anyways, so we wanted to continue using an existing “standard”.
In order to verify the websocket connection is good you need to actually send something over it and get a response. The proposal is that that should be the HealthCheck Activity - which happens to be an Invoke Activity.
@alopix Is that a reasonable summary?
On the surface this doesn’t seem unreasonable to me. But as I say, I’m sure we’re missing a few things to make this work, as its essentially a new feature as this wasn’t considered before, so it would be work for the next milestone if we get agreement. And presumably we’d like to have the DLS service send this HealthCheck. @NickEricson maybe you can comment.
@stevengum The use case is very specific to verifying that DirectLine Speech works, which is the only protocol supported on our bot side.
Sending the invoke would not prove to the client, a healthy connection to the bot, without receiving a response, as DLS does not provide any kind of “received” status as DL would. There’s no ACK to the client.
@johnataylor thanks for the clarifying. I added the fix locally and it does not crash anymore, but as with my other comment, the invokeResponse would not be sent back anyways.
Sorry, when I speak of client I mean a client using the Speech SDK and connecting via DLS (https://github.com/microsoft/cognitive-services-speech-sdk-js). So it would be a WebSocket connection, which as you say, does not seem to properly support Invoke activities. We’d like to reuse the same type of activity as with HTTP-based Skills to send a healthCheck through DLS though.