fix: correct QnAMaker response types and expose error from server (#3279

) * fixed qnamaker response types * Exposed error message from qnamaker server * undefined included as return type * code simplified
microsoft · Feb 8, 2021 · 93a5cb4 · 93a5cb4
1 parent bc2bda0
commit 93a5cb4
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 45 deletions.
diff --git a/libraries/botbuilder-ai/src/qnamaker-utils/generateAnswerUtils.ts b/libraries/botbuilder-ai/src/qnamaker-utils/generateAnswerUtils.ts
@@ -82,14 +82,18 @@ export class GenerateAnswerUtils {
             ...queryOptions,
         });
 
-        const qnaResultJson = await this.httpRequestUtils.executeHttpRequest(
+        const qnaResults = await this.httpRequestUtils.executeHttpRequest(
             url,
             payloadBody,
             this.endpoint,
             queryOptions.timeout
         );
 
-        return this.formatQnaResult(qnaResultJson);
+        if (Array.isArray(qnaResults?.answers)) {
+            return this.formatQnaResult(qnaResults);
+        }
+
+        throw new Error(`Failed to generate answers: ${qnaResults}`);
     }
 
     /**
@@ -166,22 +170,19 @@ export class GenerateAnswerUtils {
             .sort((a: QnAMakerResult, b: QnAMakerResult) => b.score - a.score);
     }
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    private formatQnaResult(qnaResult: any): QnAMakerResults {
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        qnaResult.answers = qnaResult.answers.map((ans: any) => {
-            ans.score = ans.score / 100;
+    private formatQnaResult(qnaResult: QnAMakerResults): QnAMakerResults {
+        qnaResult.answers = qnaResult.answers.map((answer: QnAMakerResult & { qnaId?: number }) => {
+            answer.score = answer.score / 100;
 
-            if (ans.qnaId) {
-                ans.id = ans.qnaId;
-                delete ans.qnaId;
+            if (answer.qnaId) {
+                answer.id = answer.qnaId;
+                delete answer.qnaId;
             }
 
-            return ans as QnAMakerResult;
+            return answer;
         });
 
-        qnaResult.activeLearningEnabled =
-            qnaResult.activeLearningEnabled != null ? qnaResult.activeLearningEnabled : true;
+        qnaResult.activeLearningEnabled ??= true;
 
         return qnaResult;
     }

diff --git a/libraries/botbuilder-ai/src/qnamaker-utils/httpRequestUtils.ts b/libraries/botbuilder-ai/src/qnamaker-utils/httpRequestUtils.ts
@@ -9,7 +9,7 @@
 import * as os from 'os';
 
 import { QnAMakerEndpoint } from '../qnamaker-interfaces/qnamakerEndpoint';
-import { QnAMakerResult } from '../qnamaker-interfaces/qnamakerResult';
+import { QnAMakerResults } from '../qnamaker-interfaces/qnamakerResults';
 
 import { getFetch } from '../globals';
 const fetch = getFetch();
@@ -31,14 +31,14 @@ export class HttpRequestUtils {
      * @param {string} payloadBody Http request body.
      * @param {QnAMakerEndpoint} endpoint QnA Maker endpoint details.
      * @param {number} timeout (Optional)Timeout for http call
-     * @returns {QnAMakerResult} a promise that resolves to the QnAMakerResult
+     * @returns {QnAMakerResults} a promise that resolves to the QnAMakerResults
      */
     public async executeHttpRequest(
         requestUrl: string,
         payloadBody: string,
         endpoint: QnAMakerEndpoint,
         timeout?: number
-    ): Promise<QnAMakerResult> {
+    ): Promise<QnAMakerResults | undefined> {
         if (!requestUrl) {
             throw new TypeError('Request url cannot be null.');
         }
@@ -60,7 +60,7 @@ export class HttpRequestUtils {
             body: payloadBody,
         });
 
-        return qnaResult.status == 204 ? this.getSuccessful204Result() : await qnaResult.json();
+        return qnaResult.status !== 204 ? await qnaResult.json() : undefined;
     }
 
     /**
@@ -92,22 +92,4 @@ export class HttpRequestUtils {
 
         return `${packageUserAgent} ${platformUserAgent}`;
     }
-
-    /**
-     * Creates a QnAMakerResult for successful responses from QnA Maker service that return status code 204 No-Content.
-     * The [Train API](https://docs.microsoft.com/en-us/rest/api/cognitiveservices/qnamakerruntime/runtime/train)
-     * is an example of one of QnA Maker's APIs that return a 204 status code.
-     *
-     * @private
-     */
-    private getSuccessful204Result(): QnAMakerResult {
-        return {
-            questions: [],
-            answer: '204 No-Content',
-            score: 100,
-            id: -1,
-            source: null,
-            metadata: [],
-        };
-    }
 }
diff --git a/libraries/botbuilder-ai/tests/qnaMaker.test.js b/libraries/botbuilder-ai/tests/qnaMaker.test.js
@@ -947,15 +947,6 @@ describe('QnAMaker', function () {
         const trainApi = `/knowledgebases/${testKbId}/train`;
         const feedbackRecords = [{ userId: 'user1', userQuestion: 'wi-fi', qnaId: '17' }];
 
-        const successfullyTrainedResult = {
-            questions: [],
-            answer: '204 No-Content',
-            score: 100,
-            id: -1,
-            source: null,
-            metadata: [],
-        };
-
         it('executeHttpRequest() should return JSON result from Train API response of 204 No-Content', async function () {
             nock(endpoint.host).post(trainApi).reply(204);
 
@@ -967,7 +958,7 @@ describe('QnAMaker', function () {
 
             const qnaResult = await httpUtils.executeHttpRequest(endpoint.host + trainApi, payloadBody, endpoint);
 
-            assert.deepStrictEqual(qnaResult, successfullyTrainedResult);
+            assert.strictEqual(qnaResult, undefined);
         });
 
         it('executeHttpRequest() should throw when payload to QnA Service is malformed', async function () {