Added error messages to abortWebSocketUpgrade() (#1741)

* Added error messages to abortWebSocketUpgrade() * changed unit tests to use trycatch blocks * changed to error message thrown Co-authored-by: johnataylor <johtaylo@microsoft.com> Co-authored-by: Chris Mullins <cleemullins@users.noreply.github.com>
microsoft · Feb 25, 2020 · b8711ea · b8711ea
1 parent f5a32d8
commit b8711ea
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 15 deletions.
diff --git a/libraries/botbuilder/src/botFrameworkAdapter.ts b/libraries/botbuilder/src/botFrameworkAdapter.ts
@@ -1216,10 +1216,14 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
         } catch (err) {
             // If the authenticateConnection call fails, send back the correct error code and close
             // the connection.
-            if (typeof(err.message) === 'string' && err.message.toLowerCase().startsWith('unauthorized')) {
-                abortWebSocketUpgrade(socket, 401);
-            } else if (typeof(err.message) === 'string' && err.message.toLowerCase().startsWith(`'authheader'`)) {
-                abortWebSocketUpgrade(socket, 400);
+            if (typeof(err.message) === 'string') {
+                if (err.message.toLowerCase().startsWith('unauthorized')) {
+                    abortWebSocketUpgrade(socket, 401, err.message);
+                } else if (err.message.toLowerCase().startsWith(`'authheader'`)) {
+                    abortWebSocketUpgrade(socket, 400, err.message);
+                } else {
+                    abortWebSocketUpgrade(socket, 500, err.message);
+                }
             } else {
                 abortWebSocketUpgrade(socket, 500);
             }
@@ -1355,10 +1359,10 @@ function delay(timeout: number): Promise<void> {
     });
 }
 
-function abortWebSocketUpgrade(socket: INodeSocket, code: number) {
+function abortWebSocketUpgrade(socket: INodeSocket, code: number, message?: string) {
     if (socket.writable) {
         const connectionHeader = `Connection: 'close'\r\n`;
-        socket.write(`HTTP/1.1 ${code} ${STATUS_CODES[code]}\r\n${connectionHeader}\r\n`);
+        socket.write(`HTTP/1.1 ${code} ${STATUS_CODES[code]}\r\n${message}\r\n${connectionHeader}\r\n`);
     }
 
     socket.destroy();

diff --git a/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js b/libraries/botbuilder/tests/streaming/botFrameworkAdapterStreaming.test.js
@@ -117,17 +117,63 @@ describe('BotFrameworkAdapter Streaming tests', () => {
             const socket = new MockNetSocket();
             const writeSpy = spy(socket, 'write');
             const destroySpy = spy(socket, 'destroy');
-
-            await adapter.useWebSocket(request, socket, Buffer.from([]), async (context) => {
-                await bot.run(context);
-                throw new Error('useWebSocket should have thrown an error');
-            }).catch(err => {
+
+            try {
+                await adapter.useWebSocket(request, socket, Buffer.from([]), async (context) => {
+                    await bot.run(context);
+                });
+            } catch (err) {
                 expect(err.message).to.equal('Unauthorized. No valid identity.');
-                const socketResponse = MockNetSocket.createNonSuccessResponse(401);
+                const socketResponse = MockNetSocket.createNonSuccessResponse(401, err.message);
                 expect(writeSpy.called).to.be.true;
                 expect(writeSpy.calledWithExactly(socketResponse)).to.be.true;
                 expect(destroySpy.calledOnceWithExactly()).to.be.true;
-            });
+            }
+        });
+
+        it('returns status code 400 when request is missing Authorization header', async () => {
+            const bot = new ActivityHandler();
+            settings = new TestAdapterSettings('appId', 'password');
+            const adapter = new BotFrameworkAdapter(settings);
+            const requestWithoutAuthHeader = new MockHttpRequest();
+
+            const socket = new MockNetSocket();
+            const writeSpy = spy(socket, 'write');
+            const destroySpy = spy(socket, 'destroy');
+
+            try {
+                await adapter.useWebSocket(requestWithoutAuthHeader, socket, Buffer.from([]), async (context) => {
+                    await bot.run(context);
+                });
+            } catch (err) {
+                expect(err.message).to.equal("'authHeader' required.");
+                const socketResponse = MockNetSocket.createNonSuccessResponse(400, err.message);
+                expect(writeSpy.called).to.be.true;
+                expect(writeSpy.calledWithExactly(socketResponse)).to.be.true;
+                expect(destroySpy.calledOnceWithExactly()).to.be.true;
+            };
+        });
+
+        try {
+
+        } catch (error) {
+
+        }
+
+        it('returns status code 500 when request logic is not callable', async () => {
+            const adapter = new BotFrameworkAdapter(new TestAdapterSettings());
+            const request = new MockHttpRequest();
+            const socket = new MockNetSocket();
+
+            const useWebSocketSpy = spy(adapter, 'useWebSocket');
+            const uncallableLogic = null;
+
+            try {
+                await adapter.useWebSocket(request, socket, Buffer.from([]), uncallableLogic);    
+            } catch (err) {
+                expect(err.message).to.equal('Streaming logic needs to be provided to `useWebSocket`');
+                expect(useWebSocketSpy.called).to.be.true;
+            }
         });
     });
 

diff --git a/libraries/botbuilder/tests/streaming/mockNetSocket.js b/libraries/botbuilder/tests/streaming/mockNetSocket.js
@@ -11,8 +11,8 @@ class MockNetSocket {
     destroy(err) { }
 }
 
-MockNetSocket.createNonSuccessResponse = (code) => {
-    return `HTTP/1.1 ${code} ${STATUS_CODES[code]}\r\nConnection: 'close'\r\n\r\n`;
+MockNetSocket.createNonSuccessResponse = (code, message) => {
+    return `HTTP/1.1 ${code} ${STATUS_CODES[code]}\r\n${message}\r\nConnection: 'close'\r\n\r\n`;
 };
 
 module.exports.MockNetSocket = MockNetSocket;