Skip to content

Commit 31ba05b

Browse files
authored
Fix reconnection scenarios and various behaviors of Direct Line ASE (#404)
* Add some tests for DLASE * Add more tests * Clean up * Clean up * Add Web Socket proxy * Refactor * Add createBotProxy * Rename to setupBotProxy * Clean up * Clean up * Clean up * Update Jest * Add test * Fix test * Add test * Fix postActivity.fail and reconnect * Simplify * Ignore @types/jsdom * Fix tests * Fix tests * Add docs * Flush * Add assertions * Add license
1 parent c2f9b2a commit 31ba05b

43 files changed

Lines changed: 5982 additions & 4760 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

__tests__/createConversation.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/** @jest-environment ./__tests__/setup/jsdomEnvironmentWithProxy */
2+
13
/// <reference path="../node_modules/@types/jest/index.d.ts" />
24

35
import createServer from './setup/createServer';
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# To-dos
2+
3+
Due to resources constraints, while we are working on PR #404 to improve the code quality, there are scenarios we missed.
4+
5+
- [ ] TEST: Connect with an invalid token
6+
- WHEN: Create chat adapter with an invalid token
7+
- THEN: Should observe `Connecting` -> `FailedToConnect`
8+
- WHEN: Call `reconnect()` with a valid token
9+
- THEN: Should observe `Online`
10+
- WHEN: Call `postActivity()`
11+
- THEN: Should send message
12+
- THEN: Should receive bot reply
13+
- [ ] TEST: Connect with a non-existing conversation ID
14+
- WHEN: Create chat adapter with a non-existing conversation ID
15+
- THEN: Should observe `Connecting` -> `FailedToConnect`
16+
- WHEN: Call `reconnect()` with a valid conversation ID
17+
- THEN: Should observe `Online`
18+
- WHEN: Call `postActivity()`
19+
- THEN: Should send message
20+
- THEN: Should receive bot reply
21+
- [ ] TEST: Renew token should work
22+
- [ ] TEST: Call `end()` after `FailedToConnect`
23+
- WHEN: Call `connect()` without a server running
24+
- THEN: Should connect 3 times
25+
- THEN: Should observe `FailedToConnect`
26+
- WHEN: Call `end()`
27+
- THEN: `activity$` should observe completion
28+
- THEN: `connectionStatus$` Should observe `Ended` -> completion
29+
- WHEN: Call `reconnect()`
30+
- THEN: Should throw
31+
- [ ] TEST: `FailedToConnect` should be observed immediately after 3 unstable connections
32+
- WHEN: Call connect()
33+
- THEN: Should observe `Online`
34+
- WHEN: Kill the socket immediately
35+
- THEN: Should observe `Connecting`
36+
- The observation should be immediate (< 3 seconds)
37+
- THEN: Should reconnect after 3-15 seconds
38+
- WHEN: Allow it to retry-connect successfully
39+
- THEN: Should observe `Online`
40+
- WHEN: Kill the socket immediately again
41+
- THEN: Should observe `Connecting`
42+
- The observation should be immediate (< 3 seconds)
43+
- THEN: Should reconnect after 3-15 seconds
44+
- WHEN: Kill the socket immediately one more time
45+
- THEN: Should observe `FailedToConnect`
46+
- The observation should be immediate (< 3 seconds)
47+
- WHEN: Call reconnect()
48+
- THEN: Should reconnect immediately
49+
- THEN: Should observe `Online`
50+
- [ ] Make sure all state transitions are tested (see `API.md`)
51+
- In the state diagram in `API.md`, make sure all edges (arrows) has their own tests
52+
- Certain scenarios are time-sensitive, the time to the call must be asserted
53+
- For example, when transitioning from `Online` to `Connecting` for the first time, the Web Socket connection must be established within first 3 seconds
54+
- If the connection is being established after 3 seconds, it means a backoff is done
55+
- Backoff is undesirable for the first retry attempt
56+
- Class functions works differently when in different state, make sure they are properly tested
57+
- For example, when the state is `Ended`, call to `reconnect()` will throw immediately
58+
- When the state is `Connecting`, call to `postActivity()` should fail
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function activityTimestampComparer({ timestamp: x }, { timestamp: y }) {
2+
return new Date(x).getTime() - new Date(y).getTime();
3+
}
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
import { createProxyMiddleware, responseInterceptor } from 'http-proxy-middleware';
2+
import { createServer } from 'http';
3+
import { match } from 'path-to-regexp';
4+
import WebSocket, { WebSocketServer } from 'ws';
5+
import express from 'express';
6+
7+
import removeInline from './removeInline';
8+
9+
import type { Data } from 'ws';
10+
import type { IncomingMessage } from 'http';
11+
import type { Options } from 'http-proxy-middleware';
12+
import type { Socket } from 'net';
13+
14+
type OnUpgradeHandler = (req: IncomingMessage, socket: Socket, head: Buffer, next: OnUpgradeHandler) => void;
15+
16+
type OnWebSocketMessageHandler = (
17+
data: Data,
18+
socket: WebSocket,
19+
req: IncomingMessage,
20+
next: OnWebSocketMessageHandler
21+
) => void;
22+
23+
type CreateBotProxyInit = {
24+
onUpgrade?: OnUpgradeHandler;
25+
onWebSocketReceiveMessage?: OnWebSocketMessageHandler;
26+
onWebSocketSendMessage?: OnWebSocketMessageHandler;
27+
streamingBotURL?: string;
28+
};
29+
30+
type CreateBotProxyReturnValue = {
31+
cleanUp: () => void;
32+
closeAllWebSocketConnections: () => void;
33+
directLineStreamingURL: string;
34+
directLineURL: string;
35+
};
36+
37+
const matchDirectLineStreamingProtocol = match('/.bot/', { decode: decodeURIComponent, end: false });
38+
39+
export default function createBotProxy(init?: CreateBotProxyInit): Promise<CreateBotProxyReturnValue> {
40+
const onUpgrade = init?.onUpgrade || ((req, socket, head, next) => next(req, socket, head, () => {}));
41+
const onWebSocketReceiveMessage =
42+
init?.onWebSocketReceiveMessage || ((data, socket, req, next) => next(data, socket, req, () => {}));
43+
const onWebSocketSendMessage =
44+
init?.onWebSocketSendMessage || ((data, socket, req, next) => next(data, socket, req, () => {}));
45+
const streamingBotURL = init?.streamingBotURL;
46+
47+
return new Promise<CreateBotProxyReturnValue>((resolve, reject) => {
48+
try {
49+
const activeSockets: Socket[] = [];
50+
const app = express();
51+
52+
streamingBotURL &&
53+
app.use('/.bot/', createProxyMiddleware({ changeOrigin: true, logLevel: 'silent', target: streamingBotURL }));
54+
55+
const onProxyRes: Options['onProxyRes'] = responseInterceptor(
56+
async (responseBuffer, proxyRes: IncomingMessage) => {
57+
const {
58+
socket: { localAddress, localPort },
59+
statusCode
60+
} = proxyRes;
61+
62+
if (statusCode && statusCode >= 200 && statusCode < 300) {
63+
try {
64+
const json = JSON.parse(responseBuffer.toString('utf8'));
65+
66+
if (json.streamUrl) {
67+
return JSON.stringify({
68+
...json,
69+
streamUrl: json.streamUrl.replace(
70+
/^wss:\/\/directline.botframework.com\/v3\/directline\//,
71+
`ws://${localAddress}:${localPort}/v3/directline/`
72+
)
73+
});
74+
}
75+
} catch (error) {
76+
// Returns original response if it is not a JSON.
77+
}
78+
}
79+
80+
return responseBuffer;
81+
82+
// There is a typing bug in `http-proxy-middleware`.
83+
// The return type of `responseIntercept` does not match `onProxyRes`.
84+
}
85+
);
86+
87+
app.use(
88+
'/v3/directline',
89+
createProxyMiddleware({
90+
changeOrigin: true,
91+
logLevel: 'silent',
92+
onProxyRes,
93+
selfHandleResponse: true,
94+
target: 'https://directline.botframework.com/'
95+
})
96+
);
97+
98+
const webSocketProxy = new WebSocketServer({ noServer: true });
99+
100+
webSocketProxy.on('connection', (socket: WebSocket, proxySocket: WebSocket, req: IncomingMessage) => {
101+
socket.addEventListener('message', ({ data }) =>
102+
onWebSocketSendMessage(data, proxySocket, req, (data, proxySocket) => proxySocket.send(data))
103+
);
104+
105+
proxySocket.addEventListener('message', ({ data }) =>
106+
onWebSocketReceiveMessage(data, socket, req, (data, socket) => socket.send(data))
107+
);
108+
});
109+
110+
const server = createServer(app);
111+
112+
server.on('error', reject);
113+
114+
server.on('upgrade', (req: IncomingMessage, socket: Socket, head: Buffer) =>
115+
onUpgrade(req, socket, head, (req, socket, head) => {
116+
activeSockets.push(socket);
117+
118+
socket.once('close', () => removeInline(activeSockets, socket));
119+
120+
const requestURL = req.url || '';
121+
122+
const isDirectLineStreaming = !!matchDirectLineStreamingProtocol(requestURL);
123+
124+
if (isDirectLineStreaming && !streamingBotURL) {
125+
console.warn('Cannot proxy /.bot/ requests without specifying "streamingBotURL".');
126+
127+
return socket.end();
128+
}
129+
130+
const targetURL = new URL(
131+
requestURL,
132+
isDirectLineStreaming ? streamingBotURL : 'wss://directline.botframework.com/'
133+
);
134+
135+
// "streamingBotURL" could be "https:" instead of "wss:".
136+
targetURL.protocol = 'wss:';
137+
138+
const proxySocket = new WebSocket(targetURL);
139+
140+
proxySocket.addEventListener('close', () => socket.end());
141+
proxySocket.addEventListener('open', () =>
142+
webSocketProxy.handleUpgrade(req, socket, head, ws =>
143+
webSocketProxy.emit('connection', ws, proxySocket, req)
144+
)
145+
);
146+
147+
socket.once('close', () => proxySocket.close());
148+
})
149+
);
150+
151+
server.listen(0, '127.0.0.1', () => {
152+
const address = server.address();
153+
154+
if (!address) {
155+
server.close();
156+
157+
return reject(new Error('Cannot get address of proxy server.'));
158+
}
159+
160+
const url = new URL(`http://${typeof address === 'string' ? address : `${address.address}:${address.port}`}`);
161+
162+
const closeAllWebSocketConnections = () => {
163+
activeSockets.map(socket => socket.end());
164+
activeSockets.splice(0);
165+
};
166+
167+
resolve({
168+
cleanUp: () => {
169+
server.close();
170+
171+
// `closeAllConnections` is introduced in Node.js 18.2.0.
172+
server.closeAllConnections?.();
173+
174+
// Calling close() and closeAllConnections() will not close all Web Socket connections.
175+
closeAllWebSocketConnections();
176+
},
177+
closeAllWebSocketConnections,
178+
directLineStreamingURL: new URL('/.bot/v3/directline', url).href,
179+
directLineURL: new URL('/v3/directline', url).href
180+
});
181+
});
182+
} catch (error) {
183+
reject(error);
184+
}
185+
});
186+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
(expect as any).activityContaining = (messageText: string, mergeActivity: { id?: string; type?: string } = {}) =>
2+
expect.objectContaining({
3+
id: expect.any(String),
4+
text: messageText,
5+
timestamp: expect.any(String),
6+
type: 'message',
7+
8+
...mergeActivity
9+
});
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*!
2+
* The MIT License (MIT)
3+
* Copyright (c) 2017 Kent C. Dodds
4+
*
5+
* Permission is hereby granted, free of charge, to any person obtaining a copy
6+
* of this software and associated documentation files (the "Software"), to deal
7+
* in the Software without restriction, including without limitation the rights
8+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
* copies of the Software, and to permit persons to whom the Software is
10+
* furnished to do so, subject to the following conditions:
11+
*
12+
* The above copyright notice and this permission notice shall be included in all
13+
* copies or substantial portions of the Software.
14+
*
15+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
21+
* SOFTWARE.
22+
*/
23+
24+
// Adopted from @testing-library/dom and removed dependencies on DOM.
25+
// https://github.com/testing-library/dom-testing-library/blob/eadf7485430968df8d1e1293535d78cdbeea20a5/src/helpers.js
26+
27+
export default function jestFakeTimersAreEnabled(): boolean {
28+
/* istanbul ignore else */
29+
// eslint-disable-next-line
30+
if (typeof jest !== 'undefined' && jest !== null) {
31+
return (
32+
// legacy timers
33+
(setTimeout as any)._isMockFunction === true ||
34+
// modern timers
35+
// eslint-disable-next-line prefer-object-has-own -- not supported by our support matrix
36+
Object.prototype.hasOwnProperty.call(setTimeout, 'clock')
37+
);
38+
}
39+
40+
// istanbul ignore next
41+
return false;
42+
}

0 commit comments

Comments
 (0)