diff --git a/__tests__/unhappy.brokenWebSocket.js b/__tests__/unhappy.brokenWebSocket.js new file mode 100644 index 000000000..7f1180619 --- /dev/null +++ b/__tests__/unhappy.brokenWebSocket.js @@ -0,0 +1,117 @@ +import 'dotenv/config'; +import 'global-agent/bootstrap'; + +import { defineEventAttribute, EventTarget } from 'event-target-shim'; +import nock from 'nock'; +import onErrorResumeNext from 'on-error-resume-next'; + +import { DirectLine } from '../src/directLine'; + +function corsReply(nockRequest) { + nockRequest.reply(function () { + const { headers } = this.req; + + return [ + 200, + null, + { + 'Access-Control-Allow-Headers': headers['access-control-request-headers'], + 'Access-Control-Allow-Methods': headers['access-control-request-method'], + 'Access-Control-Allow-Origin': headers.origin + } + ]; + }); +} + +describe('Unhappy path', () => { + let unsubscribes; + + beforeEach(() => (unsubscribes = [])); + afterEach(() => unsubscribes.forEach(fn => onErrorResumeNext(fn))); + + describe('broken Web Socket', () => { + let numErrors; + let numReconnections; + + beforeEach(async () => { + numErrors = 0; + numReconnections = 0; + + nock('https://directline.botframework.com') + .persist() + .post(uri => uri.startsWith('/v3/directline/conversations')) + .reply( + 200, + JSON.stringify({ + conversationId: '123', + token: '456', + streamUrl: 'wss://not-exist-domain' + }) + ) + .get(uri => uri.startsWith('/v3/directline/conversations')) + .reply( + 200, + JSON.stringify({ + conversationId: '123', + token: '456', + streamUrl: 'wss://not-exist-domain' + }) + ); + + corsReply( + nock('https://directline.botframework.com') + .persist() + .options(uri => uri.startsWith('/v3/directline/conversations')) + ); + + window.WebSocket = class extends ( + EventTarget + ) { + constructor() { + super(); + + numReconnections++; + + setTimeout(() => { + numErrors++; + + this.dispatchEvent(new ErrorEvent('error', { error: new Error('artificial') })); + this.dispatchEvent(new CustomEvent('close')); + }, 10); + } + }; + + defineEventAttribute(window.WebSocket.prototype, 'close'); + defineEventAttribute(window.WebSocket.prototype, 'error'); + }); + + test('should reconnect only once for every error', async () => { + const directLine = new DirectLine({ + token: '123', + webSocket: true + }); + + // Remove retry delay + directLine.getRetryDelay = () => 0; + + unsubscribes.push(() => directLine.end()); + + await new Promise(resolve => { + const subscription = directLine.activity$.subscribe(() => {}); + + setTimeout(() => { + subscription.unsubscribe(); + resolve(); + }, 2000); + }); + + // Because we abruptly stopped reconnection after 2 seconds, there is a + // 10ms window that the number of reconnections is 1 more than number of errors. + expect(Math.abs(numReconnections - numErrors)).toBeLessThanOrEqual(1); + + // As we loop reconnections for 2000 ms, and we inject errors every 10 ms. + // We should only see at most 200 errors and reconnections. + expect(numReconnections).toBeLessThanOrEqual(200); + }); + }); +}); diff --git a/package-lock.json b/package-lock.json index 57b9797cf..8ad6d0e56 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4251,6 +4251,12 @@ "integrity": "sha1-Qa4u62XvpiJorr/qg6x9eSmbCIc=", "dev": true }, + "event-target-shim": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/event-target-shim/-/event-target-shim-5.0.1.tgz", + "integrity": "sha512-i/2XbnSz/uxRCU6+NdVJgKWDTM427+MqYbkQzD321DuCQJUqOuJKIA0IM2+W2xtYHdKOmZ4dR6fExsd4SXL+WQ==", + "dev": true + }, "eventemitter3": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.4.tgz", @@ -7177,6 +7183,12 @@ "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==", "dev": true }, + "lodash.set": { + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/lodash.set/-/lodash.set-4.3.2.tgz", + "integrity": "sha1-2HV7HagH3eJIFrDWqEvqGnYjCyM=", + "dev": true + }, "lodash.sortby": { "version": "4.7.0", "resolved": "https://registry.npmjs.org/lodash.sortby/-/lodash.sortby-4.7.0.tgz", @@ -7599,6 +7611,35 @@ "integrity": "sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==", "dev": true }, + "nock": { + "version": "13.0.5", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.0.5.tgz", + "integrity": "sha512-1ILZl0zfFm2G4TIeJFW0iHknxr2NyA+aGCMTjDVUsBY4CkMRispF1pfIYkTRdAR/3Bg+UzdEuK0B6HczMQZcCg==", + "dev": true, + "requires": { + "debug": "^4.1.0", + "json-stringify-safe": "^5.0.1", + "lodash.set": "^4.3.2", + "propagate": "^2.0.0" + }, + "dependencies": { + "debug": { + "version": "4.3.1", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.1.tgz", + "integrity": "sha512-doEwdvm4PCeK4K3RQN2ZC2BYUBaxwLARCqZmMjtF8a51J2Rb0xpVloFRnCODwqjpwnAoao4pelN8l3RJdv3gRQ==", + "dev": true, + "requires": { + "ms": "2.1.2" + } + }, + "ms": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", + "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", + "dev": true + } + } + }, "node-fetch": { "version": "2.6.0", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.0.tgz", @@ -8189,6 +8230,12 @@ "sisteransi": "^1.0.3" } }, + "propagate": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", + "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", + "dev": true + }, "prr": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/prr/-/prr-1.0.1.tgz", diff --git a/package.json b/package.json index 316176b33..536e47d9e 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "babel-plugin-transform-inline-environment-variables": "^0.4.3", "concurrently": "^4.1.2", "dotenv": "^8.1.0", + "event-target-shim": "^5.0.1", "get-port": "^5.0.0", "global-agent": "^2.0.2", "has-resolved": "^1.1.0", @@ -56,6 +57,7 @@ "jest": "^24.9.0", "jest-environment-jsdom-fourteen": "^0.1.0", "jsdom": "^14.1.0", + "nock": "^13.0.5", "node-fetch": "^2.6.0", "on-error-resume-next": "^1.1.0", "restify": "^8.4.0", diff --git a/src/directLine.ts b/src/directLine.ts index 5545ab1c1..07f5d6ea1 100644 --- a/src/directLine.ts +++ b/src/directLine.ts @@ -930,6 +930,7 @@ export class DirectLine implements IBotConnection { konsole.log("creating WebSocket", this.streamUrl); const ws = new this.services.WebSocket(this.streamUrl); let sub: Subscription; + let closed: boolean; ws.onopen = open => { konsole.log("WebSocket open", open); @@ -949,7 +950,21 @@ export class DirectLine implements IBotConnection { ws.onclose = close => { konsole.log("WebSocket close", close); if (sub) sub.unsubscribe(); - subscriber.error(close); + + // RxJS.retryWhen has a bug that would cause "error" signal to be sent after the observable is completed/errored. + // We need to guard against extraneous "error" signal to workaround the bug. + closed || subscriber.error(close); + closed = true; + } + + ws.onerror = error => { + konsole.log("WebSocket error", error); + if (sub) sub.unsubscribe(); + + // RxJS.retryWhen has a bug that would cause "error" signal to be sent after the observable is completed/errored. + // We need to guard against extraneous "error" signal to workaround the bug. + closed || subscriber.error(error); + closed = true; } ws.onmessage = message => message.data && subscriber.next(JSON.parse(message.data));