-
Notifications
You must be signed in to change notification settings - Fork 127
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
Showing
43 changed files
with
5,982 additions
and
4,760 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
# To-dos | ||
|
||
Due to resources constraints, while we are working on PR #404 to improve the code quality, there are scenarios we missed. | ||
|
||
- [ ] TEST: Connect with an invalid token | ||
- WHEN: Create chat adapter with an invalid token | ||
- THEN: Should observe `Connecting` -> `FailedToConnect` | ||
- WHEN: Call `reconnect()` with a valid token | ||
- THEN: Should observe `Online` | ||
- WHEN: Call `postActivity()` | ||
- THEN: Should send message | ||
- THEN: Should receive bot reply | ||
- [ ] TEST: Connect with a non-existing conversation ID | ||
- WHEN: Create chat adapter with a non-existing conversation ID | ||
- THEN: Should observe `Connecting` -> `FailedToConnect` | ||
- WHEN: Call `reconnect()` with a valid conversation ID | ||
- THEN: Should observe `Online` | ||
- WHEN: Call `postActivity()` | ||
- THEN: Should send message | ||
- THEN: Should receive bot reply | ||
- [ ] TEST: Renew token should work | ||
- [ ] TEST: Call `end()` after `FailedToConnect` | ||
- WHEN: Call `connect()` without a server running | ||
- THEN: Should connect 3 times | ||
- THEN: Should observe `FailedToConnect` | ||
- WHEN: Call `end()` | ||
- THEN: `activity$` should observe completion | ||
- THEN: `connectionStatus$` Should observe `Ended` -> completion | ||
- WHEN: Call `reconnect()` | ||
- THEN: Should throw | ||
- [ ] TEST: `FailedToConnect` should be observed immediately after 3 unstable connections | ||
- WHEN: Call connect() | ||
- THEN: Should observe `Online` | ||
- WHEN: Kill the socket immediately | ||
- THEN: Should observe `Connecting` | ||
- The observation should be immediate (< 3 seconds) | ||
- THEN: Should reconnect after 3-15 seconds | ||
- WHEN: Allow it to retry-connect successfully | ||
- THEN: Should observe `Online` | ||
- WHEN: Kill the socket immediately again | ||
- THEN: Should observe `Connecting` | ||
- The observation should be immediate (< 3 seconds) | ||
- THEN: Should reconnect after 3-15 seconds | ||
- WHEN: Kill the socket immediately one more time | ||
- THEN: Should observe `FailedToConnect` | ||
- The observation should be immediate (< 3 seconds) | ||
- WHEN: Call reconnect() | ||
- THEN: Should reconnect immediately | ||
- THEN: Should observe `Online` | ||
- [ ] Make sure all state transitions are tested (see `API.md`) | ||
- In the state diagram in `API.md`, make sure all edges (arrows) has their own tests | ||
- Certain scenarios are time-sensitive, the time to the call must be asserted | ||
- For example, when transitioning from `Online` to `Connecting` for the first time, the Web Socket connection must be established within first 3 seconds | ||
- If the connection is being established after 3 seconds, it means a backoff is done | ||
- Backoff is undesirable for the first retry attempt | ||
- Class functions works differently when in different state, make sure they are properly tested | ||
- For example, when the state is `Ended`, call to `reconnect()` will throw immediately | ||
- When the state is `Connecting`, call to `postActivity()` should fail |
3 changes: 3 additions & 0 deletions
3
__tests__/directLineStreaming/__setup__/activityTimestampComparer.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function activityTimestampComparer({ timestamp: x }, { timestamp: y }) { | ||
return new Date(x).getTime() - new Date(y).getTime(); | ||
} |
186 changes: 186 additions & 0 deletions
186
__tests__/directLineStreaming/__setup__/createBotProxy.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,186 @@ | ||
import { createProxyMiddleware, responseInterceptor } from 'http-proxy-middleware'; | ||
import { createServer } from 'http'; | ||
import { match } from 'path-to-regexp'; | ||
import WebSocket, { WebSocketServer } from 'ws'; | ||
import express from 'express'; | ||
|
||
import removeInline from './removeInline'; | ||
|
||
import type { Data } from 'ws'; | ||
import type { IncomingMessage } from 'http'; | ||
import type { Options } from 'http-proxy-middleware'; | ||
import type { Socket } from 'net'; | ||
|
||
type OnUpgradeHandler = (req: IncomingMessage, socket: Socket, head: Buffer, next: OnUpgradeHandler) => void; | ||
|
||
type OnWebSocketMessageHandler = ( | ||
data: Data, | ||
socket: WebSocket, | ||
req: IncomingMessage, | ||
next: OnWebSocketMessageHandler | ||
) => void; | ||
|
||
type CreateBotProxyInit = { | ||
onUpgrade?: OnUpgradeHandler; | ||
onWebSocketReceiveMessage?: OnWebSocketMessageHandler; | ||
onWebSocketSendMessage?: OnWebSocketMessageHandler; | ||
streamingBotURL?: string; | ||
}; | ||
|
||
type CreateBotProxyReturnValue = { | ||
cleanUp: () => void; | ||
closeAllWebSocketConnections: () => void; | ||
directLineStreamingURL: string; | ||
directLineURL: string; | ||
}; | ||
|
||
const matchDirectLineStreamingProtocol = match('/.bot/', { decode: decodeURIComponent, end: false }); | ||
|
||
export default function createBotProxy(init?: CreateBotProxyInit): Promise<CreateBotProxyReturnValue> { | ||
const onUpgrade = init?.onUpgrade || ((req, socket, head, next) => next(req, socket, head, () => {})); | ||
const onWebSocketReceiveMessage = | ||
init?.onWebSocketReceiveMessage || ((data, socket, req, next) => next(data, socket, req, () => {})); | ||
const onWebSocketSendMessage = | ||
init?.onWebSocketSendMessage || ((data, socket, req, next) => next(data, socket, req, () => {})); | ||
const streamingBotURL = init?.streamingBotURL; | ||
|
||
return new Promise<CreateBotProxyReturnValue>((resolve, reject) => { | ||
try { | ||
const activeSockets: Socket[] = []; | ||
const app = express(); | ||
|
||
streamingBotURL && | ||
app.use('/.bot/', createProxyMiddleware({ changeOrigin: true, logLevel: 'silent', target: streamingBotURL })); | ||
|
||
const onProxyRes: Options['onProxyRes'] = responseInterceptor( | ||
async (responseBuffer, proxyRes: IncomingMessage) => { | ||
const { | ||
socket: { localAddress, localPort }, | ||
statusCode | ||
} = proxyRes; | ||
|
||
if (statusCode && statusCode >= 200 && statusCode < 300) { | ||
try { | ||
const json = JSON.parse(responseBuffer.toString('utf8')); | ||
|
||
if (json.streamUrl) { | ||
return JSON.stringify({ | ||
...json, | ||
streamUrl: json.streamUrl.replace( | ||
/^wss:\/\/directline.botframework.com\/v3\/directline\//, | ||
`ws://${localAddress}:${localPort}/v3/directline/` | ||
) | ||
}); | ||
} | ||
} catch (error) { | ||
// Returns original response if it is not a JSON. | ||
} | ||
} | ||
|
||
return responseBuffer; | ||
|
||
// There is a typing bug in `http-proxy-middleware`. | ||
// The return type of `responseIntercept` does not match `onProxyRes`. | ||
} | ||
); | ||
|
||
app.use( | ||
'/v3/directline', | ||
createProxyMiddleware({ | ||
changeOrigin: true, | ||
logLevel: 'silent', | ||
onProxyRes, | ||
selfHandleResponse: true, | ||
target: 'https://directline.botframework.com/' | ||
}) | ||
); | ||
|
||
const webSocketProxy = new WebSocketServer({ noServer: true }); | ||
|
||
webSocketProxy.on('connection', (socket: WebSocket, proxySocket: WebSocket, req: IncomingMessage) => { | ||
socket.addEventListener('message', ({ data }) => | ||
onWebSocketSendMessage(data, proxySocket, req, (data, proxySocket) => proxySocket.send(data)) | ||
); | ||
|
||
proxySocket.addEventListener('message', ({ data }) => | ||
onWebSocketReceiveMessage(data, socket, req, (data, socket) => socket.send(data)) | ||
); | ||
}); | ||
|
||
const server = createServer(app); | ||
|
||
server.on('error', reject); | ||
|
||
server.on('upgrade', (req: IncomingMessage, socket: Socket, head: Buffer) => | ||
onUpgrade(req, socket, head, (req, socket, head) => { | ||
activeSockets.push(socket); | ||
|
||
socket.once('close', () => removeInline(activeSockets, socket)); | ||
|
||
const requestURL = req.url || ''; | ||
|
||
const isDirectLineStreaming = !!matchDirectLineStreamingProtocol(requestURL); | ||
|
||
if (isDirectLineStreaming && !streamingBotURL) { | ||
console.warn('Cannot proxy /.bot/ requests without specifying "streamingBotURL".'); | ||
|
||
return socket.end(); | ||
} | ||
|
||
const targetURL = new URL( | ||
requestURL, | ||
isDirectLineStreaming ? streamingBotURL : 'wss://directline.botframework.com/' | ||
); | ||
|
||
// "streamingBotURL" could be "https:" instead of "wss:". | ||
targetURL.protocol = 'wss:'; | ||
|
||
const proxySocket = new WebSocket(targetURL); | ||
|
||
proxySocket.addEventListener('close', () => socket.end()); | ||
proxySocket.addEventListener('open', () => | ||
webSocketProxy.handleUpgrade(req, socket, head, ws => | ||
webSocketProxy.emit('connection', ws, proxySocket, req) | ||
) | ||
); | ||
|
||
socket.once('close', () => proxySocket.close()); | ||
}) | ||
); | ||
|
||
server.listen(0, '127.0.0.1', () => { | ||
const address = server.address(); | ||
|
||
if (!address) { | ||
server.close(); | ||
|
||
return reject(new Error('Cannot get address of proxy server.')); | ||
} | ||
|
||
const url = new URL(`http://${typeof address === 'string' ? address : `${address.address}:${address.port}`}`); | ||
|
||
const closeAllWebSocketConnections = () => { | ||
activeSockets.map(socket => socket.end()); | ||
activeSockets.splice(0); | ||
}; | ||
|
||
resolve({ | ||
cleanUp: () => { | ||
server.close(); | ||
|
||
// `closeAllConnections` is introduced in Node.js 18.2.0. | ||
server.closeAllConnections?.(); | ||
|
||
// Calling close() and closeAllConnections() will not close all Web Socket connections. | ||
closeAllWebSocketConnections(); | ||
}, | ||
closeAllWebSocketConnections, | ||
directLineStreamingURL: new URL('/.bot/v3/directline', url).href, | ||
directLineURL: new URL('/v3/directline', url).href | ||
}); | ||
}); | ||
} catch (error) { | ||
reject(error); | ||
} | ||
}); | ||
} |
9 changes: 9 additions & 0 deletions
9
__tests__/directLineStreaming/__setup__/expect/activityContaining.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
(expect as any).activityContaining = (messageText: string, mergeActivity: { id?: string; type?: string } = {}) => | ||
expect.objectContaining({ | ||
id: expect.any(String), | ||
text: messageText, | ||
timestamp: expect.any(String), | ||
type: 'message', | ||
|
||
...mergeActivity | ||
}); |
42 changes: 42 additions & 0 deletions
42
__tests__/directLineStreaming/__setup__/external/testing-library/jestFakeTimersAreEnabled.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/*! | ||
* The MIT License (MIT) | ||
* Copyright (c) 2017 Kent C. Dodds | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included in all | ||
* copies or substantial portions of the Software. | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
|
||
// Adopted from @testing-library/dom and removed dependencies on DOM. | ||
// https://github.com/testing-library/dom-testing-library/blob/eadf7485430968df8d1e1293535d78cdbeea20a5/src/helpers.js | ||
|
||
export default function jestFakeTimersAreEnabled(): boolean { | ||
/* istanbul ignore else */ | ||
// eslint-disable-next-line | ||
if (typeof jest !== 'undefined' && jest !== null) { | ||
return ( | ||
// legacy timers | ||
(setTimeout as any)._isMockFunction === true || | ||
// modern timers | ||
// eslint-disable-next-line prefer-object-has-own -- not supported by our support matrix | ||
Object.prototype.hasOwnProperty.call(setTimeout, 'clock') | ||
); | ||
} | ||
|
||
// istanbul ignore next | ||
return false; | ||
} |
Oops, something went wrong.