Catch response deserialization errors. (#478)

* Catch unexpected errors. Signed-off-by: dblock <[email protected]> * Eat the error instead of catching it. Signed-off-by: dblock <[email protected]> * Distinguish errors and errors. Signed-off-by: dblock <[email protected]> --------- Signed-off-by: dblock <[email protected]>
opensearch-project · Aug 12, 2024 · 3828838 · 3828838
1 parent cb320b5
commit 3828838
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 16 deletions.
diff --git a/tools/src/tester/ChapterEvaluator.ts b/tools/src/tester/ChapterEvaluator.ts
@@ -122,15 +122,17 @@ export default class ChapterEvaluator {
 
   #evaluate_status(chapter: Chapter, response: ActualResponse): Evaluation {
     const expected_status = chapter.response?.status ?? 200
-    if (response.status === expected_status) return { result: Result.PASSED }
+    if (response.status === expected_status && response.error === undefined) return { result: Result.PASSED }
 
-    const result: Evaluation = {
+    let result: Evaluation = {
       result: Result.ERROR,
       message: _.join(_.compact([
-        `Expected status ${expected_status}, but received ${response.status}: ${response.content_type}.`,
+        expected_status == response.status ?
+          `Received ${response.status ?? 'none'}: ${response.content_type ?? 'unknown'}.` :
+          `Expected status ${expected_status}, but received ${response.status ?? 'none'}: ${response.content_type ?? 'unknown'}.`,
         response.message
       ]), ' ')
-    };
+    }
 
     if (response.error !== undefined) {
       result.error = response.error as Error

diff --git a/tools/src/tester/ChapterReader.ts b/tools/src/tester/ChapterReader.ts
@@ -56,16 +56,16 @@ export default class ChapterReader {
     }).catch(e => {
       if (e.response == null) {
         this.logger.info(`<= ERROR: ${e}`)
-        throw e
+        response.message = e.message
+        response.error = e
+      } else {
+        response.status = e.response.status
+        response.content_type = e.response.headers['content-type']?.split(';')[0]
+        const payload = this.#deserialize_payload(e.response.data, response.content_type)
+        if (payload !== undefined) response.payload = payload.error
+        response.message = payload.error?.reason ?? e.response.statusText
+        this.logger.info(`<= ${response.status} (${response.content_type}) | ${response.payload !== undefined ? to_json(response.payload) : response.message}`)
       }
-      response.status = e.response.status
-      response.content_type = e.response.headers['content-type']?.split(';')[0]
-      const payload = this.#deserialize_payload(e.response.data, response.content_type)
-      if (payload !== undefined) response.payload = payload.error
-      response.message = payload.error?.reason ?? e.response.statusText
-      response.error = e
-
-      this.logger.info(`<= ${response.status} (${response.content_type}) | ${response.payload !== undefined ? to_json(response.payload) : response.message}`)
     })
     return response as ActualResponse
   }

diff --git a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml
@@ -33,7 +33,6 @@ chapters:
         result: ERROR
         message: 'Expected status 200, but received 404: application/json. no such index
           [undefined]'
-        error: Request failed with status code 404
       payload_body:
         result: SKIPPED
       payload_schema:

diff --git a/tools/tests/tester/fixtures/evals/error/prologue_error.yaml b/tools/tests/tester/fixtures/evals/error/prologue_error.yaml
@@ -16,7 +16,6 @@ prologues:
     overall:
       result: ERROR
       message: no such index [does_not_exists]
-      error: Request failed with status code 404
 
 chapters:
   - title: This chapter be skipped.

diff --git a/tools/tests/tester/fixtures/stories/failed/not_found.yaml b/tools/tests/tester/fixtures/stories/failed/not_found.yaml
@@ -29,4 +29,3 @@ chapters:
       index: movies
     response:
       status: 404
-
diff --git a/tools/tests/tester/integ/StoryEvaluator.test.ts b/tools/tests/tester/integ/StoryEvaluator.test.ts
@@ -7,6 +7,7 @@
 * compatible open source license.
 */
 
+import { Result } from 'tester/types/eval.types'
 import { construct_tester_components, load_actual_evaluation, load_expected_evaluation } from '../helpers'
 
 const { story_evaluator, opensearch_http_client } = construct_tester_components('tools/tests/tester/fixtures/specs/excerpt.yaml')
@@ -16,6 +17,10 @@ beforeAll(async () => {
   expect(info.version).toBeDefined()
 })
 
+afterEach(() => {
+  jest.resetAllMocks()
+})
+
 test('passed', async () => {
   const actual = await load_actual_evaluation(story_evaluator, 'passed')
   const expected = load_expected_evaluation('passed')
@@ -57,3 +62,42 @@ test('skipped/semver', async () => {
   const expected = load_expected_evaluation('skipped/semver')
   expect(actual).toEqual(expected)
 })
+
+test('with an unexpected error deserializing data', async () => {
+  opensearch_http_client.request = jest.fn().mockRejectedValue(new Error('This was unexpected.'))
+  const actual = await load_actual_evaluation(story_evaluator, 'passed')
+  expect(actual.result).toEqual(Result.ERROR)
+  expect(actual.chapters && actual.chapters[0]).toEqual({
+    title: "This PUT /{index} chapter should pass.",
+    path: 'PUT /{index}',
+    overall: {
+      result: Result.ERROR
+    },
+    request: {
+      parameters: {
+        index: {
+          result: Result.PASSED
+        },
+      },
+      request: {
+        result: Result.PASSED
+      }
+    },
+    response: {
+      output_values: {
+        result: Result.SKIPPED
+      },
+      payload_body: {
+        result: Result.SKIPPED
+      },
+      payload_schema: {
+        result: Result.SKIPPED
+      },
+      status: {
+        error: 'This was unexpected.',
+        message: 'Expected status 200, but received none: unknown. This was unexpected.',
+        result: Result.ERROR
+      }
+    }
+  })
+})