Unhandled exception when receiving websocket error

[orgads]

It happens occasionally when the websocket connection returns 400.

Can be easily reproduced by providing a non-working domain:

    var directLine = new DirectLine({
      secret: <bot-secret>,
      domain: 'https://europe.directline.botframework.com/v3/directline'
    });

Trace:

Error: Unexpected server response: 400                                                           
    at ClientRequest.req.on (E:\Project\node_modules\ws\lib\websocket.js:559:5)                                                                                                       
    at ClientRequest.emit (events.js:159:13)                                                     
    at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:550:21)                
    at HTTPParser.parserOnHeadersComplete (_http_common.js:115:23)                               
    at TLSSocket.socketOnData (_http_client.js:439:20)                                           
    at TLSSocket.emit (events.js:159:13)                                                         
    at addChunk (_stream_readable.js:265:12)                                                     
    at readableAddChunk (_stream_readable.js:252:11)                                             
    at TLSSocket.Readable.push (_stream_readable.js:209:10)                                      
    at TLSWrap.onread (net.js:608:20)                                                            

[orgads]

#171 is a partial fix. At least it doesn't crash, but I'm not sure how to proceed from here. I need a way to notify the client. Can you suggest one?

[orgads]

Here is a MWE (you need to install nock):

globalThis.XMLHttpRequest = require('xhr2');
globalThis.WebSocket = require('ws');
import * as nock from 'nock';
import { DirectLine } from 'botframework-directlinejs';

const conversation = {
  conversationId: '123',
  token: '456',
  streamUrl: 'wss://google.com/'
};
nock('https://directline.botframework.com')
  .post('/v3/directline/conversations')
  .reply(200, JSON.stringify(conversation));
const directLine = new DirectLine({ secret: '1234' });
directLine.activity$
  .subscribe(
    activity => {
      if (activity.type === 'message')
        console.log("received activity ", activity.text);
    },
    error => {
      console.error(error);
    }
  );

[compulim]

Hi @orgads, this is recently fixed in #324. You should start seeing this fixed in @0.13.2-master.c6ecb71.

Please let us know if it resolves the issue. Thanks.

[orgads]

Hi. Sorry for the delay.

I tested it now. It doesn't crash, but I also don't get an error event. Reproduces with the MWE I posted above.

Do I need something else for capturing errors?

[orgads]

@compulim ping

[compulim]

You are correct. The PR #324 was a rework of your #171 without triggering the bug in RxJS and also without logging out the error in the console.

Currently, DLJS was designed not to send out any events on intermittent connection errors.

Looking at directLine.ts:359, this is the enumerable for connection status:

export enum ConnectionStatus {
    Uninitialized,              // the status when the DirectLine object is first created/constructed
    Connecting,                 // currently trying to connect to the conversation
    Online,                     // successfully connected to the conversation. Connection is healthy so far as we know.
    ExpiredToken,               // last operation errored out with an expired token. Possibly waiting for someone to supply a new one.
    FailedToConnect,            // the initial attempt to connect to the conversation failed. No recovery possible.
    Ended                       // the bot ended the conversation
}

Both FailedToConnect and Ended doesn't handle the case of intermediate errors, which reconnection will take place and recovery could be possible.

Connecting looks good to me as a candidate for status-ing about reconnections. Since we debut the Web Socket feature, we never set it to Connecting after initial connection. So this will be change in API behavior.

@cwhitten and @willportnoy tl;dr the customer is asking for a signal or event to indicate when reconnection took place. After some research, I think we could reuse the Connecting enum to indicate a reconnection. But this will also introduce a behavioral change in our API.

In W3C Server-Sent Events, they also used CONNECTING to indicate both initial connection and subsequent reconnections.

We could have an opt-in/out flag. I prefer an opt-out flag because this behavior is aligned with where the industry to heading towards.

Any thoughts?

Update: Looking at our source code today, I am a bit worried about setting a new value to connectionStatus$ may impact our current logic at checkConnection(), which is a function that is called on every REST call (including WS reconnection). Probably need some time to understand how it can be done and what tests we need to add to ensure current behavior is not affected.

[orgads]

cc @yoshigev