Botbuilder-js: Invoke: healthCheck activity will throw on DirectLine Speech

[alopix]

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]

[alopix]

I tried implementing the healthCheck function in a custom adapter to fix this, but the response also does not seem to reach the client. Is this by design or is there another issue I'm missing?
It seems to be sending the invokeResponse activity but it never arrives.

[alopix]

I checked the code some more and I understand why the invokeResponse is not being received by the client.
The BotFrameworkAdapter actually prevents them from being sent here but there's nothing in processRequest which will actually result in a response being sent in DLS (via streamingHttpClient).

[EricDahlvang]

@johnataylor Would you take a look at this one please?

[johnataylor]

@alopix we don't currently support Invoke activities across the streaming channel which explains why you are hitting the exception: we probably didn't even test the scenario because clearly there is an assumption in the code that there is a ConnectorClient if auth is enabled. The fix to suppress the exception seems straight forward however I'm not sure what else would break.

Anyhow I don't see a fundamental reason why something couldn't be made to work.

Could you clarify your scenario? I was a little confused by your remark "I checked the code some more and I understand why the invokeResponse is not being received by the client." Specifically which "client?" And Invoke activities are send on POST calls - are you doing and HTTP post or is this a websocket?

[alopix]

@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.

[stevengum]

@alopix could you expand on requirements for sending the health check via Direct Line Speech? Or could it be sent over regular Direct Line?

I'm not as familiar with the Health Check semantics, nor the exact workings of the DLS channel, but a client that's able to send a streaming Invoke to a bot should already be connected to the bot.

In regards to the bot's InvokeResponse, we would need to verify that the channel (DLS in this scenario) will send the InvokeResponse back to the client.

@NickEricson do you have any insight that you could share regarding this?

[alopix]

@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.

[stevengum]

You're right, a sent invoke without a response would not prove a healthy connection to the bot. My earlier post was referring to two different things:

1. If DLS cannot connect to the bot, it sends an error to the client, therefore the connection (channel-to-bot) is unhealthy
2. Depending on what's being gauged by the health check, a DLS-sent health check may not be necessary ("Is the web app alive?")

Since the health check is specific to verifying that Direct Line Speech works, it sounds like only the first item applies. Is this correct?

Because of the way DLS handles duration of web socket connections and how client-to-channel connections are initiated by the user, the point at which a health check is sent doesn't seem to have more weight than a user being able to speak with the bot. Unless there is additional information being sent back via the health check response, in which case the health check seems necessary.

[alopix]

@stevengum

1. I'm not sure this is true. I've yet to see this error to happen on establishing the connection when the bot is not available. The connection claims to be up but only sending an activity will sometimes result in the connection to be dropped/disconnected (using the JS Speech SDK in any case).
2. We wouldn't check if the web app is alive, as this is done via a separate app health check.

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]

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.

[alopix]

@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".

[stevengum]

I agree, with the background above it seems more than reasonable to support a health check for the DLS channel.

@DDEfromOR, off the top of your head do we need to do anything extra on our end/could we do this in Direct Line ASE?

[carlosscastro]

@stevengum when you're back, please update status on this one. @DDEfromOR, any input on Steven's question above? thanks both!

[DDEfromOR]

@stevengum as far as Direct Line ASE is concerned the InvokeResponse would be unexpected and dropped from the response to the client, which would only indicate if the bot responded with success or non-success status code. This will probably change in the future as we're expanding on the Invoke protocol to better support a request-response communication model.

That being said, the idea of a health test for a bot connected to Direct Line ASE is kind of redundant as DL-ASE has a health endpoint that includes the status of the bot. I'm also assuming this is a side question and not related to the ask for a DLS health test that's the actual concern of this issue, because a DL-ASE health test would not be useful for confirming a bot can communicate with DLS. A health test for a bot connected to DLS definitely makes sense.

Please let me know if I'm misunderstanding your question, Steven!

[tsuwandy]

@stevengum, any update on this?

[alopix]

Any update on this?

[Virtual-Josh]

@johnataylor @stevengum can you provide an update for this ticket?

[congysu]

@joshgummersall could I have your suggestion on which milestone shall this one be handled? Thanks.

[stevengum]

The Bot Framework team is working with the Speech team to come up with an acceptable solution for notifying Speech clients when the DLS channel runs into issues while trying to connect and communicate with the bot.

[msomanathan]

@stevengum is this fix targeted for R14?

[stevengum]

A solution for the underlying issue is still being pursued with the Speech team. (Surfacing of errors from the channel to the client)

However, by design the Channels do not forward invoke responses from bots to the clients. Additionally, the SDK code supporting the healthCheck Invoke Activity was removed in microsoft/botbuilder-js#3482 per https://github.com/microsoft/botframework-sdk/issues/6225.

Closing issue due to lack of support for healthCheck Activities.